test: add P2SH-P2WSH and P2SH 2of3 multisig integration tests - #49
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds regression tests to ensure cmd_spend can create PSBTs that spend multisig “dust” UTXOs across multiple script/address types, and that the PSBTs can be finalized via multiple wallet signatures.
Changes:
- Add a 2-of-2 P2SH-P2wSH multisig dust spend test.
- Add a 2-of-3 P2SH-P2wSH multisig dust spend test.
- Add a 2-of-3 legacy P2SH multisig dust spend test.
| assert!(result.is_some(), "expected a psbt to be created"); | ||
| let psbt = result.unwrap(); |
| let ctx = TestContext::new(); | ||
|
|
||
| let (addr, desc) = ctx.env.create_multisig( | ||
| &[&ctx.wallet1_name, &ctx.wallet2_name], | ||
| 2, | ||
| &AddressType::P2shSegwit, | ||
| ); | ||
|
|
||
| cmd_add(&ctx.secp, &ctx.db, ctx.network, &ctx.rpc_client, desc, 0); | ||
| ctx.env.send_to_address(&addr, Amount::from_sat(555)); | ||
| ctx.env.mine_blocks(1); | ||
|
|
||
| let result = cmd_spend( | ||
| &ctx.db, | ||
| ctx.network, | ||
| &ctx.rpc_client, | ||
| Amount::from_sat(600), | ||
| addr, | ||
| false, | ||
| ); |
| // 2-of-3: any 2 of the 3 wallets must sign | ||
| let partially_signed = ctx.env.wallet_process_psbt(&ctx.wallet1_name, &psbt); | ||
| let fully_signed = ctx | ||
| .env | ||
| .wallet_process_psbt(&ctx.wallet2_name, &partially_signed); | ||
| broadcast_and_assert(&ctx, fully_signed, 1); |
|
for the 2of3 tests, should I test multiple signer combinations (e.g. wallet1+wallet2 and wallet1+wallet3) or is a single combination enough for now? |
|
A single combination is enough, I don't see any new code paths being triggered by using different signers. |
|
The other copilot suggestions are valid and worth fixing. |
4ac5b98 to
21a23ef
Compare
|
@bubb1es71 |
| let dust = cmd_list( | ||
| &ctx.db, | ||
| ctx.network, | ||
| &ctx.rpc_client, | ||
| Amount::from_sat(dust_sats + 50), | ||
| false, | ||
| ); | ||
| debug!("found dust: {:?}", dust.len()); |
| let (addr, desc) = ctx.env.create_multisig( | ||
| &[&ctx.wallet1_name, &ctx.wallet2_name, &ctx.wallet3_name], | ||
| 2, | ||
| &AddressType::Bech32, | ||
| &AddressType::P2shSegwit, | ||
| ); | ||
|
|
||
| cmd_add(&ctx.secp, &ctx.db, ctx.network, &ctx.rpc_client, desc, 0); | ||
| ctx.env.send_to_address(&addr, Amount::from_sat(555)); | ||
| ctx.env.mine_blocks(1); | ||
|
|
||
| let result = cmd_spend( | ||
| &ctx.db, | ||
| ctx.network, | ||
| &ctx.rpc_client, | ||
| Amount::from_sat(600), | ||
| addr, | ||
| false, | ||
| ); | ||
| let psbt = result.expect("expected a psbt to be created"); | ||
|
|
||
| // 2-of-3: any 2 of the 3 wallets must sign | ||
| let partially_signed = ctx.env.wallet_process_psbt(&ctx.wallet1_name, &psbt); | ||
| let fully_signed = ctx | ||
| .env | ||
| .wallet_process_psbt(&ctx.wallet2_name, &partially_signed); | ||
| broadcast_and_assert(&ctx, fully_signed, 1); |
| &ctx.db, | ||
| ctx.network, | ||
| &ctx.rpc_client, | ||
| Amount::from_sat(dust_sats + 50), |
| &ctx.db, | ||
| ctx.network, | ||
| &ctx.rpc_client, | ||
| Amount::from_sat(dust_sats + 50), |
21a23ef to
c30a8a6
Compare
c30a8a6 to
693440d
Compare
|
Hi let me know when this is ready for a human re-review. |
@bubb1es71 |
closes #22
adds integration tests for
P2SH-P2WSHandP2SH 2of3 multisigdust inputs:-
test_spend_p2sh_2of3_multisig-
test_spend_p2sh_p2wsh_2of2_multisig-
test_spend_p2sh_p2wsh_2of3_multisig