test: ensure output is large enough to pay for its fees #29283

stickies-v commented at 2:22 PM on January 19, 2024: contributor

Fixes a (rare) intermittency issue in wallet_import_rescan.py

Since we use subtract_fee_from_outputs=[0] in the send command, the output amount must at least be as large as the fee we're paying.

Example in CI: https://api.cirrus-ci.com/v1/task/6107972259020800/logs/ci.log

2024-01-18T22:16:12.383000Z TestFramework (INFO): Test that the mempool is rescanned as well if the rescan parameter is set to true
2024-01-18T22:16:20.187000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
    self.run_test()
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_import_rescan.py", line 292, in run_test
    child = self.nodes[1].send(
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 50, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 129, in __call__
    raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: The transaction amount is too small to pay the fee (-4)

Can be reproduced locally by forcing usage of the lowest possible value produced by get_rand_amount() (thanks furszy):

diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py
index 7f01d23941..925849d5c0 100755
--- a/test/functional/wallet_import_rescan.py
+++ b/test/functional/wallet_import_rescan.py
@@ -270,7 +270,7 @@ class ImportRescanTest(BitcoinTestFramework):
                 address_type=variant.address_type.value,
             ))
             variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
-            variant.initial_amount = get_rand_amount() * 2
+            variant.initial_amount = Decimal(str(round(AMOUNT_DUST, 8))) * 2
             variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
             variant.confirmation_height = 0
             variant.timestamp = timestamp

</details>

test: ensure output is large enough to pay for its fees

Fixes a (rare) intermittency issue in wallet_import_rescan.

Since we use `subtract_fee_from_outputs=[0]` in the `send` command,
the output amount must at least be as large as the fee we're paying.

3bfc5bd36e

DrahtBot commented at 2:22 PM on January 19, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	glozow, furszy, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Tests on Jan 19, 2024

glozow commented at 2:32 PM on January 19, 2024: member

utACK 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67, didn't experience this issue but in theory a minimum of AMOUNT_DUST could be too low to pay the fees

furszy commented at 3:03 PM on January 19, 2024: member

Can be replicated locally by changing line 273 to use the get_rand_amount() floor.

variant.initial_amount = Decimal(str(round(AMOUNT_DUST, 8))) * 2

in test/functional/wallet_import_rescan.py:274 in 3bfc5bd36e

 270 | @@ -270,7 +271,9 @@ def run_test(self):
 271 |                  address_type=variant.address_type.value,
 272 |              ))
 273 |              variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
 274 | -            variant.initial_amount = get_rand_amount() * 2
 275 | +            # Ensure output is large enough to pay for fees: conservatively assuming txsize of

furszy commented at 2:09 PM on January 25, 2024:

Maybe:

            # Ensure output is large enough to pay for its fees. Following transactions will contain one input and one unspendable output. Conservatively assuming...

Also, just as an extra note (no need to do it): could calculate the floor in a more dynamic manner if you want. The input type is known here, and the future tx is always fixed to be a 1 input, 1 bech32 output tx.

stickies-v commented at 2:23 PM on January 25, 2024:

Thx, will consider if I need to retouch!

furszy commented at 2:09 PM on January 25, 2024: member

utACK 3bfc5bd36

achow101 commented at 11:30 PM on January 26, 2024: member

ACK 3bfc5bd36e38ca07306a41a3f56ea102ffe02b67

achow101 merged this on Jan 26, 2024

achow101 closed this on Jan 26, 2024

stickies-v deleted the branch on Jan 26, 2024

in test/functional/wallet_import_rescan.py:277 in 3bfc5bd36e

 270 | @@ -270,7 +271,9 @@ def run_test(self):
 271 |                  address_type=variant.address_type.value,
 272 |              ))
 273 |              variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
 274 | -            variant.initial_amount = get_rand_amount() * 2
 275 | +            # Ensure output is large enough to pay for fees: conservatively assuming txsize of
 276 | +            # 500 vbytes and feerate of 20 sats/vbytes
 277 | +            variant.initial_amount = max(get_rand_amount(), (500 * 20 / COIN) + AMOUNT_DUST)
 278 |              variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)

maflcko commented at 12:16 PM on January 28, 2024:

 test  2024-01-27T00:59:20.405000Z TestFramework (ERROR): JSONRPC error 
                                   Traceback (most recent call last):
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
                                       self.run_test()
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/wallet_import_rescan.py", line 277, in run_test
                                       variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/coverage.py", line 50, in __call__
                                       return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 129, in __call__
                                       raise JSONRPCException(response['error'], status)
                                   test_framework.authproxy.JSONRPCException: Invalid amount (-3)

https://cirrus-ci.com/task/5562947183837184?logs=ci#L2559

glozow commented at 9:17 AM on January 29, 2024:

Does it need to be cast to an int maybe?

stickies-v commented at 10:36 AM on January 29, 2024:

Thanks for reporting this @maflcko, need to round the minimum value to 8 decimals to fix this. Opening a pull now.

stickies-v commented at 10:46 AM on January 29, 2024:

https://github.com/bitcoin/bitcoin/pull/29343/

fanquake referenced this in commit 11b436a66a on Jan 31, 2024

bitcoin locked this on Jan 28, 2025