(WIP!) Enable BIP-127 support

[Ademan]

DRAFT PR

This change set enables key parts of BIP-127 such that I believe it can be described as properly implementing most of BIP-127. Notably absent is the protobuf based multi-proof serialization. This change set adds:

• The BIP-127 PSBT_IN_POR_COMMITMENT key is added to proof PSBTs. This enables BIP-127 supporting hardware signers to operate properly.
• Use single SHA-256 to hash the reserves commitment as described in BIP-127 (breaks compatibility with proofs generated before this change set)
• Move verification to the Transaction structure, PSBT metadata is not required to verify a BIP-127 proof. Add a compatibility layer to PSBTs which retains API compatibility

This change set also includes changes from my cleanup branch which are not really relevant to discussion.

I'm creating this draft PR now to discuss (and hopefully concept ACK) switching to single SHA-256, and moving validation onto Transaction

In addition I'm seeking feedback on the TxOutSet trait I added. I think it's a useful abstraction, and it can facilitate "point in time" proofs, such as proving ownership of coins which are spent now, as well as out-of-wallet proofs in a seamless way. TxOutSet in particular added quite a few lines of code however.

[ulrichard]

sorry for the late reply. I had a first quick look. But before diving in deeper, I want to get the CI working again.

Is "point in time" proof validation already working? This is a highly desired feature. When I looked at this initially, I had only Electrum as a backend, and I couldn't see a way to support it with the Electrum server API. What kind of backend is required for this to work?

[Ademan]

097c7a9 supports point in time validation using the esplora backend. I added some rudimentary tests and I think it's mostly correct but there's a couple of edge cases I need to think a bit about.

Steve also said he wants bdk tests to use electrsd, which this doesn't (I'm just using the public mempool.space signet endpoint with esplora_client and a few txids I hand picked). Not sure if that's a blocker for merging, but this all should still be considered a WIP at the moment anyway.

I want to add foreign utxo support using electrum and bitcoind rpc as well, I don't believe they can support full point-in-time though. I believe electrum can at least support a maximum block height (by which a user can establish a minimum number of confirmations)

[Ademan]

RE: using a public esplora instance in the tests, I actually had a test failure due to this, so the unreliability actually matters.

[ulrichard]

There are already some regtest tests with electrsd in bdk-reserves. If I remember correctly, we would just have to add the esplora_a33e97e1 feature to be able to test using an esplora backend.

[ulrichard]

I added a point in time test with electrsd / esplora but it fails.
Curious to learn if I misunderstood something, implemented it wrong, or there is a problem left with the implementation.

[Ademan]

yeah WalletAtHeight cant support point in time regardless of backend (it's limited by the bdk database).

You need the txout_set::PointInTimeTxOutSet trait which is implemented for the esplora blocking client, take a look at txout_set::test::test_spent_at_later_height it can be adapted to use electrsd.

i was actually looking at it this morning too (but I need to get to my day job shortly)

[Ademan]

As far as I know this is what's possible:

	bdk::Wallet	electrum blocking	esplora blocking	bitcoin rpc
Current UTXO Set	🟢	🟢	🟢	🟢
UTXOs currently unspent, confirmed before a certain height	🟢	🟢	🟢	🟢
Point in time	🔴	🔴	🟢	🔴

While we're talking about it, I'm trying to come up with a concise term for the second case "UTXOs currently unspent, but confirmed before a certain height"

[Ademan]

Just to be precise, we have 2 (really 3) sets of transaction outputs

1. utxo set at tip utxos_tip
2. utxo set at X block height utxos_X
3. utxo set in the future if all utxos currently in the mempool (but nothing else) are confirmed utxos_mempool

	relevant set	bdk::Wallet	electrum blocking	esplora blocking	bitcoin rpc
Current UTXO Set	utxos_tip	🟢	🟢	🟢	🟢
UTXOs currently unspent, confirmed before a certain height	utxos_tip ∩ utxos_X	🟢	🟢	🟢	🟢
Point in time	utxos_X	🔴	🔴	🟢	🔴
UTXOS currently unspent in the tip block AND the mempool, confirmed before a certain height	utxos_tip ∩ utxos_X ∩ utxos_mempool	🔴 ❔	🟢 ❔	🟢 ❔	🟢 ❔

I believe some (all?) backends can support utxos_tip ∩ utxos_X ∩ utxos_mempool if that's desirable

[ulrichard]

Very cool! I wanted to be able to verify proofs containing UTXOs that were spent after generating the proof since forever. Super glad that this can now be done.
From the table above, for me only the second and third row are really relevant. But it's good to have all the options.

[Ademan]

I added support for electrum utxos_tip ∩ utxos_X queries as well as a test. I actually was told that it might be possible to do full point-in-time for electrum, too, but I think I'm going to add bitcoin rpc support first, and probably wait on point-in-time on electrum. I kind of want to rework the API here, but I also don't want to hold up the PR for that.

[Ademan]

I just added very WIP bitcoincore_rpc support. Notably, the confirmed_by test fails, but I actually suspect there's an issue in either bitcoincore_rpc or bdk's usage of it, you can see that old_balance.confirmed (which comes from bdk::wallet::Wallet) is consistent between esplora and electrum backends, but is conspicuously different with the bitcoincore_rpc backend. I will have to investigate further another day...

Once that's resolved, I think that will be everything I want to do with this PR, and I'll change from a draft to a "real" PR. (I imagine review will reveal some issues though)

$ cargo test -F rpc,use-esplora-blocking,electrum --test txout_set electrum -- --nocapture
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
warning: the following packages contain code that will be rejected by a future version of Rust: rstest v0.11.0, traitobject v0.1.0
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
     Running tests/txout_set.rs (target/debug/deps/txout_set-b53b110cc918c350)

running 1 test
[tests/txout_set.rs:158] old_balance.confirmed = 4999999718
test test_electrum_confirmed_by ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; finished in 21.65s

$ cargo test -F rpc,use-esplora-blocking,electrum --test txout_set esplora -- --nocapture
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
warning: the following packages contain code that will be rejected by a future version of Rust: rstest v0.11.0, traitobject v0.1.0
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
     Running tests/txout_set.rs (target/debug/deps/txout_set-b53b110cc918c350)

running 1 test
[tests/txout_set.rs:83] old_balance.confirmed = 4999999718
test test_esplora_point_in_time ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 2 filtered out; finished in 21.51s

$ cargo test -F rpc,use-esplora-blocking,electrum --test txout_set bitcoincore -- --nocapture
    Finished test [unoptimized + debuginfo] target(s) in 0.06s
warning: the following packages contain code that will be rejected by a future version of Rust: rstest v0.11.0, traitobject v0.1.0
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
     Running tests/txout_set.rs (target/debug/deps/txout_set-b53b110cc918c350)

running 1 test
[tests/txout_set.rs:158] old_balance.confirmed = 49999999718
thread 'test_bitcoincore_rpc_confirmed_by' panicked at 'assertion failed: `(left == right)`
  left: `4999999718`,
 right: `49999999718`', tests/txout_set.rs:158:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test test_bitcoincore_rpc_confirmed_by ... FAILED

failures:

failures:
    test_bitcoincore_rpc_confirmed_by

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2 filtered out; finished in 16.54s

error: test failed, to rerun pass `--test txout_set`