Upgrade or replace `bitcoind` in integration tests

[spacebear21]

Moved from https://github.com/payjoin/rust-payjoin/pull/367#issuecomment-2417165989

Currently bitcoind is on 0.21 which doesn't support bech32m addresses and is preventing us from using taproot in integration tests.

My attempts to upgrade it have failed so far (bitcoind 0_22 fails because it creates legacy wallets by default, which don't support bech32m. bitcoind 0_23 and above fail because of some deadlock between tests running in parallel).

It looks like bitcoind might be on its way out so if upgrading doesn't work perhaps we should consider replacing this dependency with something else.

[DanGould]

bitcoind-json-rpc-regtest appears to be the favored upgrade path in the rust-bitcoin issue linked in OP

[DanGould]

some functions we use from Client are missing. These are the ones we used and what's already implemented in bitcoind-json-rpc-regtest.

• [x] generate_to_address
• [x] get_new_address
• [x] get_balances
• [ ] get_address_info
• [ ] list_unspent
• [x] send_raw_transaction
• [ ] wallet_process_psbt
• [ ] finalize_psbt
• [ ] wallet_create_funded_psbt

[spacebear21]

bitcoind-json-rpc-regtest has been archived and development is now happening at https://github.com/rust-bitcoin/corepc. The corresponding crate is corepc-node.

[spacebear21]

I made a local fork of corepc that attempts to implement the missing functions for the v28 client version. I had partial success in writing those and getting it to compile and pushed my progress here (resorted to many hacks and shortcuts so it's not pretty...). I also have a working branch that modifies rust-payjoin to use this local corepc instead of bitcoind

However, some problems remain:

• When running all tests they just fail immediately due to a timeout error. Debug prints seem to indicate that tests hang on the "generating coins" initialization step and minreq kills the hung requests.
  • I ran into a similar issue when I attempted to upgrade bitcoind and mentioned it in the top comment. Perhaps they are related:

    bitcoind 0_23 and above fail because of some deadlock between tests running in parallel

• I can run tests one at a time e.g. cargo test v1_to_v1 but there are various incompatibilities that I "fixed" in my corepc branch with ugly hacks or simply commenting out unneeded fields.
  • walletcreatefundedpsbt expects amounts expressed in BTC but corepc passes satoshis.
  • Serialization errors like PSBTs not converted to string when calling PSBT RPCs.
  • Deserialization errors from RPC results like a required is_compressed field that is in fact optional in the RPC response.

Overall it seems corepc is not yet mature enough for us to make the switch at this time.

[spacebear21]

@nothingmuch mentioned a potential workaround for upgrading bitcoind which would involve using a Nix flake to initialize bitcoind and skipping the auto-download, calling the Nix binary directly.

[arminsabouri]

Currently bitcoind is on 0.21 which doesn't support bech32m addresses and is preventing us from using taproot in integration tests.

I set the BITCOIND_EXE to a locally compiled bitcoind bin. Generating bech32m addresses worked fine however the v1-v1 taproot test failed for a different reason.

thread 'integration::v1::v1_to_v1_taproot' panicked at payjoin/tests/integration.rs:126:13:
assertion `left == right` failed
  left: 4899999811 SAT
 right: 4899999812 SAT

Side note: I believe the bitcoind crate will skip autodownload if it detects bitcoind in the PATH.
Instead of ignoring certain tests we could enable them based on the version of bitcoind

[spacebear21]

Generating bech32m addresses worked fine however the v1-v1 taproot test failed for a different reason.

thread 'integration::v1::v1_to_v1_taproot' panicked at payjoin/tests/integration.rs:126:13:
assertion `left == right` failed
  left: 4899999811 SAT
 right: 4899999812 SAT

I wrote the v1-v1 taproot test but I guess I never confirmed that it actually works. Exact fee calculations are tricky to get right in tests. My first guess is that the value returned by predicted_tx_weight() in the test doesn't match the one returned by InputPair::expected_input_weight for taproot inputs.

[arminsabouri]

first guess is that the value returned by predicted_tx_weight() in the test doesn't match the one returned by InputPair::expected_input_weight for taproot inputs

Correct. When I pulled network_fees from the psbt directly the test passed

[spacebear21]

Makes sense, ideally we'd just use network_fees and drop the manual fee calculations in tests. IIRC that doesn't work for the non-taproot tests because ECSDA signatures lengths vary. expected_input_weight returns the worst-case weight so we can make conservative fee estimations.

If you push a working branch with your bitcoind changes I can take a look at why predicted_tx_weight isn't matching the expected weight.

[DanGould]

Noting this isn't really so much tech debt as it is maintenance. When the dependency was taken on it was state of the art, and the state of the art changed.

Not really tech debt to pay back, but regular maintenance

[arminsabouri]

Makes sense, ideally we'd just use network_fees and drop the manual fee calculations in tests. IIRC that doesn't work for the non-taproot tests because ECSDA signatures lengths vary. expected_input_weight returns the worst-case weight so we can make conservative fee estimations.

If you push a working branch with your bitcoind changes I can take a look at why predicted_tx_weight isn't matching the expected weight.

I can put it up but there isn't much code changed. To reproduce you just need to run latest bitcoin core locally and set BITCOIN_EXE envar

[spacebear21]

I replicated the taproot error with a local bitcoind_exe.

thread 'integration::v1::v1_to_v1_taproot' panicked at payjoin/tests/integration.rs:126:13:
assertion `left == right` failed
  left: 4899999811 SAT
 right: 4899999812 SAT

This is strange... the payjoin input calculations actually match the expected values. The reason it's off-by-one is because the original PSBT is paying 1 sat more than we should expect 🤔 I added some debug prints:

            let psbt = build_original_psbt(&sender, &uri)?;
            dbg!(psbt.clone().extract_tx_unchecked_fee_rate().weight());
            dbg!(psbt.fee().unwrap());

[payjoin/tests/integration.rs:90:13] psbt.clone().extract_tx_unchecked_fee_rate().weight() = Weight(
    520,
)
[payjoin/tests/integration.rs:91:13] psbt.fee().unwrap() = 131 SAT

520 WU is exactly 130 vB, and we explicitly specify a feerate of DEFAULT_MIN_RELAY_TX_FEE which is 1 sat/vB, so I would expect the original PSBT fee to be 130 SAT...

[spacebear21]

I replicated this with bitcoin-cli alone:

// specify a witness_v1_taproot input for funding
❯ bitcoin-cli walletcreatefundedpsbt "[{\"txid\":\"11c016e8dafa55e6e4bd7d63305adda039ef73dec4ab3372901460fb06e1c15b\",\"vout\":0}]" "[{\"bcrt1qh23xwxs0wgf8en5a3ggjlka7j3hxgf5k9zzf07\":\"0.1\"}]" 0 "{\"fee_rate\":1}
"
{
  "psbt": "cHNidP8BAHECAAAAAVvB4Qb7YBSQcjOrxN5z7zmg3VowY3295OZV+troFsARAAAAAAD9////Ap51rCQAAAAAFgAU//MIH/Ov52GhitF3oYbKui2v7RKAlpgAAAAAABYAFLqiZxoPchJ8zp2KES/bvpRuZCaWAAAAAAABASuhDEUlAAAAACJRIF5iZ+CNJF90vVfeeezsUqj
+EWhIgTK8DjZ+OiNqWfarIRYHrlQVC0lAYEyy++eZf61dFqrLsqZYWXN7A5mQiZxn6hkAFUTY31YAAIABAACAAAAAgAAAAAAAAAAAARcgB65UFQtJQGBMsvvnmX+tXRaqy7KmWFlzewOZkImcZ+oAIgICx3VhryJH4wp2oCvFqdYUNSPMsZwXh7rQBwGT3gLHjzMYFUTY31QAAIABAACAA
AAAgAEAAAANAAAAACICAmH/qonCobPMcPU2yRgPlEgzpXraunEKV3h1w6EgLBM1GBVE2N9UAACAAQAAgAAAAIAAAAAAAgAAAAA=",
  "fee": 0.00000131,
  "changepos": 0
}

❯ bitcoin-cli walletprocesspsbt "cHNidP8BAHECAAAAAVvB4Qb7YBSQcjOrxN5z7zmg3VowY3295OZV+troFsARAAAAAAD9////Ap51rCQAAAAAFgAU//MIH/Ov52GhitF3oYbKui2v7RKAlpgAAAAAABYAFLqiZxoPchJ8zp2KES/bvpRuZCaWAAAAAAABASuhDEUlAAAAACJRI
F5iZ+CNJF90vVfeeezsUqj+EWhIgTK8DjZ+OiNqWfarIRYHrlQVC0lAYEyy++eZf61dFqrLsqZYWXN7A5mQiZxn6hkAFUTY31YAAIABAACAAAAAgAAAAAAAAAAAARcgB65UFQtJQGBMsvvnmX+tXRaqy7KmWFlzewOZkImcZ+oAIgICx3VhryJH4wp2oCvFqdYUNSPMsZwXh7rQBwGT3gL
HjzMYFUTY31QAAIABAACAAAAAgAEAAAANAAAAACICAmH/qonCobPMcPU2yRgPlEgzpXraunEKV3h1w6EgLBM1GBVE2N9UAACAAQAAgAAAAIAAAAAAAgAAAAA="
{
  "psbt": "cHNidP8BAHECAAAAAVvB4Qb7YBSQcjOrxN5z7zmg3VowY3295OZV+troFsARAAAAAAD9////Ap51rCQAAAAAFgAU//MIH/Ov52GhitF3oYbKui2v7RKAlpgAAAAAABYAFLqiZxoPchJ8zp2KES/bvpRuZCaWAAAAAAABASuhDEUlAAAAACJRIF5iZ+CNJF90vVfeeezsUqj
+EWhIgTK8DjZ+OiNqWfarAQhCAUD1af502czUuJZe3+pZgUblsvoRUW6pr43UZORmKGc0JExGaFjRLR6YuH+lAH0/WEiLUM3/XJ4cACYQ4g2xcoOkACICAsd1Ya8iR+MKdqArxanWFDUjzLGcF4e60AcBk94Cx48zGBVE2N9UAACAAQAAgAAAAIABAAAADQAAAAAiAgJh/6qJwqGzzHD1N
skYD5RIM6V62rpxCld4dcOhICwTNRgVRNjfVAAAgAEAAIAAAACAAAAAAAIAAAAA",
  "complete": true,
  "hex": "020000000001015bc1e106fb6014907233abc4de73ef39a0dd5a30637dbde4e655fadae816c0110000000000fdffffff029e75ac2400000000160014fff3081ff3afe761a18ad177a186caba2dafed128096980000000000160014baa2671a0f72127cce9d8a
112fdbbe946e6426960140f569fe74d9ccd4b8965edfea598146e5b2fa11516ea9af8dd464e466286734244c466858d12d1e98b87fa5007d3f58488b50cdff5c9e1c002610e20db17283a400000000"
}

❯ bitcoin-cli decoderawtransaction "020000000001015bc1e106fb6014907233abc4de73ef39a0dd5a30637dbde4e655fadae816c0110000000000fdffffff029e75ac2400000000160014fff3081ff3afe761a18ad177a186caba2dafed128096980000000000160014baa2671a0f72127cce9d8a112fdbbe946e6426960140f569fe74d9ccd4b8965edfea598146e5b2fa11516ea9af8dd464e466286734244c466858d12d1e98b87fa5007d3f58488b50cdff5c9e1c002610e20db17283a400000000"
{
  "txid": "dc09a5b7cdffd2307157e0685d54b47b5b4006a826689fe9cf47f15c3f0b4400",
  "hash": "3bee98c006f7be5e804a35d755e575672bec14356213dc5f13b1c2e397f5560c",
  "version": 2,
  "size": 181,
  "vsize": 130,
  "weight": 520,
  "locktime": 0,
  "vin": [
    {
      "txid": "11c016e8dafa55e6e4bd7d63305adda039ef73dec4ab3372901460fb06e1c15b",
      "vout": 0,
      "scriptSig": {
        "asm": "",
        "hex": ""
      },
      "txinwitness": [
        "f569fe74d9ccd4b8965edfea598146e5b2fa11516ea9af8dd464e466286734244c466858d12d1e98b87fa5007d3f58488b50cdff5c9e1c002610e20db17283a4"
      ],
      "sequence": 4294967293
    }
  ],
  "vout": [
    {
      "value": 6.15282078,
      "n": 0,
      "scriptPubKey": {
        "asm": "0 fff3081ff3afe761a18ad177a186caba2dafed12",
        "desc": "addr(bcrt1qlless8ln4lnkrgv269m6rpk2hgk6lmgjvsfzpn)#hx6tljgx",
        "hex": "0014fff3081ff3afe761a18ad177a186caba2dafed12",
        "address": "bcrt1qlless8ln4lnkrgv269m6rpk2hgk6lmgjvsfzpn",
        "type": "witness_v0_keyhash"
      }
    },
    {
      "value": 0.10000000,
      "n": 1,
      "scriptPubKey": {
        "asm": "0 baa2671a0f72127cce9d8a112fdbbe946e642696",
        "desc": "addr(bcrt1qh23xwxs0wgf8en5a3ggjlka7j3hxgf5k9zzf07)#smy3ehgq",
        "hex": "0014baa2671a0f72127cce9d8a112fdbbe946e642696",
        "address": "bcrt1qh23xwxs0wgf8en5a3ggjlka7j3hxgf5k9zzf07",
        "type": "witness_v0_keyhash"
      }
    }
  ]
}

The fee is 131 sats despite the vsize being 130vB and the fee_rate of 1sat/vB.

[DanGould]

Seems like corepc-node and bitcoind-async-client would let us close this upgrade, test taproot, and close this issue for 1.0 stability.

This is not really an API issue but it does plug a small hole in our tests.

[arminsabouri]

closed by #1041