Wallet: Disable -fallbackfee by default #16524

pull jtimon wants to merge 1 commits into bitcoin:master from jtimon:b19-true-wallet-no-chainparams changing 5 files +16 −5
  1. jtimon commented at 8:53 PM on August 1, 2019: contributor

    Before it was 0 by default for main and 20000 for test and regtest. Now it is 0 by default for all chains, thus there's no need to call Params().

    Also now the default for main is properly documented.

    Suggestion for release notes:

    -fallbackfee was 0 (disabled) by default for the main chain, but 20000 by default for the test chains. Now it is 0 by default for all chains. Testnet and regtest users will have to add fallbackfee=20000 to their configuration if they weren't setting it and they want it to keep working like before.

    Should I propose them to the wiki for the release notes or only after merge?

    For more context, see #16402 (comment)

  2. fanquake added the label Wallet on Aug 1, 2019
  3. jtimon cross-referenced this on Aug 1, 2019 from issue Remove wallet settings from chainparams by MarcoFalke
  4. MarcoFalke commented at 9:44 PM on August 1, 2019: member

    Fine with me, but I doubt the tests are passing with this change.

  5. MarcoFalke renamed this:
    Truly decouple wallet from chainparams for -fallbackfee
    test: Disable -fallbackfee by default
    on Aug 1, 2019
  6. MarcoFalke added the label Tests on Aug 1, 2019
  7. jtimon cross-referenced this on Aug 1, 2019 from issue A: Chainparams: Rename RequireStandard() to DefaultAcceptNonstd() by jtimon
  8. jtimon commented at 10:26 PM on August 1, 2019: contributor

    Fine with me, but I doubt the tests are passing with this change.

    Oh, right, the tests that make use of fallbackfee will need to set fallbackfee=20000 explicitly now, I'm happy to fix that, but please tell me if I miss any of the tests, specially feature_pruning,feature_dbcrash,feature_fee_estimation which seem to be the tests I forget about the most lately for some reason.

    Extra thanks for the fast feedback.

  9. jtimon cross-referenced this on Aug 1, 2019 from issue B: Get rid of Params().RequireStandard() by jtimon
  10. jtimon renamed this:
    test: Disable -fallbackfee by default
    Wallet: Disable -fallbackfee by default
    on Aug 1, 2019
  11. DrahtBot commented at 11:35 PM on August 1, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16771 (Chainparams: Wallet: Decouple DefaultFallbackfee() from IsTestChain() by jtimon)

    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.

  12. jtimon force-pushed on Aug 1, 2019
  13. jtimon commented at 1:01 AM on August 2, 2019: contributor

    Added 1 commit to document current failures and one failed attempt to fix all of them at once. I'm not going to continue for todat, sorry,

  14. DrahtBot added the label Needs rebase on Aug 2, 2019
  15. promag commented at 10:29 AM on August 6, 2019: member

    Concept ACK. For now fallback fee could be set for all tests IMO, unless setting explicitly in each tests results in a short change.

  16. in test/functional/test_framework/util.py:298 in 6e26e20771 outdated 
     294 | @@ -295,6 +295,8 @@ def initialize_datadir(dirname, n):
 295 |          f.write("[regtest]\n")
 296 |          f.write("port=" + str(p2p_port(n)) + "\n")
 297 |          f.write("rpcport=" + str(rpc_port(n)) + "\n")
 298 | +        # TODO: Do it per test or in the test framework, don't be lazzy
    MarcoFalke commented at 12:39 PM on August 6, 2019:

    I think this is fine to do here.

  17. jtimon force-pushed on Aug 7, 2019
  18. jtimon commented at 2:37 AM on August 7, 2019: contributor

    Alright, setting it up for all tests is easier, I guess, not a strong opinion. Rebased, squashed and removed the TODOs.

  19. jtimon force-pushed on Aug 7, 2019
  20. DrahtBot removed the label Needs rebase on Aug 7, 2019
  21. jtimon force-pushed on Aug 7, 2019
  22. jtimon cross-referenced this on Aug 31, 2019 from issue Chainparams: Wallet: Decouple DefaultFallbackfee() from IsTestChain() by jtimon
  23. laanwj commented at 11:26 AM on September 30, 2019: member

    Concept ACK

    Should I propose them to the wiki for the release notes or only after merge?

    The release notes are only put on the wiki momentarily before a major release. Between releases, please include the release note change in your PR, preferably as fragment to avoid merge conflicts.

  24. laanwj added this to the milestone 0.20.0 on Sep 30, 2019
  25. MarcoFalke commented at 2:43 PM on October 2, 2019: member

    ACK 47c0246c44b687f369c15e885c9ef071c22a2597

  26. MarcoFalke commented at 2:47 PM on October 2, 2019: member

    Can you add a file ./doc/release-notes-16524.md with the content:

    Low-level changes
=================

Tests
---

- `-fallbackfee` was 0 (disabled) by default for the main chain, but 20000 by default for the test chains. Now it is 0 by default for all chains. Testnet and regtest users will have to add fallbackfee=20000 to their configuration if they weren't setting it and they want it to keep working like before. (#16524)
  27. MarcoFalke added the label Waiting for author on Oct 2, 2019
  28. Truly decouple wallet from chainparams for -fallbackfee
    Before it was 0 by default for main and 20000 for test and regtest.
Now it is 0 by default for all chains, thus there's no need to call Params().

