Skip to content

Conversation

theStack
Copy link
Contributor

@theStack theStack commented Dec 9, 2021

The functional test feature_coinstatsindex.py currently fails on master branch, if descriptor wallets are used (argument --descriptors; or if BDB is not compiled, see #23682 (comment)). This is due to the fact that different change output types are used for created transactions (P2WPKH for legacy wallets, P2TR for descriptor wallets; the former doesn't have a ScriptPubKeyMan for bech32m), resulting in different tx sizes and hence also fees. Fix this by explicitely setting the output type via passing both -addresstype=bech32 and -changetype=bech32 as argument. The former would not be needed by now, but makes the test more deterministic and avoids a failure if bech32m becomes the default address type.

Should further pave the way for #23682.

@theStack theStack force-pushed the 202112-test-fix-feature_coinstatsindex-with-descriptors branch from 8bc3964 to 61fb410 Compare December 9, 2021 15:41
@maflcko
Copy link
Member

maflcko commented Dec 9, 2021

cr ACK 61fb410

@maflcko maflcko merged commit 2f26d8e into bitcoin:master Dec 9, 2021
@theStack theStack deleted the 202112-test-fix-feature_coinstatsindex-with-descriptors branch December 9, 2021 16:57
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 10, 2021
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
…escriptors` and add to test runner

7b3cddc test: add feature_coinstatsindex.py --descriptors to test_runner.py (Sebastian Falbesoner)
f2aba12 test: fix test feature_coinstatsindex.py for descriptor wallets (Sebastian Falbesoner)

Pull request description:

  The functional test feature_coinstatsindex.py currently fails on master branch, if descriptor wallets are used (argument `--descriptors`; or if BDB is not compiled, see bitcoin/bitcoin#23682 (comment)). This is due to the fact that different change output types are used for created transactions (P2WPKH for legacy wallets, P2TR for descriptor wallets; the former doesn't have a ScriptPubKeyMan for bech32m), resulting in different tx sizes and hence also fees. Fix this by explicitely setting the output type via passing both `-addresstype=bech32` and `-changetype=bech32` as argument. The former would not be needed by now, but makes the test more deterministic and avoids a failure if bech32m becomes the default address type.

  Should further pave the way for #23682.

ACKs for top commit:
  MarcoFalke:
    cr ACK 7b3cddc

Tree-SHA512: 300a53f539c0b874da5fc1dd1e4e41b9408dc5526c5858c79f0aabf2ab07e57df4c9cc627fafe25246206752754a91a2977a3df8f8b2d98fb98e51c7e4d81633
@bitcoin bitcoin locked and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants