Wallet: Disable -fallbackfee by default #16524

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. 
   MarcoFalke commented at 9:44 pm on August 1, 2019: member

   Fine with me, but I doubt the tests are passing with this change.
4. MarcoFalke renamed this:
   Truly decouple wallet from chainparams for -fallbackfee
   test: Disable -fallbackfee by default
   on Aug 1, 2019
5. MarcoFalke added the label Tests on Aug 1, 2019
6. 
   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.

7. jtimon renamed this:
   test: Disable -fallbackfee by default
   Wallet: Disable -fallbackfee by default
   on Aug 1, 2019
8. 
   DrahtBot commented at 11:35 pm on August 1, 2019: member

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

   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.

9. jtimon force-pushed on Aug 1, 2019
10. 
    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,
11. DrahtBot added the label Needs rebase on Aug 2, 2019
12. 
    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.
13. 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.

14. jtimon force-pushed on Aug 7, 2019
15. 
    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.
16. jtimon force-pushed on Aug 7, 2019
17. DrahtBot removed the label Needs rebase on Aug 7, 2019
18. jtimon force-pushed on Aug 7, 2019
19. 
    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.

20. laanwj added this to the milestone 0.20.0 on Sep 30, 2019
21. 
    MarcoFalke commented at 2:43 pm on October 2, 2019: member

    ACK 47c0246c44b687f369c15e885c9ef071c22a2597
22. 
    MarcoFalke commented at 2:47 pm on October 2, 2019: member

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

    0Low-level changes
    1=================
    2
    3Tests
    4---
    5
    6- `-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)
    

23. MarcoFalke added the label Waiting for author on Oct 2, 2019
24. 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
25. jtimon force-pushed on Oct 2, 2019
26. 
    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.

27. MarcoFalke removed the label Waiting for author on Oct 2, 2019
28. 
    MarcoFalke commented at 5:42 pm on October 2, 2019: member

    ACK ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
     1Hash: SHA512
     2
     3ACK ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4
     4-----BEGIN PGP SIGNATURE-----
     5
     6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
     7pUgysQwAhkIYBFGteQsfMxLROVL2n3C3HXXl/sufCxtEeyZUTkZJBAV+6Qu/lRc7
     88eaFGekk92tvnoMIKQL5pANIJfxWKJQsSOKtZGAa+zQ3CtAvZJwiZPZOMpUPorz2
     9MeyaBVf4UrmrDdq1L4gJMv0pfE7RsSG1Svl6weDm03Nb2C81inZwzqsLkrqNAaCj
    10TauZPi+mad2wX7SS2r0VHIp4/ZFsHijJbLoTd30f0q0GJQXnoV9zR8LuAvZ//Fu0
    11wnUaawJ2yO4YeiGY0K0OGoI+LE4zGzXuRcMa1UFRZ5xGtJfoHQLt1fk4kI8FbNPl
    1263KQJIIXyYrERlv78NJpzhWK1IfeO4RvLakw/RlnXwKmpSpjIBdKI5Ugj+AI74e2
    13N+CKrlCZw8b+c7MzTu0ABRipYuF/V9xgjuLPkakqyHsCALDVAS0Kd4ZkPdi7qOPL
    14Jk82Tit07J5jBYYu6xGO8eCvwMpjINYwG+vgEe9CAUu8hfh7wq6QG4WSaTpJ+nJX
    15TtgZij77
    16=SeZE
    17-----END PGP SIGNATURE-----
    

    Timestamp of file with hash 0fc571c1687439a4b0b402de2f3462f3b403a6ae5f6c7f06e547b017fe116a3d -

29. MarcoFalke referenced this in commit a689c11907 on Oct 2, 2019
30. MarcoFalke merged this on Oct 2, 2019
31. MarcoFalke closed this on Oct 2, 2019

    ---

32. jtimon deleted the branch on Oct 2, 2019
33. 
    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?
34. jtimon referenced this in commit 411a4955f9 on Oct 3, 2019
35. MarcoFalke referenced this in commit fad1dbda98 on Oct 3, 2019
36. jtimon referenced this in commit a5cf62e2f3 on Oct 3, 2019
37. instagibbs referenced this in commit 95c9387215 on Oct 7, 2019
38. darwin referenced this in commit b774a5ec74 on Oct 7, 2019
39. SomberNight referenced this in commit 26950197db on Jun 17, 2020
40. SomberNight referenced this in commit 2580832a88 on Jun 17, 2020
41. deadalnix referenced this in commit cbcbb33624 on Jul 31, 2020
42. ghubstan referenced this in commit 685839d348 on Aug 12, 2020
43. kristapsk referenced this in commit 17d4094860 on Oct 29, 2020
44. MarcoFalke locked this on Dec 16, 2021

Contributors
jtimon MarcoFalke DrahtBot promag laanwj

Labels
Wallet Tests

Milestone
0.20.0