diff options
author | Andrew Chow <github@achow101.com> | 2023-05-04 11:01:55 -0400 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2023-05-04 11:08:16 -0400 |
commit | 30bf70c8b60f070c47ce9b51b5d2520c386d3559 (patch) | |
tree | 9b461d9524055cbdff1fda8c524be9d1eb894641 | |
parent | aebcd18c654a1706954a9e2c9cbfe97dfe531357 (diff) | |
parent | afc2dd54848fa70ed408ae259420ff8648f21efc (diff) |
Merge bitcoin/bitcoin#27325: test: various `converttopsbt` check cleanups in rpc_psbt.py
afc2dd54848fa70ed408ae259420ff8648f21efc test: various `converttopsbt` check cleanups in rpc_psbt.py (Sebastian Falbesoner)
Pull request description:
In the functional test rpc_psbt.py, some comments around the `converttopsbt` RPC checks are wrong or outdated and can be removed:
> _Error could be either "TX decode failed" (segwit inputs causes
> parsing to fail) or "Inputs must not have scriptSigs and
> scriptWitnesses"_
Decoding a valid TX with at least one input always succeeds with the [heuristic](https://github.com/bitcoin/bitcoin/blob/e352f5ab6b60ec1cc549997275e945238508cdee/src/core_read.cpp#L126), i.e. this comment is not right and we can assert for the error string "Inputs must not have scriptSigs and scriptWitnesses" on the calls below.
> _We must set iswitness=True because the serialized transaction has
> inputs and is therefore a witness transaction_
This is also unneeded (and confusing, w.r.t. "is therefore a witness transaction"?), for a TX with one input there is no need to set the `iswitness` parameter. For sake of completeness, we still keep one variant where iswitness is explicitly set to true.
Lastly, there is a superflous `converttopsbt` call on the raw tx which is the same as just [about ~10 lines above](https://github.com/bitcoin/bitcoin/blob/master/test/functional/rpc_psbt.py#L393-L397), so it can be removed.
ACKs for top commit:
ismaelsadeeq:
tested ACK afc2dd54848fa70ed408ae259420ff8648f21efc
achow101:
ACK afc2dd54848fa70ed408ae259420ff8648f21efc
Tree-SHA512: 467efefdb3f61efdb79145044b90fc8dc2f0c425f078117a99112b0074e7d4a32c34e464f665fbf8de70d06caaa5d4ad5908c1d75d2e7607eccb0837480afab3
-rwxr-xr-x | test/functional/rpc_psbt.py | 16 |
1 files changed, 7 insertions, 9 deletions
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index ef773463d8..49cdfd0e66 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -397,17 +397,15 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].decodepsbt(new_psbt) # Make sure that a non-psbt with signatures cannot be converted - # Error could be either "TX decode failed" (segwit inputs causes parsing to fail) or "Inputs must not have scriptSigs and scriptWitnesses" - # We must set iswitness=True because the serialized transaction has inputs and is therefore a witness transaction signedtx = self.nodes[0].signrawtransactionwithwallet(rawtx['hex']) - assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, hexstring=signedtx['hex'], iswitness=True) - assert_raises_rpc_error(-22, "", self.nodes[0].converttopsbt, hexstring=signedtx['hex'], permitsigdata=False, iswitness=True) + assert_raises_rpc_error(-22, "Inputs must not have scriptSigs and scriptWitnesses", + self.nodes[0].converttopsbt, hexstring=signedtx['hex']) # permitsigdata=False by default + assert_raises_rpc_error(-22, "Inputs must not have scriptSigs and scriptWitnesses", + self.nodes[0].converttopsbt, hexstring=signedtx['hex'], permitsigdata=False) + assert_raises_rpc_error(-22, "Inputs must not have scriptSigs and scriptWitnesses", + self.nodes[0].converttopsbt, hexstring=signedtx['hex'], permitsigdata=False, iswitness=True) # Unless we allow it to convert and strip signatures - self.nodes[0].converttopsbt(signedtx['hex'], True) - - # Explicitly allow converting non-empty txs - new_psbt = self.nodes[0].converttopsbt(rawtx['hex']) - self.nodes[0].decodepsbt(new_psbt) + self.nodes[0].converttopsbt(hexstring=signedtx['hex'], permitsigdata=True) # Create outputs to nodes 1 and 2 node1_addr = self.nodes[1].getnewaddress() |