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.

Sign up for GitHub

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

Jump to bottom

Add feerate_percentiles to blockinfo endpoint #3753

Merged
merged 16 commits into from
May 24, 2024
Merged

Add feerate_percentiles to blockinfo endpoint #3753

merged 16 commits into from
May 24, 2024

Conversation

benbuschmann
Copy link
Contributor

@benbuschmann benbuschmann commented May 14, 2024 •
edited

Add feerate_percentiles to blockinfo

@benbuschmann benbuschmann changed the title Add fee_rate_percentile.fr_10th to blockinfo Add fee rate percentile - 10th to blockinfo endpoint May 15, 2024
@benbuschmann benbuschmann changed the title Add fee rate percentile - 10th to blockinfo endpoint Add fee rate 10th percentile to blockinfo endpoint May 15, 2024
@raphjaph
Copy link
Collaborator

Why only the 10th percentile? We could consider allowing all of them.

Just out of interest, what are you building with this?

@benbuschmann
Copy link
Contributor Author

Why only the 10th percentile? We could consider allowing all of them.

Just out of interest, what are you building with this?

I am looking for the lowest fee that was included in a block and min_fee_rate seems unreliable. I will be using the low fee rate to trigger a dynamic inscription. I can add all 5 percentiles later today if you think it's worth doing all at once.

@raphjaph
Copy link
Collaborator

As part of the strict backwards compatibility requirements for recursive endpoints we can only add fields, never remove or change an existing one. So I think adding all of them into one field makes sense here.

@benbuschmann benbuschmann changed the title Add fee rate 10th percentile to blockinfo endpoint Add feerate_percentiles to blockinfo endpoint May 17, 2024
@benbuschmann
Copy link
Contributor Author

@raphjaph ok I added feerate_percentiles, does this look correct to you?

raphjaph
raphjaph requested changes May 20, 2024
View reviewed changes
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

Looks good so far. Just a small docs addition needed

docs/src/inscriptions/recursion.md Show resolved Hide resolved
raphjaph
raphjaph requested changes May 22, 2024
View reviewed changes
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

Looks good, just a small nit

docs/src/inscriptions/recursion.md Outdated Show resolved Hide resolved
raphjaph
raphjaph approved these changes May 23, 2024
View reviewed changes
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

LGTM

@raphjaph raphjaph enabled auto-merge (squash) May 23, 2024 16:41
@raphjaph raphjaph merged commit 034cb72 into ordinals:master May 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raphjaph raphjaph raphjaph approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Tracker
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@benbuschmann @raphjaph