-fallbackfee should apply to estimatesmartfee

kylezs commented at 0:50 am on April 4, 2023: none

Please describe the feature you’d like to see added.

-fallback fee is used by sendtoaddress to provide a fallback fee when sending transactions. However, this does not apply to estimatesmartfee, which still returns the error.

The reason for this is because it’s difficult to test client applications on regtest, that would use estimatesmartfee on mainnet, but because it requires a lot of transactions before estimatesmartfee to work, it’s difficult. This related stack exchange question has had a bit of interest: https://bitcoin.stackexchange.com/questions/89607/how-can-i-force-estimatesmartfee-to-return-estimates-on-regtest

Describe the solution you’d like

See above.

Describe any alternatives you’ve considered

A separate flag would also work, like fallbackestimatefee

Please leave any additional context

No response

kylezs added the label Feature on Apr 4, 2023

willcl-ark commented at 9:49 pm on April 5, 2023: member

@kylezs Could you program your test to use the --fallbackfee if it recieves the error in reponse, or as Antoine reponded to you on SE, fill up your test mempool so that estimatesmartfee works as intended?

Accessing --fallbackfee from within the fee estimation rpc involves importing a lot of wallet code which would be preferable to avoid…

benthecarman commented at 7:01 pm on April 24, 2023: contributor

+1 this would be nice, applications like CLN use this rpc and makes it hard to test in regtest / signet environments

instagibbs commented at 3:12 pm on May 2, 2023: member

What’s the problem with just doing error handling, and manually applying your own fallback fee? Faking a result is another layer of confusion imo

kylezs commented at 3:36 pm on May 2, 2023: none

What’s the problem with just doing error handling, and manually applying your own fallback fee? Faking a result is another layer of confusion imo

Because a -fallback fee already exists. Why can’t it exist for other use cases?

Also because the manually applying a fallback case is not actually testing the case I want to test, the case where the fee is returned by the RPC, which is going to be the case 99% of the time on mainnet.

instagibbs commented at 3:49 pm on May 2, 2023: member

If you absolutely cannot handle RPC exceptions your test harness could also:

mock out estimatesmartfee calls to give something deterministic
load a saved fee estimation file

glozow added the label TX fees and policy on Jun 1, 2023

glozow commented at 12:30 pm on June 1, 2023: member

Because a -fallback fee already exists. Why can’t it exist for other use cases?

Node and wallet are 2 separate components; it doesn’t makes sense for a wallet option to bleed into node behavior. Another example: the wallet setting -walletrejectlongchains=0 does not mean the mempool should also accept long chains of transactions, and the existence of that wallet option does not mean we should also add that option to the node. If inheritance exists, the wallet’s options should be informed by the node’s options instead of the other way around. e.g. if the node is configured to disallow descendant count larger than 5, then the wallet shouldn’t make chain longer than 5 since the node it submits transactions to won’t allow it.

If we were to do this, we should have a separate set of fallback feerates for the fee estimator (perhaps they would have the same values as the wallet ones, but these should be independent concepts).

However, I also don’t think it makes sense for our fee estimator to fall back to hard-coded values. When there is no data, it should say so rather than blindly guess. That can be pretty dangerous.

It also sounds like your application here should add error handling for this case. Just because your mainnet fee estimator hasn’t given you this error (yet) doesn’t mean you shouldn’t handle this case.

If your test cares about the fee estimator’s behavior, the the test setup should include some chain activity with transactions getting mined at various feerates so you can assert the results are what you expect. If your test isn’t concerned with the fee estimator’s exact behavior, as @instagibbs says, you can mock it or load a placeholder fee_estimates.dat file to populate the estimator with data.

maflcko removed the label Feature on Jun 1, 2023

maflcko added the label Questions and Help on Jun 1, 2023

kylezs commented at 7:22 am on June 2, 2023: none

If your test isn’t concerned with the fee estimator’s exact behavior, as @instagibbs says, you can mock it or load a placeholder fee_estimates.dat file to populate the estimator with data.

This might be the way to go, is there one that exists that can be downloaded and loaded in? I don’t really mind what the exact behaviour of the estimator is.

instagibbs commented at 1:39 pm on June 2, 2023: member

Any long-running node has them if the node has ever restarted. I just grabbed one locally when needed.

instagibbs commented at 1:46 pm on June 2, 2023: member

#27622 might want to keep an eye on this though, I think we should probably have a flag allowing any age of file, exposed on regtest/hidden only?

glozow commented at 2:32 pm on June 2, 2023: member

I’m going to close this particular request as I think it’s clear we shouldn’t add a fallback feerate for the estimator. Will keep in mind the importance of being able to easily load a fee_estimates.dat for testing. Thanks very much for your feedback @kylezs!

glozow closed this on Jun 2, 2023

kylezs commented at 2:36 pm on June 2, 2023: none

Sounds good. Cheers!

bitcoin locked this on Jun 1, 2024

-fallbackfee should apply to estimatesmartfee #27415

Please describe the feature you’d like to see added.

Is your feature related to a problem, if so please describe it.

Describe the solution you’d like

Describe any alternatives you’ve considered

Please leave any additional context