Skip to content
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

Fix feerate calculation #9558

Merged
merged 7 commits into from Nov 18, 2022
Merged

Fix feerate calculation #9558

merged 7 commits into from Nov 18, 2022

Conversation

lontivero
Copy link
Collaborator

@lontivero lontivero commented Nov 16, 2022

Fixes #9323 and removes fee rate tolerance workaround. This PR takes into account the shared overhead for estimated size and effective fee rate depending on whether the coordinator pays for it or not.

Update:

Look, a bitcoin transaction has inputs, outputs and 58 bytes that are common (version, locktime, two 3 byte varints are non-witness data, marker and flags are witness data).
Clients pay the specified fee rate for their inputs and outputs however nobody is paying for the common parts. That means that if the fee rate is 100sats/B and you register one input (69bytes) and one output (31bytes) you have to pay 100 sats, because the fee rate is 100 sats/B and you have registered 100 bytes. However the transactions is 158 bytes and that means that you didn't pay the correct fee rate (100 sats/bytes) but a much lower fee (63.29 sats/byte)

For that reason the guard condition EffectiveFeeRate < Parameters.MiningFeeRate started to fail when the mining fee rate went up and the number of inputs/outputs went down. In other words: it was working just by pure chance.

- First, someone has to pay for the common overhead, otherwise all the math is wrong.
- Second, the balance and effective feerate were not taking the shared overhead into account.
Co-authored-by: Kimi <58662979+kiminuo@users.noreply.github.com>
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

cACK, see my review here #9566

And one more, the coordinator and the blame script are the same. So we are paying that fee twice.

Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Rounds aborted because not enough fee
4 participants