litecoin MWEB tx deserializer

[chappjc]

Resolves https://github.com/spesmilo/electrumx/issues/179

Works to sync testnet and mainnet with litecoind 0.21.2. Python and litecoin are not my core competencies but I need this fixed nevertheless. I hope someone more knowledgeable about the MWEB upgrade can vet this.

EDIT: This may not work for mempool txns, which also seem to be processed with these deserialization functions.

[benma]

@chappjc any chance you could pick this up again?

Can you explain in more detail what the problem was with this PR?

[chappjc]

It works for all transactions in blocks, but certain MWEB transactions in mempool will be parsed incorrectly (wrong locktime and hash/txid). That might be OK however because such transactions do not end up in the canonical block anyway, instead landing in a special MWEB that does not concern electrumx (yet).

This issue is here:

        if flag & 8: # MWEB flag
            # Given this transaction is in the main block, not the MW extension
            # block, this should be the HogEx/integration transaction, which has
            # no mweb tx, just a zero byte.
            # https://github.com/litecoin-project/litecoin/blob/bb242e33551157e0db3b70f90f5738f34b82cc51/src/mweb/mweb_node.cpp#L17-L20
            # assert self._read_byte() == 0 # fail if there is a mweb tx
            self._read_byte()

        start = self.cursor
        locktime = self._read_le_uint32()
        orig_ser += self.binary[start:self.cursor]
        vsize = (3 * base_size + self.binary_length) // 4

If the transaction is flagged as MW, there will be a place for the mweb tx data just before the locktime bytes. For all mw-flaged transactions in a block, there is just a single byte there indicating no mweb tx data, so we can just discard one byte (that indicates null mw tx) and move on to the locktime.

However, in mempool there are MW txns that have more data here, and the length is variable, so it can only be processed by parsing the mw tx section properly. In particular, if you uncomment that assert self._read_byte() == 0, it will eventually crash with such a tx in mempool. It's quite unfortunate that they decided to put lockTime after the mw tx data because even if we just want to discard that data (it doesn't affect the tx hash, so we can), we have to parse it correctly to know it's length.

That said, I've been running this branch for months on testnet and it processes all blocks correctly, and does not crash for any mempool tx, but transaction hashes for certain mweb transactions in mempool will be incorrect.

Here's how to parse this non-null MW tx section (in Go): https://github.com/decred/dcrdex/blob/89273503dcd19210b6150c9360ee9cd581b6edc9/dex/networks/ltc/tx.go#L211 But in this python code PR, we're just reading the one byte and taking the "HogEx - that's all folks" path even if there is more data there.

[benma]

@chappjc thanks so much for this great write-up.

Does that mean that transactions fetched via the blockchain.transaction.get method will also contain this new transaction serialization format? If so, that would break wallet apps and the wallet apps would need to be patched beforehand. If so, would -rpcserialversion=1 fix that?

Edit:

Maybe a simpler intermediate fix is to discard mempool transactions that contain more data there?

[chappjc]

@chappjc thanks so much for this great write-up.

Does that mean that transactions fetched via the blockchain.transaction.get method will also contain this new transaction serialization format? If so, that would break wallet apps and the wallet apps would need to be patched beforehand. If so, would -rpcserialversion=1 fix that?

First Q, yes, but the only mined transaction in regular blocks like this would be the HogEx/integ transaction. The others do not have the mw flag bit set.

With -rpcserialversion=1 there's still an issue because when these pure mw tnxs reported by getrawmempool are stripped down to v1 serialization they crash the parser. I don't recall why, I think because they have no inputs. But assuming that is fixed, yeah, would need to v1 serialization for the consumers until they understand the v2 serialization.

Maybe a simpler intermediate fix is to discard mempool transactions that contain more data there?

Yeah, no idea how to do that though. Knowing little about electrumx (or python for that matter), I don't see how to signal to the caller to just skip it. _read_tx_parts either returns a result or throws an exception, which seems to be fatal to the caller. This surely must be simple but I don't know the patterns.

[SomberNight]

Maybe a simpler intermediate fix is to discard mempool transactions that contain more data there?

Yeah, no idea how to do that though. Knowing little about electrumx (or python for that matter), I don't see how to signal to the caller to just skip it. _read_tx_parts either returns a result or throws an exception, which seems to be fatal to the caller. This surely must be simple but I don't know the patterns.

You could raise an exception, and suppress/disregard it when called from the mempool code, but let it propagate when called from the block deserializer.

[chappjc]

Replaced by https://github.com/spesmilo/electrumx/pull/196 since github says I force pushed at some point and it won't reopen this one.