Skip to content

Commit

Permalink
Merge bitcoin#30353: test: fix inconsistency in fundrawtransaction we…
Browse files Browse the repository at this point in the history
…ight limits test

00b8e26 test: fix inconsistency in fundrawtransaction weight limits test (furszy)

Pull request description:

  Fix bitcoin#30309 (comment) inconsistency.

  Currently, the test is passing due to a mistake in the test inputs
  selection process. We are selecting the parent transaction change
  output as one of the inputs of the transaction to fund, which
  helps to surpass the target amount when it shouldn't due to the
  fee reduction.

  The failure arises when the test behaves as intended by its coder;
  that is, when it does not select the change output. In this case,
  the pre-selected inputs aren't enough to cover the target amount.

  Fix this by excluding the parent transaction's change output from
  the inputs selection and including an extra input to cover the tx
  fee.

  The CI failure can be replicated with the following patch in master:

  ```diff
  diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py
  --- a/test/functional/wallet_fundrawtransaction.py(revision 9b480f7)
  +++ b/test/functional/wallet_fundrawtransaction.py(date 1720652934739)
  @@ -1322,7 +1322,7 @@
           outputs = []
           for _ in range(1472):
               outputs.append({wallet.getnewaddress(address_type="legacy"): 0.1})
  -        txid = self.nodes[0].send(outputs=outputs)["txid"]
  +        txid = self.nodes[0].send(outputs=outputs, change_position=0)["txid"]
           self.generate(self.nodes[0], 1)

           # 272 WU per input (273 when high-s); picking 1471 inputs will exceed the max standard tx weight.
  @@ -1330,7 +1330,7 @@

           # 1) Try to fund transaction only using the preset inputs
           input_weights = []
  -        for i in range(1471):
  +        for i in range(1, 1472):  # skip first output as it is the parent tx change output
               input_weights.append({"txid": txid, "vout": i, "weight": 273})
           assert_raises_rpc_error(-4, "Transaction too large", wallet.fundrawtransaction, hexstring=rawtx, input_weights=input_weights)
  ```

ACKs for top commit:
  achow101:
    ACK 00b8e26
  ismaelsadeeq:
    Code review and Tested ACK 00b8e26

Tree-SHA512: 5ef792961b7fad4999fc30aa03366432103ddf672ca5cbb366c9eab4c2e46d5ae1ab0c073dfc4fbb2b4e63203653bc0e54463c731c5f8655140207ba5f8e542e
  • Loading branch information
achow101 committed Jul 11, 2024
2 parents 00feabf + 00b8e26 commit 33af14e
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions test/functional/wallet_fundrawtransaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -1322,15 +1322,15 @@ def test_weight_limits(self):
outputs = []
for _ in range(1472):
outputs.append({wallet.getnewaddress(address_type="legacy"): 0.1})
txid = self.nodes[0].send(outputs=outputs)["txid"]
txid = self.nodes[0].send(outputs=outputs, change_position=0)["txid"]
self.generate(self.nodes[0], 1)

# 272 WU per input (273 when high-s); picking 1471 inputs will exceed the max standard tx weight.
rawtx = wallet.createrawtransaction([], [{wallet.getnewaddress(): 0.1 * 1471}])

# 1) Try to fund transaction only using the preset inputs
# 1) Try to fund transaction only using the preset inputs (pick all 1472 inputs to cover the fee)
input_weights = []
for i in range(1471):
for i in range(1, 1473): # skip first output as it is the parent tx change output
input_weights.append({"txid": txid, "vout": i, "weight": 273})
assert_raises_rpc_error(-4, "Transaction too large", wallet.fundrawtransaction, hexstring=rawtx, input_weights=input_weights)

Expand Down

0 comments on commit 33af14e

Please sign in to comment.