Skip to content

Conversation

@theStack
Copy link
Contributor

The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads to smaller transaction sizes, needing adaption of some constants in the following test cases:

  • test_dust_to_fee(): adaption of dust calculation (p2wpkh spend estimate of 67 is taken from src/policy/policy.cpp:GetDustThreshold())
  • test_maxtxfee_fails(): lowering -maxtxfee setting to trigger fail

@fanquake fanquake added the Tests label Oct 20, 2019
@fanquake fanquake requested a review from instagibbs October 20, 2019 23:51
@laanwj
Copy link
Member

laanwj commented Oct 21, 2019

ACK 8ca1c1cd008b489735e7b24302375220f40edd38

@instagibbs
Copy link
Member

cursory ACK 8ca1c1c

didn't use pencil and paper to confirm all the numbers, but at least a full explanation was added for the latter magic number.

The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads
to smaller transaction sizes, needing adaption of some constants in the
following test cases:
    - test_dust_to_fee(): adaption of dust calculation
          (p2wpkh spend estimate of 67 is taken from src/policy/policy.cpp:GetDustThreshold())
    - test_maxtxfee_fails(): lowering -maxtxfee setting to trigger fail
@theStack theStack force-pushed the 20191020-test-wallet-bumpfee_adapt_constants_after_default_address_change branch from 8ca1c1c to 8d8e5a7 Compare October 22, 2019 14:41
@luke-jr
Copy link
Member

luke-jr commented Nov 4, 2019

Concept ACK

maflcko pushed a commit that referenced this pull request Nov 4, 2019
…fee tests

8d8e5a7 test: use default address type (bech32) for wallet_bumpfee tests (Sebastian Falbesoner)

Pull request description:

  The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads to smaller transaction sizes, needing adaption of some constants in the following test cases:
  - `test_dust_to_fee()`: adaption of dust calculation (p2wpkh spend estimate of 67 is taken from `src/policy/policy.cpp:GetDustThreshold()`)
  - `test_maxtxfee_fails()`: lowering `-maxtxfee` setting to trigger fail

Top commit has no ACKs.

Tree-SHA512: b4163700d56c11955f811bc5fe6edaf7aec69931d7db741c03b055fb518bb9825c031fb931c513b37a1968085cb8c2f263adf664b357aff8ee42795fd0f88d2d
@maflcko maflcko merged commit 8d8e5a7 into bitcoin:master Nov 4, 2019
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 7, 2019
…et_bumpfee tests

8d8e5a7 test: use default address type (bech32) for wallet_bumpfee tests (Sebastian Falbesoner)

Pull request description:

  The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads to smaller transaction sizes, needing adaption of some constants in the following test cases:
  - `test_dust_to_fee()`: adaption of dust calculation (p2wpkh spend estimate of 67 is taken from `src/policy/policy.cpp:GetDustThreshold()`)
  - `test_maxtxfee_fails()`: lowering `-maxtxfee` setting to trigger fail

Top commit has no ACKs.

Tree-SHA512: b4163700d56c11955f811bc5fe6edaf7aec69931d7db741c03b055fb518bb9825c031fb931c513b37a1968085cb8c2f263adf664b357aff8ee42795fd0f88d2d
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
…et_bumpfee tests

8d8e5a7 test: use default address type (bech32) for wallet_bumpfee tests (Sebastian Falbesoner)

Pull request description:

  The use of native segwit addresses (pure p2wpkh instead of p2sh-p2wpkh) leads to smaller transaction sizes, needing adaption of some constants in the following test cases:
  - `test_dust_to_fee()`: adaption of dust calculation (p2wpkh spend estimate of 67 is taken from `src/policy/policy.cpp:GetDustThreshold()`)
  - `test_maxtxfee_fails()`: lowering `-maxtxfee` setting to trigger fail

Top commit has no ACKs.

Tree-SHA512: b4163700d56c11955f811bc5fe6edaf7aec69931d7db741c03b055fb518bb9825c031fb931c513b37a1968085cb8c2f263adf664b357aff8ee42795fd0f88d2d
@theStack theStack deleted the 20191020-test-wallet-bumpfee_adapt_constants_after_default_address_change branch December 1, 2020 09:57
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
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.

6 participants