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

Simplify fee estimations from mempool #10818

Merged
merged 4 commits into from Jun 5, 2023
Merged

Conversation

lontivero
Copy link
Collaborator

This PR simplifies a bit the logic for estimating the minimum feerate to pay according to the mempool histogram and updates the UTs to use more recent data.

Please @adamPetho take a deep look.

@lontivero
Copy link
Collaborator Author

I found it, it was confusing block size in megabytes with virtual megabytes. Please @adamPetho take a look now.

Copy link
Collaborator

@adamPetho adamPetho left a comment

Choose a reason for hiding this comment

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

LGTM, one final question below from me.

var (sanityFeeRate, minEstimations) = GetFeeEstimationsFromMempoolInfo(mempoolInfo);

var minEstimations = GetFeeEstimationsFromMempoolInfo(mempoolInfo);
var minEstimationFor260Mb = new FeeRate((decimal)minEstimations.GetValueOrDefault(260 / 4));
Copy link
Collaborator

Choose a reason for hiding this comment

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

260 / 4 How come this? I don't get it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there is no way to know how many bytes a virtual byte is in this context so, we could think that 4 virtual bytes are more or less equivalent to 1 real byte. I mean, the mempool size is in real bytes while everything else is expressed in virtual bytes, we need a way to "convert" virtual bytes to real bytes or real bytes to virtual byte.

@lontivero lontivero merged commit c857025 into zkSNACKs:master Jun 5, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants