Skip to content

PoC - move process_compute_budget_instructions() into own crate and more #2193

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

Conversation

tao-stones
Copy link

@tao-stones tao-stones commented Jul 18, 2024

Problem

  • solana-compute-budget has process_compute_budget_instructions();
  • process_compute_budget_instructions() needs to access builtin programs' default costs to correctly allocate compute budget for builtin instructions;
  • each builtin programs define their own default costs; and they all (in)directly depends on solana-compute-budget crate;
    So, there will be circular dependencies for process_compute_budget_instructions() to builtin default cost.

Summary of Changes

  1. Commit 1: refactor, move statically defined BUILT_IN_INSTRUCTION_COSTS out of solana-cost-model into its own crate.

    • it serves as adaptor to reference to all current, and future, builtin crates
    • being in its own crate, it can be shared between solana-cost-model and other crates
    • future works will be isolated into this crate. For example: look up builtin cost by instruction instead of program-id; add ability to build the map at compile-time, so dev dont need to worry about updating map when adding new builtin
  2. Commit 2: refactor, move process_compute_budget_instructions() out of solana-compute-budget into its own crate.

    • new crate solana-compute-budget-processor breaks circular dep,
    • solana-compute-budget becomes purely "type def", safer and easier to share
    • encourages process_compute_budget_instructions() to do One Thing: parse transaction for compute-budget-limits, its result must be consistent regardless of current working_bank. Secondary functions can regulate/validate results on call site using working_bank.
  3. Commit 3: refactor, Added intermediate InstructionDetails struct to store raw information after instructions iteration; break process_compute_budget_instructions() into two steps: one iterate instructions to retrieve InstructionDetails that's suitable for caching-n-reusing, then sanitize and convert to ComputeBudgetLimits.

  4. Commit 4: refactor: Instruction processor takes SVMMessage in place of CompiledInstruction, fixes process_compute_budget_instructions use non-owned SVMInstruction from svm-transaction #2112

  5. Commit 5: feat, feature gated, compute budget allocates default cost units for builtin instructions, including compute-budget instructions themselves; Error transaction early if compute-unit-limit is invalid.
    This fixes Feature Gate: Fail transaction if requested cu-limit exceeds max limit #885, compute-budget allocates constantly defined default-cost for builtin instructions #2212

Fixes #

@tao-stones tao-stones changed the title PoC - move process_compute_budget_instructions() into own crate and mroe PoC - move process_compute_budget_instructions() into own crate and more Jul 19, 2024
@tao-stones tao-stones force-pushed the poc-crate-for-process-compute-budget-instructions branch 9 times, most recently from f37a8bd to f5e03ca Compare July 25, 2024 21:20
…olana-cost-model to its own crate. Makes sharing builtin costs between crates easier, and provide space to future refactoring
…ompute-budget into its own crate; so it could uses builtin programs crates without creating circular deps
…w information after instructions iteration; break part of function that sanitizes and converts into ComputeBudgetLimits in separate function.
@tao-stones tao-stones force-pushed the poc-crate-for-process-compute-budget-instructions branch from f5e03ca to 34cf732 Compare July 25, 2024 22:52
…ructions, including compute-budget instructions themselves;

         Error transaction early if compute-unit-limit is invalid
@tao-stones tao-stones force-pushed the poc-crate-for-process-compute-budget-instructions branch from 34cf732 to c090bfb Compare July 26, 2024 17:01
@joncinque
Copy link

This PR contains changes to the solana sdk, which will be moved to a new repo within the next week, when v2.2 is branched from master.

Please merge or close this PR as soon as possible, or re-create the sdk changes when the new repository is ready at https://github.com/anza-xyz/solana-sdk

@tao-stones
Copy link
Author

out-dated, close

@tao-stones tao-stones closed this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants