Skip to content

Conversation

@furszy
Copy link
Member

@furszy furszy commented Aug 7, 2023

Coming from #28139 (comment).

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
Copy link
Contributor

DrahtBot commented Aug 7, 2023

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 DrahtBot added the Tests label Aug 7, 2023
@maflcko
Copy link
Member

maflcko commented Aug 7, 2023

concept ACK

Same as we do with the nodes default wallets.
No test case on this file is meant to exercise fee estimation.
@furszy furszy force-pushed the 2023_test_locked_wallet_hardcode_fee branch from 86aa6ca to 5364dd8 Compare August 10, 2023 12:54
@furszy
Copy link
Member Author

furszy commented Aug 10, 2023

Updated per feedback. Thanks theStack.

Tiny change. Instead of calculating the fee manually, the code now uses
the get_fee() util function.

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 5364dd8

@fanquake fanquake merged commit aadaa56 into bitcoin:master Aug 14, 2023
@furszy furszy deleted the 2023_test_locked_wallet_hardcode_fee branch August 14, 2023 15:29
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 15, 2023
5364dd8 test: locked_wallet, skip default fee estimation (furszy)

Pull request description:

  Coming from bitcoin#28139 (comment).

  No test case in this file is meant to exercise fee estimation. All default wallets have a
  custom tx fee set [here](https://github.com/bitcoin/bitcoin/blob/b7138252ace6d21476964774e094ed1143cd7a1c/test/functional/wallet_fundrawtransaction.py#L100). The only one missing is the one created for `locked_wallet`.

ACKs for top commit:
  theStack:
    ACK 5364dd8

Tree-SHA512: 514c02708081d18330d759d10e306cee16c6350de243c68f0973777d2582f5d81968a237393c1f59aba245297e03f3f98d3ae5249a042469d0d016255f568719
@bitcoin bitcoin locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants