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

Remove prioritization_fee.rs #3235

Merged
merged 5 commits into from
Nov 4, 2024
Merged

Remove prioritization_fee.rs #3235

merged 5 commits into from
Nov 4, 2024

Conversation

ksolana
Copy link

@ksolana ksolana commented Oct 20, 2024

The prioritization_fee.rs had only one relevant calculation of
prioritization fee that was used by ComputeBudgetLimits. This patch moves the calculation to get_prioritization_fee and removes prioritization_fee.rs altogether.

@apfitzge
Copy link

are the other methods used? if the compute unit price isn't used then I feel this trait is useless and we should remove the whole thing.

@tao-stones
Copy link

Agree, it's the time to refactor out PrioritizationFeeDetails since enum PrioritizationFeeType has been reduced to one variance, the legacy logic of converting between user specified priority fee and cu price has gone - now transactions can only specify cu price. So all PrioritizationFeeDetails does is prio-fee = cu-price * cu-limit, which can be done inline in ComputeBudgetLimits, where FeeBudgetLimits are created.

tao-stones
tao-stones previously approved these changes Oct 22, 2024
Copy link

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

changes look good to me - thanks for cleaning it up.

Copy link

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

please update title of the PR and add a basic description to the PR

@ksolana ksolana changed the title Remove dead method in prioritization_fee.rs Remove prioritization_fee.rs Oct 23, 2024
@ksolana ksolana force-pushed the prio_fee branch 2 times, most recently from aa64bd1 to 230e6b3 Compare October 26, 2024 14:00
@apfitzge apfitzge self-requested a review October 31, 2024 14:15
@ksolana ksolana force-pushed the prio_fee branch 2 times, most recently from 91b1ab0 to a835a96 Compare November 2, 2024 04:41
Copy link

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

lgtm

@ksolana ksolana merged commit faf5c73 into anza-xyz:master Nov 4, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants