invalid transaction decoding #20579

issue widdoh opened this issue on December 5, 2020
  1. widdoh commented at 7:06 PM on December 5, 2020: none

    <!-- Describe the issue -->

    using: bitcoin-tx -json 020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000 gives an error: invalid transaction encoding

    <!--- What behavior did you expect? -->

    Decode the transaction into a JSON block

    <!--- What was the actual behavior (provide screenshots if the issue is GUI-related)? -->

    error: invalid transaction encoding

    <!--- How reliably can you reproduce the issue, what are the steps to do so? -->

    All the time under a debug build x86 or release build x86 or debug Release Build x64

    <!-- What version of Bitcoin Core are you using, where did you get it (website, self-compiled, etc)? -->

    Downloaded the entire source code on 30/11/2020 19:21:00 UTC from Github

    <!-- What type of machine are you observing the error on (OS/CPU and disk type)? -->

    Windows 10 Pro, Intel Cor I7-5500 CPU@2.40 GHz, 512 GB SSD PS same error on windows server 2019 48 cores, and various other windows systems

    <!-- GUI-related issue? What is your operating system and its version? If Linux, what is your desktop environment and graphical shell? -->

    No, not GUI related

    <!-- Any extra information that might be useful in the debugging process. -->

    Compiled the source code using Visual Studio 2019 ver. 16.8.2 Compiles Successfully P.S. Downloaded pre compiled bitcoin-tx.exe File Version 0.20.1.0 31/07/2020 and tested it. It decodes the transaction successfully

    <!--- This is normally the contents of a `debug.log` or `config.log` file. Raw text or a link to a pastebin type site are preferred. -->

  2. widdoh renamed this:
    invalid transaction encoding
    invalid transaction decoding
    on Dec 5, 2020
  3. jonasschnelli commented at 8:46 AM on December 7, 2020: contributor

    Where have you encoded this transaction? Could it be that the encoding is invalid 😎?

  4. widdoh commented at 9:20 AM on December 7, 2020: none

    Lol possible except this and many more transactions where extracted from the blockchain. I decoded the entire chain and there were a few thousand similar transactions. Also note in my post I said that the precompiled Bitcoin-tx decided it properly

  5. iFA88 commented at 2:41 PM on December 7, 2020: none

    Hey, i just want to open the same issue. I use Bitcoin Core version v0.20.1 and can not decode my raw transaction:

    0200000000010176da57a8c5e8ea391d0454991aff153f2250e8b27040f20f122d07370bcac876000000006a47304402204ade4923b31e6d42c1b703a4c0fb44cb6de058abcda03e580864da3bfd14449c02205e038e3125091547f59e158067837df6e48bd958be8ce5b463ec9daca90f91440121037da65957101c17ca267052f9b7c8ddda088d37ee5b70a3ee3d2ea70b3554ce35ffffffff02e80300000000000017a914dfe89617d47489acf2fd3b024b232122f3fbc72a87281c0000000000001976a914118c67c7bf1596fa67c2571ea7595ed8021308cc88ac0000000000
    

    The bug is that when the segwit array length indicator is zero then the decoder throws error.

    When i remove the segwit mark and the data then works:

    020000000176da57a8c5e8ea391d0454991aff153f2250e8b27040f20f122d07370bcac876000000006a47304402204ade4923b31e6d42c1b703a4c0fb44cb6de058abcda03e580864da3bfd14449c02205e038e3125091547f59e158067837df6e48bd958be8ce5b463ec9daca90f91440121037da65957101c17ca267052f9b7c8ddda088d37ee5b70a3ee3d2ea70b3554ce35ffffffff02e80300000000000017a914dfe89617d47489acf2fd3b024b232122f3fbc72a87281c0000000000001976a914118c67c7bf1596fa67c2571ea7595ed8021308cc88ac00000000
    
  6. MarcoFalke commented at 4:08 PM on December 7, 2020: member

    Probably another regression of #17775

  7. MarcoFalke added this to the milestone 0.21.0 on Dec 7, 2020
  8. MarcoFalke added the label Utils/log/libs on Dec 7, 2020
  9. MarcoFalke added the label Bug on Dec 7, 2020
  10. jonatack commented at 9:30 PM on December 7, 2020: member

    On master: I can decode @widdoh's transaction but see the same thing as what @iFA88 reports.

  11. widdoh commented at 9:50 PM on December 7, 2020: none

    After decoding the blockchain data files I came across 8656 transactions similar to the one I posted!

  12. iFA88 commented at 10:36 PM on December 7, 2020: none

    It's possible that there is a regression here that we should look into, but inevitably the result will always be wrong for some (possibly exceptional) cases.

    That's wrong, it is very easy to decode any transaction even signed or empty.

  13. widdoh commented at 10:40 PM on December 7, 2020: none

    I am using Bitcoin-tx to do the decoding. In the source code the error is generated by : if (!DecodeHexTx(tx, line, true)) { throw std::runtime_error("invalid transaction encoding");

  14. sipa commented at 10:42 PM on December 7, 2020: member

    @widdoh You've indeed stumbled upon (yet another) inadequacy in the heuristic hex decoder. It incorrectly decides to not return the extended serialization if it doesn't pass a heurstic, even if the legacy decoder fails. See #20595 for a fix. @iFA88 Your hex encoding is actually invalid. It has the extended serialization marker ("0001") for a transaction with no witnesses. That is invalid according to BIP144.

  15. sipa commented at 10:54 PM on December 7, 2020: member

    @widdoh Your issue seems like it may be a regression caused by #17775.

  16. iFA88 commented at 11:11 PM on December 7, 2020: none

    iFA88 Your hex encoding is actually invalid. It has the extended serialization marker ("0001") for a transaction with no witnesses. That is invalid according to BIP144.

    I dont see that in https://en.bitcoin.it/wiki/BIP_0141 can you please highlight for me?

  17. sipa commented at 11:12 PM on December 7, 2020: member

    BIP144 specifies the P2P serialization, not BIP141.

    If the witness is empty, the old serialization format must be used.

  18. iFA88 commented at 11:26 PM on December 7, 2020: none

    You have right, sorry i linked bad page.

  19. widdoh commented at 12:11 AM on December 8, 2020: none

    @sipa thanks i will try the fix and decode the entire blockchain again and report any issues

  20. sipa commented at 12:13 AM on December 8, 2020: member

    @widdoh Cool. I believe it will succeed on all blockchain transaction with that patch, though in theory it's still possible that some decode incorrectly as 0-input transactions instead.

  21. widdoh commented at 12:25 AM on December 8, 2020: none

    @sipa sorry no cigar but an improvement. File blk01499.dat (just a file i tested before) had 56 such transactions. After your fix they are down to 28! Lucky i did not decode the whole chain to check :) Below is a sample tx that failed! 020000000001010000000000000000000000000000000000000000000000000000000000000000FFFFFFFF4B03C68708046FF8415C622F4254432E434F4D2FFABE6D6DE1965D02C68F928E5B244AB1965115A36F56EB997633C7F690124BBF43644E23080000000CA3D3AF6D005A65FF0200FD00000000FFFFFFFF03F4C1FB4B0000000016001497CFC76442FE717F2A3F0CC9C175F7561B6619970000000000000000266A24AA21A9ED957D1036A80343E0D1B659497E1B48A38EBE876A056D45965FAC4A85CDA84E1900000000000000002952534B424C4F434B3A8E092581AB01986CBADC84F4B43F4FA4BB9E7A2E2A0CAF9B7CF64D939028E22C0120000000000000000000000000000000000000000000000000000000000000000000000000

  22. sipa commented at 12:29 AM on December 8, 2020: member

    @widdoh That one works for me?

    $ ./src/bitcoin-tx -json 020000000001010000000000000000000000000000000000000000000000000000000000000000FFFFFFFF4B03C68708046FF8415C622F4254432E434F4D2FFABE6D6DE1965D02C68F928E5B244AB1965115A36F56EB997633C7F690124BBF43644E23080000000CA3D3AF6D005A65FF0200FD00000000FFFFFFFF03F4C1FB4B0000000016001497CFC76442FE717F2A3F0CC9C175F7561B6619970000000000000000266A24AA21A9ED957D1036A80343E0D1B659497E1B48A38EBE876A056D45965FAC4A85CDA84E1900000000000000002952534B424C4F434B3A8E092581AB01986CBADC84F4B43F4FA4BB9E7A2E2A0CAF9B7CF64D939028E22C0120000000000000000000000000000000000000000000000000000000000000000000000000
    {
        "txid": "c61adde6f1c09ef4c9864e5c5aa7d92e0c6e76e4413710cf989bae9145ef141e",
        "hash": "ed87639c4c575798908a307e4f4938b52ab1335208fdbd461d67fdab64134fb3",
        "version": 2,
    ...
    
  23. widdoh commented at 12:36 AM on December 8, 2020: none

    @sipa could you please post the source code for the decodr function just in case I copied the wrong one. Although I copied it from your post

  24. sipa commented at 12:38 AM on December 8, 2020: member
    static bool DecodeTx(CMutableTransaction& tx, const std::vector<unsigned char>& tx_data, bool try_no_witness, bool try_witness)
    {
        CMutableTransaction tx_extended, tx_legacy;
        bool ok_extended = false, ok_legacy = false;
    
        // Try decoding with extended serialization support, and remember if the result successfully consumes the entire input.
        // Note that this will generally also work correctly on transactions in legacy format (without witness). This version
        // however will interpret the 0001 marker as a marker rather than as a no-input transaction.
        if (try_witness) {
            CDataStream ssData(tx_data, SER_NETWORK, PROTOCOL_VERSION);
            try {
                ssData >> tx_extended;
                if (ssData.empty()) ok_extended = true;
            } catch (const std::exception&) {
                // Fall through.
            }
        }
    
        // Try decoding with legacy serialization, and remember if the result successfully consumes the entire input.
        if (try_no_witness) {
            CDataStream ssData(tx_data, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS);
            try {
                ssData >> tx_legacy;
                if (ssData.empty()) ok_legacy = true;
            } catch (const std::exception&) {
                // Fall through.
            }
        }
    
        // If both deserializations succeeded, prefer the one for which CheckTxScriptsSanity returns true.
        // If that's the case for both, prefer the extended one.
        if (ok_extended && CheckTxScriptsSanity(tx_extended)) {
            tx = std::move(tx_extended);
            return true;
        }
        if (ok_legacy && CheckTxScriptsSanity(tx_legacy)) {
            tx = std::move(tx_legacy);
            return true;
        }
    
        // If only one deserialization succeeded, use that one.
        if (ok_extended) {
            tx = std::move(tx_extended);
            return true;
        }
        if (ok_legacy) {
            tx = std::move(tx_legacy);
            return true;
        }
    
        // If none succeeded, we failed.
        return false;
    }
    
  25. widdoh commented at 10:10 AM on December 8, 2020: none

    @sipa my bad. The code i copied is the same you posted but apparently forgot to delete the error file from a previous cancelled run and though it was generated by your code :) I will run it now on the rest of the chain. Thanks

  26. widdoh commented at 12:23 PM on December 9, 2020: none

    @sipa well done. Entire blockchain decoded and no errors! Thank you

  27. laanwj closed this on Dec 10, 2020

  28. sidhujag referenced this in commit 0fc0cf7160 on Dec 10, 2020
  29. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me