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

stickies-v · 2024-01-19T14:22:30Z

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):

git diff on 5f3a057

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

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.

DrahtBot · 2024-01-19T14:22:33Z

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.

glozow

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

furszy

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

furszy

utACK 3bfc5bd

furszy · 2024-01-25T14:09:33Z

test/functional/wallet_import_rescan.py

            ))
            variant.key = self.nodes[1].dumpprivkey(variant.address["address"])
-            variant.initial_amount = get_rand_amount() * 2
+            # Ensure output is large enough to pay for fees: conservatively assuming txsize of


Maybe:

Suggested change

# Ensure output is large enough to pay for fees: conservatively assuming txsize of

# 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.

Thx, will consider if I need to retouch!

achow101 · 2024-01-26T23:30:32Z

ACK 3bfc5bd

maflcko · 2024-01-28T12:16:26Z

test/functional/wallet_import_rescan.py

+            # Ensure output is large enough to pay for fees: conservatively assuming txsize of
+            # 500 vbytes and feerate of 20 sats/vbytes
+            variant.initial_amount = max(get_rand_amount(), (500 * 20 / COIN) + AMOUNT_DUST)
            variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount)


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

Does it need to be cast to an int maybe?

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

26ad2ae test: fix wallet_import_rescan unrounded minimum amount (stickies-v) Pull request description: Addresses #29283 (comment). Fixes a `JSONRPCException: Invalid amount (-3)` exception by ensuring the amount sent to `sendtoaddress` is rounded to 8 decimals. See https://cirrus-ci.com/task/5562947183837184?logs=ci#L2559 Note: since `round` can also round down, `min_amount` is not _exactly_ guaranteed, but this is not a problem for the current usage. I've added a docstring to highlight this. ACKs for top commit: sr-gi: ACK [26ad2ae](26ad2ae) Tree-SHA512: 82ce16447f30535f17fa73336f7e4f74639e33215a228294b9b8005b8050a760b90a3726de279cce98c7e439f09104172b74072be3a300dbd461bf0c3f54b954

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

3bfc5bd

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.

DrahtBot added the Tests label Jan 19, 2024

stickies-v mentioned this pull request Jan 19, 2024

test: wallet rescan with reorged parent + IsFromMe child in mempool #29179

Merged

glozow reviewed Jan 19, 2024

View reviewed changes

furszy reviewed Jan 19, 2024

View reviewed changes

furszy reviewed Jan 25, 2024

View reviewed changes

achow101 merged commit ff0eac0 into bitcoin:master Jan 26, 2024

stickies-v deleted the 2024-01/wallet-import-rescan-fix-intermittency branch January 26, 2024 23:35

maflcko reviewed Jan 28, 2024

View reviewed changes

stickies-v mentioned this pull request Jan 29, 2024

test: fix wallet_import_rescan unrounded minimum amount #29343

Merged

bitcoin locked and limited conversation to collaborators Jan 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

stickies-v commented Jan 19, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Jan 19, 2024 •

edited

Loading

Uh oh!

glozow left a comment

Uh oh!

furszy left a comment •

edited

Loading

Uh oh!

furszy left a comment

Uh oh!

furszy Jan 25, 2024

Uh oh!

stickies-v Jan 25, 2024

Uh oh!

achow101 commented Jan 26, 2024

Uh oh!

maflcko Jan 28, 2024

Uh oh!

glozow Jan 29, 2024

Uh oh!

stickies-v Jan 29, 2024

Uh oh!

stickies-v Jan 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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

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

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

Uh oh!

Conversation

stickies-v commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Uh oh!

glozow left a comment

Choose a reason for hiding this comment

Uh oh!

furszy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

furszy left a comment

Choose a reason for hiding this comment

Uh oh!

furszy Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

stickies-v Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

achow101 commented Jan 26, 2024

Uh oh!

maflcko Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

glozow Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

stickies-v Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

stickies-v Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

stickies-v commented Jan 19, 2024 •

edited

Loading

DrahtBot commented Jan 19, 2024 •

edited

Loading

furszy left a comment •

edited

Loading