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.2: Add agave-precompiles crate (backport of #5509) #5523

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Mar 27, 2025

Problem

Precompile verification is subject to runtime feature gates and should live in the agave monorepo rather than creating a circular dependency between agave and the sdk repos.

Summary of Changes

  • Pull in precompile definitions and the verification code from all 3 precompiles

Everything from sdk that was pulled into agave in this PR will get deprecated in the sdk crates

Fixes #


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

@mergify mergify bot requested a review from a team as a code owner March 27, 2025 00:34
@mergify mergify bot added the conflicts label Mar 27, 2025
Copy link
Author

mergify bot commented Mar 27, 2025

Cherry-pick of e1162f7 has failed:

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

You are currently cherry-picking commit e1162f731.
  (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:   precompiles/Cargo.toml
	new file:   precompiles/src/ed25519.rs
	new file:   precompiles/src/lib.rs
	new file:   precompiles/src/secp256k1.rs
	new file:   precompiles/src/secp256r1.rs
	modified:   program-runtime/src/invoke_context.rs
	modified:   programs/bpf_loader/Cargo.toml
	modified:   runtime/src/verify_precompiles.rs
	modified:   svm/Cargo.toml
	modified:   svm/src/message_processor.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   Cargo.lock
	both modified:   Cargo.toml
	both modified:   program-runtime/Cargo.toml
	both modified:   programs/bpf_loader/src/syscalls/mod.rs
	both modified:   programs/sbf/Cargo.lock
	both modified:   runtime/Cargo.toml
	both modified:   runtime/src/bank.rs
	both modified:   svm/examples/Cargo.lock

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

Copy link
Author

mergify bot commented Mar 27, 2025

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

@jstarry

This comment was marked as resolved.

This comment was marked as resolved.

@jstarry

This comment was marked as resolved.

This comment was marked as resolved.

@jstarry

This comment was marked as resolved.

This comment was marked as resolved.

@jstarry jstarry force-pushed the mergify/bp/v2.2/pr-5509 branch from 95c9e17 to 483f2ed Compare March 27, 2025 13:59
@@ -419,7 +421,7 @@ solana-example-mocks = "=2.2.1"
solana-faucet = { path = "faucet", version = "=2.2.6" }
solana-feature-gate-client = "0.0.2"
solana-feature-gate-interface = "=2.2.1"
solana-feature-set = "=2.2.1"
solana-feature-set = "=2.2.4" # will be removed
Copy link

Choose a reason for hiding this comment

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

This was required to satisfy the dependency resolver

Choose a reason for hiding this comment

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

these can be gone before the next 2.2 release, right?

Copy link

Choose a reason for hiding this comment

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

yeah they will be removed in #5537

@@ -43,7 +43,7 @@ solana-compute-budget = { path = "../../compute-budget", version = "=2.2.6" }
solana-compute-budget-instruction = { path = "../../compute-budget-instruction", version = "=2.2.6" }
solana-curve25519 = { path = "../../curves/curve25519", version = "=2.2.6" }
solana-decode-error = "=2.2.1"
solana-feature-set = "=2.2.1"
solana-feature-set = "=2.2.4" # will be removed
Copy link

Choose a reason for hiding this comment

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

This was required to satisfy the dependency resolver

@jstarry jstarry requested a review from joncinque March 27, 2025 14:34
@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2025

Codecov Report

Attention: Patch coverage is 97.13542% with 33 lines in your changes missing coverage. Please review.

Project coverage is 83.3%. Comparing base (3f90a6c) to head (4eb3241).
Report is 1 commits behind head on v2.2.

Additional details and impacted files
@@            Coverage Diff            @@
##             v2.2    #5523     +/-   ##
=========================================
  Coverage    83.3%    83.3%             
=========================================
  Files         801      805      +4     
  Lines      370379   371531   +1152     
=========================================
+ Hits       308650   309802   +1152     
  Misses      61729    61729             
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@t-nelson
Copy link

@mergify rebase

Copy link
Author

mergify bot commented Mar 27, 2025

rebase

✅ Branch has been successfully rebased

@t-nelson t-nelson force-pushed the mergify/bp/v2.2/pr-5509 branch from 483f2ed to 4eb3241 Compare March 27, 2025 18:22
@jstarry jstarry requested a review from apfitzge March 27, 2025 21:26
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.

Backport lgtm.

Discussed offline with @jstarry about the concern with SVM dependency on monorepo.

  1. may be possible to migrate precompiles to normal bpf programs
  2. use a similar setup to builtins for precompiles; agave adds them to TransactionProcessor

@jstarry jstarry merged commit 318dd92 into v2.2 Mar 27, 2025
46 checks passed
@jstarry jstarry deleted the mergify/bp/v2.2/pr-5509 branch March 27, 2025 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants