micro-bitcoin / uBitcoin

Bitcoin library for microcontrollers. Supports Arduino, mbed, bare metal.
https://micro-bitcoin.github.io/
MIT License
168 stars 33 forks source link

Parse PSBT fail #18

Closed motorina0 closed 2 years ago

motorina0 commented 2 years ago

Sometimes the PSBT parsing fails. I have managed to reproduce the issue only with Testnet transactions, it seams to work fine on Mainnet

I have this simple logic for parsing a base64 PSBT:

PSBT parseBase64Psbt(String psbtBase64) {
  PSBT psbt;
  psbt.parseBase64(psbtBase64);
  return psbt;
}

Below there are two sample PSBTs, the only difference between them is the UTXO they spend (and the non_witness_utxo field).

Parse OK:

cHNidP8BAFUCAAAAAZb0NKDeWDjDLUUJ9C1M0ekNy8YtQFGYeriEROSiOYkDAQAAAAD/////AZYmAAAAAAAAGXapFE+F3U9Bb3lQqswKVxk0CST1c7YUiKwAAAAAAAEA3gIAAAAAAQFscPXX0vlqTTiI7X2a1xQveCT4YASg3vLBbR/zI3AFqAAAAAAA/v///wIHULkKAAAAABYAFMBhT3nZo0LqERUgtLTU1UTzsDsnECcAAAAAAAAWABTAbhxJCek7YcYuLEM44JvUnB0A4QJHMEQCICY3o/JKVbrMbj1bnkx+0dW3awSjw32OvL/6Mx4DbWcEAiBm4e27soJRDfzvwuse0Evx6zAP6SWIUMBDpUzBJ/3MbAEhAvN07zGeFZbvIOlV51Ha4QMuUsQuTgJx7xyfX0jP1whkeFEjACIGAp1KUdkEqXtXPugZp1Nq93IG5pO6M7F/PU/JpUulQldVGHgQk1lUAACAAQAAgAAAAIAAAAAAAQAAAAAA

Parse NOK:

cHNidP8BAFUCAAAAAdf99EBDuOfB6EQX3sXC2Rx2c1TBUhfuyjrcBCpQAGHhAQAAAAD/////AYvIGgAAAAAAGXapFE+F3U9Bb3lQqswKVxk0CST1c7YUiKwAAAAAAAEA3gIAAAAAAQFETF8DJsm6wbdvnII5lfTT5+/cPFx3evGYX2N1gsyTGQEAAAAA/v///wKJG38cAgAAABYAFJSDYagzwunqwaUB58AjdBXOVhMKBckaAAAAAAAWABRW3uMg4Freeu8NqlopVhthzRhscAJHMEQCIGTYq9crv+BSQTPPajLxjogfRYozLn9xdbK6ETpRCGYDAiAK4uHFXAgJgtU7wfaFfntodPy4ybODFmY61rOfPrFW+gEhAqOBlXPZrobcH/dgepaOMbpSJYaZVSTbuDUeS6Q7bJqlXUkjACIGA6v5xv9rqGJiUOBI7NG+LsPxkmcWzNlhcy1K1Ap1oAUqGHgQk1lUAACAAQAAgAAAAIAAAAAAAgAAAAAA
stepansnigirev commented 2 years ago

Thanks! I'll look into that over the weekend.

What worries me about these transactions:

I'd suggest using witness_utxo when you are preparing a segwit transaction, omitting non_witness_utxo as it's required only for legacy transactions. PSBT will be also much smaller in this case.

motorina0 commented 2 years ago

not sure if you'll be able to sign this tx as witness_utxo is not provided, I need to double-check the code.

  • it is possible, I did sign a p2wpkh

I'd suggest using witness_utxo when you are preparing a segwit transaction

  • I will try this
motorina0 commented 2 years ago

I'd suggest using witness_utxo when you are preparing a segwit transaction,

  • the issue reproduces also when spending a non-segwit UTXO (p2pkh):
    cHNidP8BAHECAAAAAcCUt8KE/D0nm+GKT0KCkmGw4tE/raDvZ5ohtpmw/R/0AAAAAAD/////AtIEAAAAAAAAFgAUBygY8xLaUKwxTC433JZgWy68siOBIQAAAAAAABYAFFonoWnO1Lu/ARs8GRK4Hm7iV6PzAAAAAAABAOECAAAAAAEBrKHU59Ql5z9U9OrT/vezsW6p0zrq/H/e8D5wWTb0di8AAAAAAP7///8CECcAAAAAAAAZdqkUJhr0QZB180z3REQywqZhK26T4fOIrM/AJgAAAAAAFgAU/SWEMxW5Z01If9kcAnLp/bwZIdsCRzBEAiBAf8nXOVYQwEoNEvnPY1JtmRFRL5K5YpC/pUdaw14FrwIgaP27OUyV5ufX4VML+uzfklF48mkCf7i6D3/4DPOhdDoBIQLHvHqtEUjR4jJ4z+RqVGgIhvEE08kJyf0H2PRUxUcZUiZSIwAiBgIYl+QB5cHV6ahtrKXoXdMQqYIbwwEil0hyqwv5bPBEjxh/jg/pLAAAgAEAAIAAAACAAAAAAAEAAAAAIgIDbSKhCgivNNcoT4rF61UIW5tauSZClR2UIiLs5auGUW4Yf44P6VQAAIABAACAAAAAgAEAAAABAAAAAAA=
motorina0 commented 2 years ago

How can one debug this code? I would like to give it a try but I don't know where to start.

stepansnigirev commented 2 years ago

The strange thing is that I tried parsing all these transaction on a PC (using cpp example) and they all worked fine. How do you check that transaction parsing failed? using if(psbt){...} case?

One useful debug tip is to check value of ubtc_errno that I introduced some time ago - it is an error code that is assigned to different values depending on what went wrong. PSBT error codes are defined in PSBT.cpp. Now thinking about that it probably makes sense to move them to the header... They are not assigned everywhere, but better than nothing.

I'll try running it on the board as well, just need to get home and find TTGo. Let's see if it's a hardware-related issue.

motorina0 commented 2 years ago

How do you check that transaction parsing failed? using if(psbt){...} case?

  • yes
    PSBT psbt = parseBase64Psbt(psbtBase64);
    if (!psbt) {

    Later Edit:

  • value for ubtc_errno is 2 (UBTC_ERR_PSBT_SCOPE)
stepansnigirev commented 2 years ago

I was able to reproduce the error on ESP32, gonna try to figure out what's wrong. Looks like it's failing parsing non-witness utxo in the input.

stepansnigirev commented 2 years ago

Fixed in https://github.com/micro-bitcoin/uBitcoin/commit/9309bbcc35e3d07030606c94ee6c1f3c3f6dd220 Please try with the current master and let me know if it works.

64-bit math on 32-bit systems is tricky, so I slightly changed the way how we parse txout amount - this was causing tx hash validation errors. TxOut amounts were parsed incorrectly if they are larger than max 32-bit value. You had large prevouts in the transactions and that revealed the bug =) I'll add this case into the test suite and think how we can run tests on the hardware.

motorina0 commented 2 years ago

Fix confirmed! It worked 🥳 Sample Testnet Tx spending a high amount.