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

v2.0: Fix reserve minimal compute units for builtins (backport of #3799) #3930

Open
wants to merge 1 commit into
base: v2.0
Choose a base branch
from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Dec 5, 2024

Problem

Implementing solana-foundation/solana-improvement-documents#170 by defining MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT to 3K CUs, then use it to allocate builtin instructions' CU Meters for VM and cost tracking for leaders.

Summary of Changes

  • When calculates default tx cu limits, use MAX_BUILTIN_ALLOCATION_COMPUTE_UNIT_LIMIT per builtin instruction, including compute-budget program instructions.
  • Cost model reads cu limits from RuntimeTransaction's static_meta, replacing a localized implementation that isn't consistent with compute-budget.
  • Changes are behind Feature gate
  • updated existing tests to allow additional feature_set parameters (touch many files)

Feature Gate Issue: #2562


This is an automatic backport of pull request #3799 done by [Mergify](https://mergify.com).

- Add feature gate, issue #2562;
- Implement SIMD-170;

---------

Co-authored-by: Justin Starry <[email protected]>
(cherry picked from commit 3e9af14)

# Conflicts:
#	builtins-default-costs/src/lib.rs
#	compute-budget/src/compute_budget_limits.rs
#	compute-budget/src/compute_budget_processor.rs
#	core/src/banking_stage/consumer.rs
#	core/src/banking_stage/immutable_deserialized_packet.rs
#	core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs
#	cost-model/src/cost_model.rs
#	cost-model/src/transaction_cost.rs
#	programs/compute-budget-bench/benches/compute_budget.rs
#	programs/sbf/tests/programs.rs
#	runtime-transaction/benches/process_compute_budget_instructions.rs
#	runtime-transaction/src/compute_budget_instruction_details.rs
#	runtime-transaction/src/compute_budget_program_id_filter.rs
#	runtime-transaction/src/lib.rs
#	runtime-transaction/src/runtime_transaction.rs
#	runtime-transaction/src/runtime_transaction/sdk_transactions.rs
#	runtime/src/bank.rs
#	runtime/src/bank/tests.rs
#	runtime/src/prioritization_fee_cache.rs
#	sdk/src/feature_set.rs
#	svm-transaction/src/svm_message.rs
#	svm-transaction/src/svm_message/sanitized_message.rs
#	svm-transaction/src/svm_message/sanitized_transaction.rs
#	svm/src/transaction_processor.rs
#	transaction-view/src/resolved_transaction_view.rs
#	transaction-view/src/transaction_view.rs
@mergify mergify bot requested a review from a team as a code owner December 5, 2024 03:50
@mergify mergify bot added conflicts feature-gate Pull Request adds or modifies a runtime feature gate labels Dec 5, 2024
Copy link
Author

mergify bot commented Dec 5, 2024

Cherry-pick of 3e9af14 has failed:

On branch mergify/bp/v2.0/pr-3799
Your branch is up to date with 'origin/v2.0'.

You are currently cherry-picking commit 3e9af14f3a.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   runtime-transaction/src/builtin_programs_filter.rs
	modified:   sdk/program/src/message/sanitized.rs
	modified:   sdk/program/src/message/versions/sanitized.rs

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   builtins-default-costs/src/lib.rs
	deleted by us:   compute-budget/src/compute_budget_limits.rs
	both modified:   compute-budget/src/compute_budget_processor.rs
	both modified:   core/src/banking_stage/consumer.rs
	both modified:   core/src/banking_stage/immutable_deserialized_packet.rs
	deleted by us:   core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs
	both modified:   cost-model/src/cost_model.rs
	both modified:   cost-model/src/transaction_cost.rs
	deleted by us:   programs/compute-budget-bench/benches/compute_budget.rs
	both modified:   programs/sbf/tests/programs.rs
	deleted by us:   runtime-transaction/benches/process_compute_budget_instructions.rs
	deleted by us:   runtime-transaction/src/compute_budget_instruction_details.rs
	deleted by us:   runtime-transaction/src/compute_budget_program_id_filter.rs
	both modified:   runtime-transaction/src/lib.rs
	both modified:   runtime-transaction/src/runtime_transaction.rs
	deleted by us:   runtime-transaction/src/runtime_transaction/sdk_transactions.rs
	both modified:   runtime/src/bank.rs
	both modified:   runtime/src/bank/tests.rs
	both modified:   runtime/src/prioritization_fee_cache.rs
	both modified:   sdk/src/feature_set.rs
	deleted by us:   svm-transaction/src/svm_message.rs
	deleted by us:   svm-transaction/src/svm_message/sanitized_message.rs
	deleted by us:   svm-transaction/src/svm_message/sanitized_transaction.rs
	both modified:   svm/src/transaction_processor.rs
	deleted by us:   transaction-view/src/resolved_transaction_view.rs
	deleted by us:   transaction-view/src/transaction_view.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@t-nelson
Copy link

this has been untouched with conflicts for two weeks. can we close it?

@tao-stones
Copy link

this has been untouched with conflicts for two weeks. can we close it?

I also think better not to backport this one, but let me sync up w @jstarry tomorrow when he's back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts feature-gate Pull Request adds or modifies a runtime feature gate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants