KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
104 stars 94 forks source link

dash coinbase transaction (de)serialization issue / wrong txid #1982

Open DeckerSU opened 1 year ago

DeckerSU commented 1 year ago

There is a bug in mm2 related to DASH coinbase (or not only?) serialization / deserialization. mm2 shows txid of tx 70ce3e63579742f3030e2a263ced5aa7ce606cbacfabdad5718387efac81211a belonged to address XnsqxQLGQv1FTwXJhakqMYL8MpT1G2tegZ as 6899ce7eff3c19ae827d82c575c2486fc0c6102e274cfe83d94604857587d30a. I did a small investigation and here is the results:

  1. mm2 output on my_tx_history for address XnsqxQLGQv1FTwXJhakqMYL8MpT1G2tegZ is:
    "result": {
    "transactions": [
      {
        "tx_hex": "03000500010000000000000000000000000000000000000000000000000000000000000000ffffffff5803dca01d04018f
    0c650cfabe6d6d0000000000000000000000000000000000000000000000000000000000000000010000000000000067f8f92f7f4d62000000000
    0156233626333643837323136306162316236303731320000000002ef67c105000000001976a914e976ec3a46c7c8b65e7914e4257c966265b9e7
    2488ac1c7d0308000000001976a91485b630193f8d9025495f51b4a3f884d32b92fa1d88ac00000000460200dca01d00d54c0324c6bbfa66bc500
    419a0c69249e70ee476f3ea589dad71616ec187d32dc7955b8b2ef67a6b5eaba5343b7770452553be3d9bfd71e7c67d020171d72655",
        "tx_hash": "6899ce7eff3c19ae827d82c575c2486fc0c6102e274cfe83d94604857587d30a",
        "from": [],
        "to": [
          "XnsqxQLGQv1FTwXJhakqMYL8MpT1G2tegZ",
          "XwyHjrAVXFSs2uHKw5BB2U31m8D41izE24"
        ],
        "total_amount": "0",
        "spent_by_me": "0",
        "received_by_me": "1.34446364",
        "my_balance_change": "1.34446364",
        "block_height": 1941724,
        "timestamp": 1695321857,
        "fee_details": {
          "type": "Utxo",
          "coin": "DASH",
          "amount": "0.000000000000000"
        },
        "coin": "DASH",
        "internal_id": "6899ce7eff3c19ae827d82c575c2486fc0c6102e274cfe83d94604857587d30a",
        "transaction_type": "StandardTransfer",
        "memo": null,
        "confirmations": 1688
      },

    As we can see tx_hash here is 6899ce7eff3c19ae827d82c575c2486fc0c6102e274cfe83d94604857587d30a, but should be 70ce3e63579742f3030e2a263ced5aa7ce606cbacfabdad5718387efac81211a instead.

  2. getrawtransaction for 70ce3e63579742f3030e2a263ced5aa7ce606cbacfabdad5718387efac81211a got directly from dashd is
    03000500010000000000000000000000000000000000000000000000000000000000000000ffffffff5803dca01d04018f0c650cfabe6d6d0000000000000000000000000000000000000000000000000000000000000000010000000000000067f8f92f7f4d620000000000156233626333643837323136306162316236303731320000000002ef67c105000000001976a914e976ec3a46c7c8b65e7914e4257c966265b9e72488ac1c7d0308000000001976a91485b630193f8d9025495f51b4a3f884d32b92fa1d88ac00000000460200dca01d00d54c0324c6bbfa66bc500419a0c69249e70ee476f3ea589dad71616ec187d32dc7955b8b2ef67a6b5eaba5343b7770452553be3d9bfd71e7c67d020171d72655

    So, from the first look it corresponds tx_hex from mm2 output. But.

  3. Let's calculate txid of this tx manually, for example, let's take sha256d hash of raw tx via simple python script:
    
    import hashlib
    import binascii

def doubleSha256(hex): bin = binascii.unhexlify(hex) hash = hashlib.sha256(bin).digest() hash2 = hashlib.sha256(hash).digest() return binascii.hexlify(hash2)

Getting raw transaction data

raw = '03000500010000000000000000000000000000000000000000000000000000000000000000ffffffff5803dca01d04018f0c650cfabe6d6d0000000000000000000000000000000000000000000000000000000000000000010000000000000067f8f92f7f4d620000000000156233626333643837323136306162316236303731320000000002ef67c105000000001976a914e976ec3a46c7c8b65e7914e4257c966265b9e72488ac1c7d0308000000001976a91485b630193f8d9025495f51b4a3f884d32b92fa1d88ac00000000460200dca01d00d54c0324c6bbfa66bc500419a0c69249e70ee476f3ea589dad71616ec187d32dc7955b8b2ef67a6b5eaba5343b7770452553be3d9bfd71e7c67d020171d72655'

Calculating Double-SHA256 hash

hash = doubleSha256(raw)

Converting result to big-endian hex notation

txid = binascii.hexlify(binascii.unhexlify(hash)[::-1]) txid = str(txid,"ascii")

print("Tx_ID:\n "+txid)

It will give us `70ce3e63579742f3030e2a263ced5aa7ce606cbacfabdad5718387efac81211a`, but mm2 returns `tx_hash` = `6899ce7eff3c19ae827d82c575c2486fc0c6102e274cfe83d94604857587d30a` for this raw tx.
4. Electrum server returns correct data for this tx:

{ "id": 1, "method":"blockchain.scripthash.listunspent", "params":["b85136e199d2b5b0bcfa78c7c293e6c01d76581e0f31f402904754da5a1a700f"] }

Response is:

{"jsonrpc":"2.0","result":[{"tx_hash":"d829ccc4741b9ccc0af4f4d8af66ca6064edff09d7891b351a855cf01714df7e","tx_pos":1,"height":1934382,"value":134438317},{"tx_hash":"75424b78efaa61ce8f0e485bcca80014f3ba20b2f66afec97746264f75a686d7","tx_pos":1,"height":1939138,"value":76054298},{"tx_hash":"70ce3e63579742f3030e2a263ced5aa7ce606cbacfabdad5718387efac81211a","tx_pos":1,"height":1941724,"value":134446364}],"id":1}

`blockchain.scripthash.get_history` electrum request also gives correct txid, so it's not en Electrum issue.
5. After small debug session i found the error reason. The actial error is in `tx_details_by_hash`:
https://github.com/KomodoPlatform/komodo-defi-framework/blob/79f620559a635704959d3e88067f7ffe5ee30f1f/mm2src/coins/utxo/utxo_common.rs#L3336
Here we get correct raw tx from Electrum or native coin daemon. But later when we calculate `tx_hash` we did it from `tx` (`pub struct Transaction`):
https://github.com/KomodoPlatform/komodo-defi-framework/blob/79f620559a635704959d3e88067f7ffe5ee30f1f/mm2src/coins/utxo/utxo_common.rs#L3461-L3462
So, we got `tx` as `deserialize` of `verbose_tx`:
https://github.com/KomodoPlatform/komodo-defi-framework/blob/79f620559a635704959d3e88067f7ffe5ee30f1f/mm2src/coins/utxo/utxo_common.rs#L3337
And then inside `tx.hash()` we hashing serialized tx:
https://github.com/KomodoPlatform/komodo-defi-framework/blob/79f620559a635704959d3e88067f7ffe5ee30f1f/mm2src/mm2_bitcoin/chain/src/transaction.rs#L260-L267
So, the `tx_hash` got like `dhash256(serialize(deserialize(verbose_tx)))`.
6. The main issue that `serialize(deserialize(verbose_tx))` for the given tx gives us:

03000500010000000000000000000000000000000000000000000000000000000000000000ffffffff5803dca01d04018f0c650cfabe6d6d0000000000000000000000000000000000000000000000000000000000000000010000000000000067f8f92f7f4d620000000000156233626333643837323136306162316236303731320000000002ef67c105000000001976a914e976ec3a46c7c8b65e7914e4257c966265b9e72488ac1c7d0308000000001976a91485b630193f8d9025495f51b4a3f884d32b92fa1d88ac00000000460200dc

And the real raw tx is longer:

03000500010000000000000000000000000000000000000000000000000000000000000000ffffffff5803dca01d04018f0c650cfabe6d6d0000000000000000000000000000000000000000000000000000000000000000010000000000000067f8f92f7f4d620000000000156233626333643837323136306162316236303731320000000002ef67c105000000001976a914e976ec3a46c7c8b65e7914e4257c966265b9e72488ac1c7d0308000000001976a91485b630193f8d9025495f51b4a3f884d32b92fa1d88ac00000000460200dca01d00d54c0324c6bbfa66bc500419a0c69249e70ee476f3ea589dad71616ec187d32dc7955b8b2ef67a6b5eaba5343b7770452553be3d9bfd71e7c67d020171d72655

7. This is because `mm2` doesn't know anything about DASH "tx tail", updated transaction structure introduced in DASH [0.13.0](https://docs.dash.org/ru/0.13.0/merchants/technical.html). As we can see we have additional fields here:

"extraPayloadSize": , "extraPayload": …

For our tx it is:

"extraPayloadSize": 70, "extraPayload": "0200dca01d00d54c0324c6bbfa66bc500419a0c69249e70ee476f3ea589dad71616ec187d32dc7955b8b2ef67a6b5eaba5343b7770452553be3d9bfd71e7c67d020171d72655"


8. In the current situation, we have at least two approaches to solve this problem. The first and easiest approach is to use the patch provided in this link: https://gist.github.com/DeckerSU/4f685c0bd21e63c85de8faec39ce56f8. Since all of this code is inside the `tx_details_by_hash` function, which takes `hash: &[u8]` as an input argument, there is no need for additional serialization and deserialization of `verbose_tx` to obtain its hash. We can simply assign `tx_hash` to the given `hash`. This approach will always return the correct transaction ID for all cases in which mm2 may incorrectly serialize or deserialize a transaction, such as when there are additional transaction fields that we are not yet aware of.

Another solution is to modify the serialization and deserialization of DASH transactions to include the transaction tail, which consists of everything after `nLockTime`, such as `extraPayloadSize` and `extraPayload`. However, this solution will only fix the "wrong txid" issue for DASH transactions and not for other coins that may also have additional fields in their transactions.
DeckerSU commented 1 year ago

Also, we can check if serialize(deserialize(verbose_tx)) != verbose_tx here and throw an error (put the error in the log) to identify all the such cases.

cipig commented 1 year ago

will this https://github.com/spesmilo/electrumx/pull/234 change something?

pshenmic commented 1 year ago

Dash 0.13 is a fairly old version released in the 2019, there was another CB TX v2 introduced I believe, and there one more ongoing in the Dash v20

Although that could be easier to just pass tx hash, I feel like the proper serialization / deserialization of each known coin will be better in the long run

I eager to provide any necessary help to achieve that regarding Dash protocol changes

shamardy commented 1 year ago

I feel like the proper serialization / deserialization of each known coin will be better in the long run

Thank you for your input @pshenmic, we try to support serialization / deserialization for each known coin but new tx formats sometimes lead to problems. We aim to do this for dash too.

I eager to provide any necessary help to achieve that regarding Dash protocol changes

If you can specify what new fields were added and what fields were removed with each tx version, that would be great help. You can also just point us to these changes in code or any other useful resources and we will pick it up from there :)