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

feat: add compute budget instructions #1

Merged
merged 3 commits into from
Aug 31, 2024

Conversation

sonicfromnewyoke
Copy link
Contributor

I hope I understand your vision for that repository correctly, and compute_budget-related instructions should be located here.

Marking that MR as draft due to next reasons:

  • can't compile that lib and properly check due to compilation issues in the bincode dependency
  • want to add some tests and documentation
  • lint & clean-up the MR

@joncinque
Copy link
Owner

Thanks for your contribution! I would love to integrate these changes.

I've updated everything to Zig 0.13, so everything should compile properly now once you rebase. Let me know when you want me to take a closer look.

src/compute_budget.zig Outdated Show resolved Hide resolved
src/associated_token_account.zig Show resolved Hide resolved
@sonicfromnewyoke sonicfromnewyoke marked this pull request as ready for review August 25, 2024 17:43
Copy link
Owner

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just one tiny nit, the rest looks great!

src/associated_token_account.zig Show resolved Hide resolved
src/compute_budget.zig Show resolved Hide resolved
},
/// Set a specific compute unit limit that the transaction is allowed to consume.
set_compute_unit_limit: struct {
units: u32,
Copy link
Owner

Choose a reason for hiding this comment

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

nit: can this be renamed to compute_units for total clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed units -> compute_units

regarding avoiding allocations for program instructions, Ito not blowing up that thread I'd prefer to open the issue under the solana-program-sdk and describe a couple of possible allocation strategies:

  1. create & destroy
  2. comptime-based
  3. etc.

Copy link
Owner

Choose a reason for hiding this comment

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

Please, that would be perfect

Copy link
Owner

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Perfect, thanks so much for your contribution!

thank you

@joncinque joncinque merged commit ce8da19 into joncinque:main Aug 31, 2024
4 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.

2 participants