search

BP-WG / bp-wallet

Modern, lightweight & standard-compliant bitcoin wallet runtime & cli without rust-bitcoin dependencies
Apache License 2.0
17 stars 12 forks source link

non-mandatory-script-verify-flag error when broadcasting tx with electrum #62

Closed zoedberg closed 2 months ago

zoedberg commented 3 months ago

The same error reported in https://github.com/BP-WG/descriptor-wallet/issues/83 happens with bp-wallet too.

dr-orlovsky commented 3 months ago

My investigation shows that this is a clear signature failure: Bitcoin Core regtest doesn't accept transactions as valid, indicating invalidity of signatures.

This happens ONLY and for ALL transactions with 2 outputs - but not for transactions with 1 output. It happens both for SegWit and Taproot sigs, since they has outputs the same way (we do not test pre-segwit).

This happens both on descriptor-wallet and bp-wallet, for the same set of transactions (with 2 outputs). Since signing code of bp-wallet was based on rust-bitcoin (used in descriptor-wallet) - but not the old rust-bitcoin but current master of it, I assume it is a pending bug in rust-bitcoin. I just can't track it: I have compared the implementation to the BIPs, there is nothing wrong.

Moving this bug to bp-consensus since the bug is there.

dr-orlovsky commented 2 months ago

After a very long debugging with @zoedberg we were able to figure out that the issue was related to the indeterminism in float->int conversion in https://github.com/BP-WG/bp-wallet/blob/dbfb8ba551240a788033f157feed5a6a51351219/src/indexers/electrum.rs#L222