Skip to content

remove cu price rounding#3047

Merged
apfitzge merged 2 commits into
anza-xyz:masterfrom
apfitzge:remove_cu_rounding_feature
Oct 2, 2024
Merged

remove cu price rounding#3047
apfitzge merged 2 commits into
anza-xyz:masterfrom
apfitzge:remove_cu_rounding_feature

Conversation

@apfitzge
Copy link
Copy Markdown

@apfitzge apfitzge commented Oct 1, 2024

Problem

Summary of Changes

  • Remove all code related to that feature
  • Remove all the now unused variables/fields

Fixes #

@apfitzge apfitzge marked this pull request as ready for review October 1, 2024 21:35
@apfitzge apfitzge requested a review from tao-stones October 1, 2024 21:35
Copy link
Copy Markdown

@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.

Overall look reasonable, it is essentially a manual revert of solana-labs#31549.

I closed related feature Issue solana-labs#31453.

Comment thread sdk/src/packet.rs Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Might be better to change it to 0010_000, so there would be no "gap"?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These are serialized in banking_trace data, so I think best to keep the meaning of each flag consistent.

ROUND_COMPUTE_UNIT_PRICE is actually an exception since that was populated only in banking_stage's copy of the Packet.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Still not great to leave a gap in bit flags. If don't want to introduce a break change in banking_trace at the moment, how about tag 0b0010_0000 as UNUSED?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@apfitzge apfitzge force-pushed the remove_cu_rounding_feature branch from 540ddc4 to f9687a3 Compare October 2, 2024 01:29
@apfitzge apfitzge force-pushed the remove_cu_rounding_feature branch from f9687a3 to 1378868 Compare October 2, 2024 13:44
@apfitzge apfitzge requested a review from tao-stones October 2, 2024 14:02
@apfitzge apfitzge self-assigned this Oct 2, 2024
Copy link
Copy Markdown

@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

@apfitzge apfitzge merged commit b39bcb6 into anza-xyz:master Oct 2, 2024
@apfitzge apfitzge deleted the remove_cu_rounding_feature branch October 2, 2024 16:45
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
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.

2 participants