wallet: default -fallbackfee to same as -mintxfee on test chains

ajtowns commented at 2:55 am on August 22, 2023: contributor

On mainnet, the error message:

0Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.

is helpful, because if you aren’t able to set a fee based on an external source or the internal fee estimation code, then you’re likely to either waste money when the default is too high, or have a tx that won’t confirm if the default is too low. On regtest and friends it’s not help, and the advice may well be wrong: particularly on regtest, mining additional empty blocks won’t help at all.

But for test net’s there is an easy default available that is quite reliable: just use the minimum tx fee.

wallet: default -fallbackfee to same as -mintxfee on test chains 957cde2a80

DrahtBot commented at 2:55 am on August 22, 2023: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	BrandonOdiwuor

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

DrahtBot added the label Wallet on Aug 22, 2023

ajtowns commented at 2:57 am on August 22, 2023: contributor

Test with:

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

BrandonOdiwuor commented at 9:32 am on August 23, 2023: contributor

Tested ACK #28316

The PR fixes the ‘fee estimation failed’ error on regtest

pre-fix

0$ ./bitcoin-cli -regtest sendtoaddress $(./bitcoin-cli -regtest getnewaddress) 1.0
1
2error code: -6
3error message:
4Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.

after the fix:

0$ ./bitcoin-cli -regtest sendtoaddress $(./bitcoin-cli -regtest getnewaddress) 1.0
1375bf0933754164c5da7567a6fcdb6ae34140155031cba6f6edcd404960cc835
2
3$ ./bitcoin-cli getrawtransaction 375bf0933754164c5da7567a6fcdb6ae34140155031cba6f6edcd404960cc835
402000000000101e386af630bb3d7a9d170f07cb472c28ffe82b72f797b91a4964803c8e6546d2c0000000000fdffffff027310102401000000160014fe3eccfd9050103fbed369df8c49e5e22f37472000e1f50500000000160014ff2df796bd9ca70e89edd21b8a73c7a53d28abf3024730440220791a2399dc6a4e552902738f9c2d7fbac7bcdc521780e50fe194a922b82b1fdf02205e1c2c26667149277440e4a80a5386d327b9b3e3e6fa87232353c71cd5579a520121026bb5c51c88ad288868560702573ce7e21f6a104e46d052c283e80f37fd9ad58765000000

instagibbs commented at 5:47 pm on August 28, 2023: member

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

ajtowns commented at 9:51 am on August 29, 2023: contributor

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:

#7296 (2016-01-13) introduced it
#9481 (2017-03-14) did a gui update to warn when the fallback fee might be used
#11882 (2018-03-02) dropped it on mainnet after the fee spikes in late 2017. See also #11918
#16402 (2019-07-27) removed IsFallbackFeeEnabled() from chainparams replacing it with IsTestChain() (cf #16402 (comment))
#16524 (2019-10-03) dropped the hardcoded 20sat/vb default for test nets with the goal of removing a call to Params()
#16771 (2019-09; closed) was an alternative to #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 (#16770)
#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.

maflcko commented at 9:58 am on August 29, 2023: member

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?

ajtowns commented at 10:40 am on August 29, 2023: contributor

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 ?

DrahtBot added the label CI failed on Sep 3, 2023

DrahtBot removed the label CI failed on Sep 5, 2023

ajtowns commented at 10:56 am on September 12, 2023: contributor

Doesn’t seem to be much interest, closing

ajtowns closed this on Sep 12, 2023

bitcoin locked this on Sep 11, 2024

wallet: default -fallbackfee to same as -mintxfee on test chains #28316

Reviews

pre-fix

after the fix: