Skip to content

Conversation

@morcos
Copy link
Contributor

@morcos morcos commented Jan 13, 2017

This builds off of #9380... It's just a cleanup PR and should be no change in functionality.

Previously the minReasonableRelayFee which was passed through CTxMempool to CBlockPolicyEstimator was just getting its statically initialized value of DEFAULT_MIN_RELAY_TX_FEE so this PR shortcuts passing it through for no reason.

It's perfectly fine to have a smaller min bucket feerate in fee estimation, it is just slightly less space efficient. However changing this number invalidates old fee estimate files.. So we change the hardcoded minimum rate to be the same rate that was being passed in previously (1000 from DEFAULT_MIN_RELAY_TX_FEE) and we can worry about changing it in the future if that ever becomes not small enough.

@theuni

@morcos morcos force-pushed the removeMinReasonableFee branch from b07419d to ad82cb0 Compare January 17, 2017 18:22
@morcos
Copy link
Contributor Author

morcos commented Jan 17, 2017

rebased

@theuni
Copy link
Member

theuni commented Jan 17, 2017

utACK ad82cb0.

To help other reviewers: The mempool previously accepted an argument that appeared to be coming from the "-minrelaytxfee" option, but in reality (as @morcos said above) the CTxMempool had already been statically created with DEFAULT_MIN_RELAY_TX_FEE before the option was parsed: https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L86

@maflcko
Copy link
Member

maflcko commented Jan 17, 2017 via email

@TheBlueMatt
Copy link
Contributor

utACK ad82cb0.

@ryanofsky
Copy link
Contributor

utACK ad82cb0

Copy link
Contributor

@gmaxwell gmaxwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK.

@laanwj laanwj merged commit ad82cb0 into bitcoin:master Mar 7, 2017
laanwj added a commit that referenced this pull request Mar 7, 2017
ad82cb0 Remove unnecessary min fee argument in CTxMemPool constructor (Alex Morcos)
2a7b56c CBlockPolicyEstimator now uses hard coded minimum bucket feerate (Alex Morcos)
ac9d3d2 Change fee estimation bucket limit variable names (Alex Morcos)

Tree-SHA512: 6e3bc7df3497ed60c7620845d222063e33a0238020f5c3316e61e0eff758078588ea8dd51196ceb59aa561ba106f8cdae62cebe521adb3247108bb49f15252d6
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 5, 2019
ad82cb0 Remove unnecessary min fee argument in CTxMemPool constructor (Alex Morcos)
2a7b56c CBlockPolicyEstimator now uses hard coded minimum bucket feerate (Alex Morcos)
ac9d3d2 Change fee estimation bucket limit variable names (Alex Morcos)

Tree-SHA512: 6e3bc7df3497ed60c7620845d222063e33a0238020f5c3316e61e0eff758078588ea8dd51196ceb59aa561ba106f8cdae62cebe521adb3247108bb49f15252d6
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants