test: locked_wallet, skip default fee estimation #28232

pull furszy wants to merge 1 commits into bitcoin:master from furszy:2023_test_locked_wallet_hardcode_fee changing 1 files +8 −2

furszy commented at 1:49 PM on August 7, 2023: member

Coming from #28139 (review).

No test case in this file is meant to exercise fee estimation. All default wallets have a custom tx fee set here. The only one missing is the one created for locked_wallet.
DrahtBot commented at 1:49 PM on August 7, 2023: contributor


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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK theStack

Concept ACK MarcoFalke

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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #28264 (test: refactor: support sending funds with outpoint result by theStack)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot added the label Tests on Aug 7, 2023
maflcko commented at 2:22 PM on August 7, 2023: member

concept ACK
maflcko approved

Type	Reviewers
ACK	theStack
Concept ACK	MarcoFalke

in test/functional/wallet_fundrawtransaction.py:608 in 86aa6ca862 outdated

 605 | -        value = inputs[0]["amount"] - Decimal("0.00002200")
 606 | +
 607 | +        # Deduce exact fee to produce a changeless transaction
 608 | +        expected_tx_size = 110  # Total tx size: 110 vbytes, p2wpkh -> p2wpkh. Input 68 vbytes + rest of tx is 42 vbytes.
 609 | +        fee_rate = self.min_relay_tx_fee / 1000  # btc/vbytes
 610 | +        value = inputs[0]["amount"] - (fee_rate * expected_tx_size)

theStack commented at 10:44 AM on August 10, 2023:

nit: could use the get_fee helper (from test_framework.util) for not having to bother with manual fee-rate unit coversion and fee calculation:

        value = inputs[0]["amount"] - get_fee(expected_tx_size, self.min_relay_tx_fee)

furszy commented at 12:56 PM on August 10, 2023:

sure, pushed. Thanks. would be nice to have a document somewhere mentioning the existence of these utility functions.

test: locked_wallet, skip default fee estimation

Same as we do with the nodes default wallets.
No test case on this file is meant to exercise fee estimation.

5364dd8666

furszy force-pushed on Aug 10, 2023
furszy commented at 12:58 PM on August 10, 2023: member

Updated per feedback. Thanks theStack.

Tiny change. Instead of calculating the fee manually, the code now uses the get_fee() util function.
theStack approved
theStack commented at 2:10 PM on August 10, 2023: contributor

ACK 5364dd8666ae1afa35536f9b4fc0170eeaf37888
fanquake merged this on Aug 14, 2023
fanquake closed this on Aug 14, 2023
furszy deleted the branch on Aug 14, 2023
sidhujag referenced this in commit 946d885c70 on Aug 15, 2023
bitcoin locked this on Aug 14, 2024

Contributors

Labels

Linked (view graph)

#28139 test: create wallet specific for test_locked_wallet case

test: locked_wallet, skip default fee estimation #28232

Reviews

Conflicts