Remove wallet settings from chainparams #16402

1. 
   MarcoFalke commented at 8:25 pm on July 16, 2019: member

   Feels a bit odd to have wallet setting in the chainparams, so remove them from there
2. Remove wallet settings from chainparams fa4a605a4c
3. MarcoFalke added the label Refactoring on Jul 16, 2019
4. MarcoFalke added the label Wallet on Jul 16, 2019
5. 
   promag commented at 10:04 pm on July 16, 2019: member

   ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f, missed s/2018/2019?
6. 
   DrahtBot commented at 6:57 am on July 20, 2019: member

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

   Conflicts

   No conflicts as of last run.

7. darosior approved
8. 
   darosior commented at 1:59 pm on July 21, 2019: member

   ACK fa4a605a4c611abe9af4c18aab20f4d1d039170f
9. 
   practicalswift commented at 11:09 am on July 22, 2019: contributor

   utACK fa4a605a4c611abe9af4c18aab20f4d1d039170f
10. 
    MarcoFalke commented at 1:45 pm on July 25, 2019: member

    @jonasschnelli @jtimon, any objections on this? Otherwise I will merge.
11. MarcoFalke added this to the milestone 0.19.0 on Jul 25, 2019
12. 
    meshcollider commented at 10:27 am on July 27, 2019: contributor

    Seems sane, LGTM fa4a605a4c611abe9af4c18aab20f4d1d039170f
13. meshcollider merged this on Jul 27, 2019
14. meshcollider closed this on Jul 27, 2019

    ---

15. meshcollider referenced this in commit dfb7fd60f2 on Jul 27, 2019
16. MarcoFalke deleted the branch on Jul 27, 2019
17. konez2k referenced this in commit a0d1c1a2fd on Jul 27, 2019
18. 
    jtimon commented at 5:33 pm on July 27, 2019: contributor

    I dislike using IsTestNet for many things, that was my worry when I complained in #15891 (review) , but on the other hand I like to remove wallet stuff from chainparams. I’m not really sure how I feel about this. Will the bech32 and base58 prefixes do the same?

    I guess I would like to hear more opinions about this.

19. 
    MarcoFalke commented at 7:08 pm on July 27, 2019: member

    Will the bech32 and base58 prefixes do the same?

    I think they should stay because they are used even when no wallet is compiled, so they can be considered chain params. Though, chain-specific wallet default values, should, if needed, reside in ./src/wallet/chainparams. Otherwise they will make it harder to move the wallet to a separate repository, since both repos would have to be kept perfectly in sync with regard to the settings.

20. 
    jtimon commented at 6:47 pm on August 1, 2019: contributor

    I understand the desire to decouple from the wallet, but the more I think about this, the less I like it. Now IsTestNet serves for two quite unrelated things. If it was WalletIsTestnet separated from the other one, perhaps it would be better. Let’s say we want regtest to be more like mainnet with respect to the fallback fee, like it happened with the default for acceptnonstdtxs in #15891. Then we would need to reintroduce m_fallback_fee_enabled or, even worse, introduce an IsRegtest method that ends up being used for all kinds of things. For decoupling the wallet part of the chainparams, gaving a src/wallet/chainparams sounds like the long term solution. Or perhaps we can simply have the same defaults for mainnet and the test chains for wallet arguments, since testers can set them to whatever they want manually anyway. Perhaps the extra complexity is not worth it in this case. Either way, I don’t see how this helps towards that goal of separating the wallet.
21. 
    MarcoFalke commented at 7:32 pm on August 1, 2019: member

    m_fallback_fee only exists for testing, if it is deemed that the value should be the same for mainnet, the whole fallback fee logic can be removed instead.
22. 
    jtimon commented at 8:58 pm on August 1, 2019: contributor

    What about doing #16524 ? That’s what I think help the most decoupling wallet and chainparams from each other. I think the same default as for mainnet should be used, which in practice is currently 0 despite what the constant says. I don’t understand why have different defaults for different chains when they’re just defaults that can be manually set by the user, or in a config file where I think they truly belong. Much less now that we have sections. One can just:

    0[main]
    1fallbackfee=0
    2
    3[test]
    4fallbackfee=20000
    5
    6[regtest]
    7fallbackfee=20000
    

    If that’s the functionality it ones.

23. MarcoFalke referenced this in commit a689c11907 on Oct 2, 2019
24. deadalnix referenced this in commit cbcbb33624 on Jul 31, 2020
25. Munkybooty referenced this in commit 2dcad7dc8d on Nov 16, 2021
26. Munkybooty referenced this in commit b1402fbea0 on Nov 18, 2021
27. Munkybooty referenced this in commit 10630ea064 on Nov 24, 2021
28. Munkybooty referenced this in commit 9ce8479513 on Nov 30, 2021
29. Munkybooty referenced this in commit edd26075f7 on Dec 15, 2021
30. MarcoFalke locked this on Dec 16, 2021

Contributors
MarcoFalke promag DrahtBot darosior practicalswift meshcollider jtimon

Labels
Refactoring Wallet

Milestone
0.19.0