New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee #7084

Merged
merged 2 commits into from Feb 2, 2016

Conversation

Projects
None yet
5 participants
@MarcoFalke
Member

MarcoFalke commented Nov 23, 2015

maxTxFee is the absolute upper bound on fees of transactions created by the wallet.

  • 10000*minRelayTxFee_default = 0.1 BTC/kB
  • maxTxFee_default = 0.1 BTC

Further info:
This check was initially only added to the mempool to reject high fee raw transactions by default (sendrawtransaction) but was later extended to also check wallet transactions. minRelayTxFee may not be the best config param to adjust fee behavior so switching to the already used maxTxFee could make sense.

This fixes the issue:

-> settxfee 0.1
<- true

-> sendtoaddress mwEHWzBhaPYiErWWhmWW7QCjGoKNPaQQax 16
<- Error: The transaction was rejected! This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here. (code -4)
@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Nov 24, 2015

Contributor

utACK

Contributor

dcousens commented Nov 24, 2015

utACK

@jtimon

This comment has been minimized.

Show comment
Hide comment
@jtimon

jtimon Nov 27, 2015

Member

Maybe you prefer to pass it to AcceptToMemoryPool explicitly directly. That would be a little bit more disruptive though.

Anyway, better is better, utACK.

Bike-shedding: Can the global be named globalMaxTxFee ? You could also add the following to the beginning of AcceptToMemoryPool() ``` const CAmount& maxTxFee = globalMaxTxFee; ``` as a preparation for making it a explicit parameter to AcceptToMemoryPool.

EDIT: never mind the bike-shedding nit, the name maxTxFee has to be preserved because it's currently used like that in the wallet.

Member

jtimon commented Nov 27, 2015

Maybe you prefer to pass it to AcceptToMemoryPool explicitly directly. That would be a little bit more disruptive though.

Anyway, better is better, utACK.

Bike-shedding: Can the global be named globalMaxTxFee ? You could also add the following to the beginning of AcceptToMemoryPool() ``` const CAmount& maxTxFee = globalMaxTxFee; ``` as a preparation for making it a explicit parameter to AcceptToMemoryPool.

EDIT: never mind the bike-shedding nit, the name maxTxFee has to be preserved because it's currently used like that in the wallet.

@MarcoFalke MarcoFalke closed this Dec 1, 2015

@MarcoFalke MarcoFalke deleted the MarcoFalke:MarcoFalke-2015-mempoolMaxTxFee branch Dec 1, 2015

@MarcoFalke MarcoFalke restored the MarcoFalke:MarcoFalke-2015-mempoolMaxTxFee branch Dec 1, 2015

@MarcoFalke MarcoFalke reopened this Dec 1, 2015

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Dec 10, 2015

Member

Rebased (trivial)

Member

MarcoFalke commented Dec 10, 2015

Rebased (trivial)

@morcos

View changes

Show outdated Hide outdated src/wallet/wallet.cpp
@morcos

This comment has been minimized.

Show comment
Hide comment
@morcos

morcos Dec 10, 2015

Member

utACK

Member

morcos commented Dec 10, 2015

utACK

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jan 7, 2016

Member

@laanwj Anything holding this back?

Member

MarcoFalke commented Jan 7, 2016

@laanwj Anything holding this back?

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jan 29, 2016

Member

You should also move -maxtxfee in init; it's still under the wallet options, whereas it now applies to transactions submitted over RPC too.

Member

laanwj commented Jan 29, 2016

You should also move -maxtxfee in init; it's still under the wallet options, whereas it now applies to transactions submitted over RPC too.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Jan 30, 2016

Contributor

utACK faa1b1f, but waiting on fix for init option as specified by @laanwj above.

Contributor

dcousens commented Jan 30, 2016

utACK faa1b1f, but waiting on fix for init option as specified by @laanwj above.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jan 30, 2016

Member

I have also fixed the doxygen comments in main.h. Let me know if this is not wanted.

Member

MarcoFalke commented Jan 30, 2016

I have also fixed the doxygen comments in main.h. Let me know if this is not wanted.

Show outdated Hide outdated src/main.h
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Feb 2, 2016

Member

I have also fixed the doxygen comments in main.h. Let me know if this is not wanted.

I'm ok with that, but let's limit the amount of new changes from here on, as they keep people busy with reviewing and commenting :) I'm trying to get this ready for merge.

Member

laanwj commented Feb 2, 2016

I have also fixed the doxygen comments in main.h. Let me know if this is not wanted.

I'm ok with that, but let's limit the amount of new changes from here on, as they keep people busy with reviewing and commenting :) I'm trying to get this ready for merge.

@jtimon

This comment has been minimized.

Show comment
Hide comment
@jtimon

jtimon Feb 2, 2016

Member

re-utACK fa1193e

Member

jtimon commented Feb 2, 2016

re-utACK fa1193e

@laanwj laanwj merged commit fa1193e into bitcoin:master Feb 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 2, 2016

Merge #7084: mempool: Replace maxFeeRate of 10000*minRelayTxFee with …
…maxTxFee

fa1193e [doxygen] Actually display comment (MarcoFalke)
fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

@MarcoFalke MarcoFalke deleted the MarcoFalke:MarcoFalke-2015-mempoolMaxTxFee branch Feb 2, 2016

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7084: mempool: Replace maxFeeRate of 10000*minRelayTxFee with …
…maxTxFee

fa1193e [doxygen] Actually display comment (MarcoFalke)
fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7084: mempool: Replace maxFeeRate of 10000*minRelayTxFee with …
…maxTxFee

fa1193e [doxygen] Actually display comment (MarcoFalke)
fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #7084: mempool: Replace maxFeeRate of 10000*minRelayTxFee with …
…maxTxFee

fa1193e [doxygen] Actually display comment (MarcoFalke)
fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

codablock added a commit to codablock/dash that referenced this pull request Dec 9, 2017

Merge #7084: mempool: Replace maxFeeRate of 10000*minRelayTxFee with …
…maxTxFee

fa1193e [doxygen] Actually display comment (MarcoFalke)
fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

codablock added a commit to codablock/dash that referenced this pull request Dec 11, 2017

Merge #7084: mempool: Replace maxFeeRate of 10000*minRelayTxFee with …
…maxTxFee

fa1193e [doxygen] Actually display comment (MarcoFalke)
fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment