Skip to content

Commit

Permalink
Merge bitcoin#27325: test: various converttopsbt check cleanups in …
Browse files Browse the repository at this point in the history
…rpc_psbt.py

afc2dd5 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 afc2dd5
  achow101:
    ACK afc2dd5

Tree-SHA512: 467efefdb3f61efdb79145044b90fc8dc2f0c425f078117a99112b0074e7d4a32c34e464f665fbf8de70d06caaa5d4ad5908c1d75d2e7607eccb0837480afab3
  • Loading branch information
achow101 committed May 4, 2023
2 parents aebcd18 + afc2dd5 commit 30bf70c
Showing 1 changed file with 7 additions and 9 deletions.
16 changes: 7 additions & 9 deletions test/functional/rpc_psbt.py
Original file line number Diff line number Diff line change
Expand Up @@ -397,17 +397,15 @@ def run_test(self):
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()
Expand Down

0 comments on commit 30bf70c

Please sign in to comment.