Also now the default for main is properly documented
    ea4cc3a7b3
  29. jtimon force-pushed on Oct 2, 2019
  30. jtimon commented at 4:57 PM on October 2, 2019: contributor

    Fixed nit, added the suggested release notes file.

    As a reminder, this is incompatible with #16771 and we don't want to merge both of them. Personally, I would be happy with either one of them.

  31. MarcoFalke removed the label Waiting for author on Oct 2, 2019
  32. MarcoFalke commented at 5:42 PM on October 2, 2019: member

    ACK ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgysQwAhkIYBFGteQsfMxLROVL2n3C3HXXl/sufCxtEeyZUTkZJBAV+6Qu/lRc7
8eaFGekk92tvnoMIKQL5pANIJfxWKJQsSOKtZGAa+zQ3CtAvZJwiZPZOMpUPorz2
MeyaBVf4UrmrDdq1L4gJMv0pfE7RsSG1Svl6weDm03Nb2C81inZwzqsLkrqNAaCj
TauZPi+mad2wX7SS2r0VHIp4/ZFsHijJbLoTd30f0q0GJQXnoV9zR8LuAvZ//Fu0
wnUaawJ2yO4YeiGY0K0OGoI+LE4zGzXuRcMa1UFRZ5xGtJfoHQLt1fk4kI8FbNPl
63KQJIIXyYrERlv78NJpzhWK1IfeO4RvLakw/RlnXwKmpSpjIBdKI5Ugj+AI74e2
N+CKrlCZw8b+c7MzTu0ABRipYuF/V9xgjuLPkakqyHsCALDVAS0Kd4ZkPdi7qOPL
Jk82Tit07J5jBYYu6xGO8eCvwMpjINYwG+vgEe9CAUu8hfh7wq6QG4WSaTpJ+nJX
TtgZij77
=SeZE
-----END PGP SIGNATURE-----

    Timestamp of file with hash 0fc571c1687439a4b0b402de2f3462f3b403a6ae5f6c7f06e547b017fe116a3d -

    </details>

  33. MarcoFalke referenced this in commit a689c11907 on Oct 2, 2019
  34. MarcoFalke merged this on Oct 2, 2019
  35. MarcoFalke closed this on Oct 2, 2019
  36. jtimon cross-referenced this on Oct 2, 2019 from issue Testchains: Introduce custom chain whose constructor... by jtimon
  37. jtimon deleted the branch on Oct 2, 2019
  38. jtimon cross-referenced this on Oct 2, 2019 from issue Chainparams: Rename IsTestChain() to AllowAcceptNonstd() by jtimon
  39. jtimon commented at 6:32 PM on October 3, 2019: contributor

    Oops, actually the release notes are wrong. People have to set fallbackfee=0.0002 rather than fallbackfee=20000 because it is in BTC, not satoshis. What to do? a PR only for that?

  40. jtimon referenced this in commit 411a4955f9 on Oct 3, 2019
  41. MarcoFalke referenced this in commit fad1dbda98 on Oct 3, 2019
  42. jtimon referenced this in commit a5cf62e2f3 on Oct 3, 2019
  43. achow101 cross-referenced this on Oct 7, 2019 from issue Set the fallbackfee in bitcoind params by achow101
  44. instagibbs referenced this in commit 95c9387215 on Oct 7, 2019
  45. darwin referenced this in commit b774a5ec74 on Oct 7, 2019
  46. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
  47. darosior cross-referenced this on Apr 11, 2020 from issue pyln-testing: specify fallbackfee for newer versions of bitcoind by darosior
  48. SomberNight referenced this in commit 26950197db on Jun 17, 2020
  49. SomberNight referenced this in commit 2580832a88 on Jun 17, 2020
  50. chappjc cross-referenced this on Jun 17, 2020 from issue multi: eliminate funding confirmation requirement by chappjc
  51. nopara73 cross-referenced this on Jul 22, 2020 from issue Update to Bitcoin Core 0.20.0 by nopara73
  52. deadalnix referenced this in commit cbcbb33624 on Jul 31, 2020
  53. ghubstan referenced this in commit 685839d348 on Aug 12, 2020
  54. ghubstan cross-referenced this on Aug 13, 2020 from issue Add API test harness for Linux & OSX by ghubstan
  55. michaelfolkson cross-referenced this on Oct 5, 2020 from issue Wallet: Reenable -fallbackfee by default for regtest and signet (and maybe testnet too)? by michaelfolkson
  56. kristapsk referenced this in commit 17d4094860 on Oct 29, 2020
  57. yanmaani cross-referenced this on Feb 15, 2021 from issue Reintroduce fallbackfee by yanmaani
  58. bitcoin locked this on Dec 16, 2021
Contributors
jtimonMarcoFalkeDrahtBotpromaglaanwj
Labels
WalletTests

Milestone
0.20.0

Linked (view graph)
#8994 Testchains: Introduce custom chain whose constructor... #16402 Remove wallet settings from chainparams#16526 A: Chainparams: Rename RequireStandard() to DefaultAcceptNonstd()#16527 B: Get rid of Params().RequireStandard()#16770 Chainparams: Rename IsTestChain() to AllowAcceptNonstd()#16771 Chainparams: Wallet: Decouple DefaultFallbackfee() from IsTestChain()#20087 Wallet: Reenable -fallbackfee by default for regtest and signet (and maybe testnet too)?
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC