-
Notifications
You must be signed in to change notification settings - Fork 117
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
Fee bumping when fee estimation doesn't meet min relay fee #1191
Conversation
Pull Request Test Coverage Report for Build 11952886458Details
💛 - Coveralls |
itest failing due to feeEstimation changes |
9c745bd
to
454ea22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, LGTM 🎉
454ea22
to
dfc0a23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks close! Just one note: re handling manual feerates.
The itest can be expanded a bit to check for failure on an insufficient manual feerate for both mint and transfer as well.
882d026
to
8325301
Compare
8325301
to
d410b47
Compare
d410b47
to
1159a3f
Compare
Famous last words. 😄 Anyways, I've changed both Please also have a look at Lines 685 to 686 in 1159a3f
If I'm right we could remove a check there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, LGTM 🎉
1159a3f
to
d0d7417
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great changes + nice to see itest improvements 🎉
d0d7417
to
409b018
Compare
This commit includes a minrelayfee check in `checkFeeRateSanity` so that we can err fast if a manually provided feerate doesn't meet minrelayfee.
If the fee estimation returns a fee rate lower than the min relay fee, we should use the min relay fee instead. This commit implements this behavior for the minting transaction.
If the fee estimation returns a fee rate lower than the min relay fee, we should use the min relay fee instead. This commit implements this behavior for the tapfreighter.
The `testMinRelayFeeBump` itest checks that the minting transaction and a basic send obtain a fee bump when the min relay fee is increased to a value that is higher than the fee estimation. fix
409b018
to
b9a64f4
Compare
Both the minting transaction and normal on chain transactions suffer from edge cases where the fee estimation comes up with a fee that doesn't meet the current min relay fee. This PR changes that behavior by checking the estimated fee against the min relay fee, and bumps the fee if it doesn't clear the minimum fee height implied by min relay fee.
fixes #1171