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

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Test with:

$ rm -rf ~/.bitcoin/regtest && ./bitcoind -regtest -daemon
$ ./bitcoin-cli -regtest createwallet "dummy"
$ ./bitcoin-cli -regtest -generate 101 >/dev/null
$ ./bitcoin-cli -regtest sendtoaddress $(./bitcoin-cli -regtest getnewaddress) 1.0 

feel like this has been modified multiple times in the past. Latest(?) I think was where it was all set to default off: #16524

earlier was turned off for mainnet: #11882

tend to be nack-ish to make more config defaults difference because it "doesn't seem very helpful", but not strong feeling either

feel like this has been modified multiple times in the past. Latest(?) I think was where it was all set to default off: #16524

earlier was turned off for mainnet: #11882

tend to be nack-ish to make more config defaults difference because it "doesn't seem very helpful", but not strong feeling either

Rephrased the OP more straightforwardly.

Having to run bitcoin-cli -regtest stop; bitcoind -daemon -fallbackfee=0.00001 just to send a transaction to myself on regtest is annoying, and seems like precisely the reason we have defaults...

OTOH, running bitcoin-cli -named -regtest sendtoaddress $ADDR 1.0 -fee_rate=3 works now, so perhaps I should just train myself to do that and ignore the suggestion in the error message.

Historically:

• Add sane fallback for fee estimation #7296 (2016-01-13) introduced it
• [Qt] Show more significant warning if we fall back to the default fee #9481 (2017-03-14) did a gui update to warn when the fallback fee might be used
• Disable default fallbackfee on mainnet #11882 (2018-03-02) dropped it on mainnet after the fee spikes in late 2017. See also fees: Remove fallbackfee default #11918
• Remove wallet settings from chainparams #16402 (2019-07-27) removed IsFallbackFeeEnabled() from chainparams replacing it with IsTestChain() (cf Remove wallet settings from chainparams #16402 (comment))
• Wallet: Disable -fallbackfee by default #16524 (2019-10-03) dropped the hardcoded 20sat/vb default for test nets with the goal of removing a call to Params()
• Chainparams: Wallet: Decouple DefaultFallbackfee() from IsTestChain() #16771 (2019-09; closed) was an alternative to Wallet: Disable -fallbackfee by default #16524 that reintroduced the wallet defaults into chainparams in order to drop the IsTestChain() function entirely, in place of having more fine-grained control over testing params via chainparams (Chainparams: Rename IsTestChain() to AllowAcceptNonstd() #16770)
• wallet: Give slightly more understandable advice when needing -fallbackfee #18214 (2020-02; closed) suggested the error message could be better

We already use Params().IsTestChain() in DescriptorScriptPubKeyMan::SetupDescriptorGeneration, and also use Params() for .MessageStart() and .GetChainTypeString(). Those all seem straightforward to abstract into interfaces::Chain if we care.

OTOH, running bitcoin-cli -named -regtest sendtoaddress $ADDR 1.0 -fee_rate=3 works now, so perhaps I should just train myself to do that and ignore the suggestion in the error message.

You can also use settxfee (to persist the config for the wallet and the duration of your RPC session), or persist your fee settings in a config file?

unrelated: I guess we can drop IsTestChain now and replace it with a check on the enum class ChainType == MAIN?

unrelated: I guess we can drop IsTestChain now and replace it with a check on the enum class ChainType == MAIN?

Better to keep IsTestChain() but replace m_is_test_chain with m_chain_type==MAIN ?

Doesn't seem to be much interest, closing