feat: ephemeral cranks#9
Conversation
WalkthroughThis PR adds a second on-chain program, ChangesEphemeral Rollup Crank Feature
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dodecahedr0x
force-pushed
the
dode/ephemeral-cranks
branch
from
7b9b392 to
7e38e4a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
1-119: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd least-privilege
permissions:and disable credential persistence.Static analysis flags the default
GITHUB_TOKENpermissions (unset → broad scope) andactions/checkoutpersisting credentials on the runner across all three jobs. Since none of these jobs need to write to the repo, tighten both.🔒 Proposed hardening
on: push: branches: [main] pull_request: +permissions: + contents: read + # Cancel superseded runs on the same ref to save runner minutes. concurrency:And for each
actions/checkout@v4step:- uses: actions/checkout@v4 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1 - 119, The CI workflow currently relies on the default broad GITHUB_TOKEN scope and persists checkout credentials in all jobs; tighten this by adding an explicit least-privilege permissions block for the workflow/jobs and disabling credential persistence on each actions/checkout@v4 step. Update the workflow definitions in the CI jobs (fmt, default, e2e) and the checkout configuration so they do not retain repository credentials after cloning.Source: Linters/SAST tools
programs/hydra/src/processor/trigger.rs (1)
112-119: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
data_mut_ptr()for the write path
AccountView::data_ptr()is*const u8, butwrite_u64takes*mut u8. Switch this back todata_mut_ptr()or add an explicit cast if the write is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@programs/hydra/src/processor/trigger.rs` around lines 112 - 119, The write path in the trigger update block is using `AccountView::data_ptr()` even though `write_u64` requires a mutable pointer. Update the `crank_ai` access in the `unsafe` section to use `data_mut_ptr()` instead, or make the mutability explicit if that is the intended write path, so the `write_u64` calls for `OFF_NEXT_EXEC_SLOT`, `OFF_EXECUTED`, and `OFF_REMAINING` operate on a `*mut u8`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 27-104: The CI workflow still uses mutable action refs, so update
the workflows in ci.yml to pin each third-party action to an immutable commit
SHA instead of tags or branches. Replace the uses entries for actions/checkout,
dtolnay/rust-toolchain, taiki-e/install-action, Swatinem/rust-cache, and
actions/setup-node with their corresponding commit-pinned versions, and keep the
existing step structure and names unchanged.
In `@crates/hydra-api/Cargo.toml`:
- Around line 33-37: `ephemeral-rollups-pinocchio` is currently always enabled
in `hydra-api` instead of being gated like the other optional dependencies.
Update `Cargo.toml` so this crate is declared optional and only activated
through the appropriate feature in the same way as `solana-define-syscall` and
`solana-program-error`, then wire it into the relevant feature set in
`hydra-api` (for example the `cpi-pinocchio` path or a dedicated `ephemeral`
feature) to match the intent described by `program/mod.rs`.
In `@crates/hydra-api/src/cpi.rs`:
- Around line 16-25: Update the doc example under hydra_api::cpi::native so it
matches the current CPI API: change the import path to
hydra_api::cpi::base::native and adjust the hydra_cpi::create example to include
the required signer_seeds argument. Keep the example aligned with the current
create signature in cpi.rs so the snippet copy-pastes and compiles.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1-119: The CI workflow currently relies on the default broad
GITHUB_TOKEN scope and persists checkout credentials in all jobs; tighten this
by adding an explicit least-privilege permissions block for the workflow/jobs
and disabling credential persistence on each actions/checkout@v4 step. Update
the workflow definitions in the CI jobs (fmt, default, e2e) and the checkout
configuration so they do not retain repository credentials after cloning.
In `@programs/hydra/src/processor/trigger.rs`:
- Around line 112-119: The write path in the trigger update block is using
`AccountView::data_ptr()` even though `write_u64` requires a mutable pointer.
Update the `crank_ai` access in the `unsafe` section to use `data_mut_ptr()`
instead, or make the mutability explicit if that is the intended write path, so
the `write_u64` calls for `OFF_NEXT_EXEC_SLOT`, `OFF_EXECUTED`, and
`OFF_REMAINING` operate on a `*mut u8`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7404ae9-aebb-49ea-847c-4f3bb5e5d028
📒 Files selected for processing (49)
.github/workflows/ci.yml.gitignoreCargo.tomlMakefileREADME.mdcrates/hydra-api/Cargo.tomlcrates/hydra-api/src/consts.rscrates/hydra-api/src/cpi.rscrates/hydra-api/src/instruction.rscrates/hydra-api/src/lib.rscrates/hydra-api/src/program/helpers.rscrates/hydra-api/src/program/mod.rscrates/hydra-api/src/program/processor.rscrates/hydra-api/src/state.rscrates/hydra-cranker/src/cache.rscrates/hydra-cranker/src/fire.rscrates/hydra-cranker/src/main.rscrates/hydra-cranker/src/mode.rsexamples/anchor/Anchor.tomlexamples/anchor/programs/hydra-example-anchor/Cargo.tomlexamples/anchor/programs/hydra-example-anchor/src/lib.rsexamples/anchor/programs/hydra-example-anchor/tests/mollusk.rsexamples/native/src/lib.rsexamples/native/tests/mollusk.rsexamples/pinocchio/src/lib.rsexamples/pinocchio/tests/mollusk.rsprograms/hydra-ephemeral/Cargo.tomlprograms/hydra-ephemeral/src/entrypoint.rsprograms/hydra-ephemeral/src/lib.rsprograms/hydra-ephemeral/src/processor/cancel.rsprograms/hydra-ephemeral/src/processor/close.rsprograms/hydra-ephemeral/src/processor/common.rsprograms/hydra-ephemeral/src/processor/create.rsprograms/hydra-ephemeral/src/processor/mod.rsprograms/hydra-ephemeral/src/processor/trigger.rsprograms/hydra/Cargo.tomlprograms/hydra/src/entrypoint.rsprograms/hydra/src/lib.rsprograms/hydra/src/processor/cancel.rsprograms/hydra/src/processor/close.rsprograms/hydra/src/processor/common.rsprograms/hydra/src/processor/create.rsprograms/hydra/src/processor/mod.rsprograms/hydra/src/processor/trigger.rstests/e2e/Cargo.tomltests/e2e/tests/ephemeral_cranks.rstests/lib.rstests/programs/noop/Cargo.tomltests/programs/noop/src/lib.rs
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust (rustfmt) | ||
| uses: dtolnay/rust-toolchain@master | ||
| with: | ||
| toolchain: 1.91.1 | ||
| components: rustfmt | ||
|
|
||
| - name: Check formatting | ||
| run: make fmt-check | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Default feature set: lint + tests with the `ephemeral` feature OFF. | ||
| # --------------------------------------------------------------------------- | ||
| default: | ||
| name: lint + test (default) | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust (clippy) | ||
| uses: dtolnay/rust-toolchain@master | ||
| with: | ||
| toolchain: 1.91.1 | ||
| components: clippy,rustfmt | ||
|
|
||
| - name: Install Solana toolchain (build-sbf) | ||
| run: | | ||
| sh -c "$(curl -sSfL https://release.anza.xyz/${SOLANA_VERSION}/install)" | ||
| echo "$HOME/.local/share/solana/install/active_release/bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Install cargo-nextest | ||
| uses: taiki-e/install-action@nextest | ||
|
|
||
| - uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - name: Default CI checks | ||
| run: make ci | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Live end-to-end: boots the real three-process stack (mb-test-validator + | ||
| # ephemeral-validator + hydra-cranker) and asserts ephemeral cranks fire on | ||
| # schedule. The validators ship as the `@magicblock-labs/ephemeral-validator` | ||
| # npm package (mb-test-validator wraps `solana-test-validator`, which the Anza | ||
| # toolchain provides). The `tests/e2e` crate is its own `[workspace]`. | ||
| # --------------------------------------------------------------------------- | ||
| e2e: | ||
| name: e2e (ephemeral cranks, live validators) | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| # Pin to the version the test was developed against. | ||
| EPHEMERAL_VALIDATOR_VERSION: 0.13.3 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust (clippy) | ||
| uses: dtolnay/rust-toolchain@master | ||
| with: | ||
| toolchain: 1.91.1 | ||
| components: clippy | ||
|
|
||
| - name: Install Solana toolchain (build-sbf + solana-test-validator) | ||
| run: | | ||
| sh -c "$(curl -sSfL https://release.anza.xyz/${SOLANA_VERSION}/install)" | ||
| echo "$HOME/.local/share/solana/install/active_release/bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Install Node | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
|
|
||
| - name: Install MagicBlock validators (mb-test-validator + ephemeral-validator) | ||
| run: | | ||
| npm install -g "@magicblock-labs/ephemeral-validator@${EPHEMERAL_VALIDATOR_VERSION}" | ||
| mb-test-validator --version | ||
| ephemeral-validator --version | ||
|
|
||
| - uses: Swatinem/rust-cache@v2 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoff
Consider pinning third-party actions to commit SHAs.
actions/checkout@v4, dtolnay/rust-toolchain@master, taiki-e/install-action@nextest, Swatinem/rust-cache@v2, and actions/setup-node@v4 are all tag/branch refs, which are mutable. Pinning to a commit SHA hardens against supply-chain tampering, at the cost of manual bumps (particularly painful for @master).
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 27-27: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 45-45: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 80-80: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 41-72: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 27-27: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 30-30: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 45-45: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 48-48: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 59-59: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 61-61: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 80-80: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 83-83: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 94-94: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 104-104: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 30-30: action functionality is already included by the runner (superfluous-actions): use rustup and/or cargo in a script step
(superfluous-actions)
[info] 48-48: action functionality is already included by the runner (superfluous-actions): use rustup and/or cargo in a script step
(superfluous-actions)
[info] 83-83: action functionality is already included by the runner (superfluous-actions): use rustup and/or cargo in a script step
(superfluous-actions)
[warning] 100-100: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 27 - 104, The CI workflow still uses
mutable action refs, so update the workflows in ci.yml to pin each third-party
action to an immutable commit SHA instead of tags or branches. Replace the uses
entries for actions/checkout, dtolnay/rust-toolchain, taiki-e/install-action,
Swatinem/rust-cache, and actions/setup-node with their corresponding
commit-pinned versions, and keep the existing step structure and names
unchanged.
Source: Linters/SAST tools
| [dependencies] | ||
| ephemeral-rollups-pinocchio = { workspace = true } | ||
| pinocchio = { workspace = true } | ||
| solana-address = { workspace = true } | ||
| solana-define-syscall = { workspace = true, optional = true } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
ephemeral-rollups-pinocchio is not feature-gated unlike its siblings.
solana-define-syscall and solana-program-error are both declared optional = true and only pulled in via the program/cpi-pinocchio features. ephemeral-rollups-pinocchio is added as a plain dependency, so it will always be compiled for every consumer of hydra-api (including pure base-program/client-only consumers), contradicting the feature-gating intent stated in crates/hydra-api/src/program/mod.rs's doc comment.
♻️ Suggested fix
-ephemeral-rollups-pinocchio = { workspace = true }
+ephemeral-rollups-pinocchio = { workspace = true, optional = true }And add it to the relevant feature (e.g. cpi-pinocchio or a new ephemeral feature) alongside pinocchio/cpi.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/hydra-api/Cargo.toml` around lines 33 - 37,
`ephemeral-rollups-pinocchio` is currently always enabled in `hydra-api` instead
of being gated like the other optional dependencies. Update `Cargo.toml` so this
crate is declared optional and only activated through the appropriate feature in
the same way as `solana-define-syscall` and `solana-program-error`, then wire it
into the relevant feature set in `hydra-api` (for example the `cpi-pinocchio`
path or a dedicated `ephemeral` feature) to match the intent described by
`program/mod.rs`.
| //! ```ignore | ||
| //! use hydra_api::cpi::native as hydra_cpi; | ||
| //! use hydra_api::instruction::{CreateArgs, SchedMeta}; | ||
| //! | ||
| //! // Inside your user-facing instruction's handler: | ||
| //! hydra_cpi::create( | ||
| //! payer_ai, crank_ai, system_program_ai, | ||
| //! &CreateArgs { seed, authority: [0u8; 32], /* ... */ }, | ||
| //! )?; | ||
| //! ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stale doc example path and missing signer_seeds argument.
The example imports hydra_api::cpi::native, but after the base/ephemeral split the path is hydra_api::cpi::base::native. The create(...) call also omits the now-required signer_seeds parameter. Copy-pasting this doc won't compile.
📝 Suggested doc fix
- //! use hydra_api::cpi::native as hydra_cpi;
+ //! use hydra_api::cpi::base::native as hydra_cpi;
//! use hydra_api::instruction::{CreateArgs, SchedMeta};
//!
//! // Inside your user-facing instruction's handler:
//! hydra_cpi::create(
//! payer_ai, crank_ai, system_program_ai,
//! &CreateArgs { seed, authority: [0u8; 32], /* ... */ },
+ //! signer_seeds,
//! )?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! ```ignore | |
| //! use hydra_api::cpi::native as hydra_cpi; | |
| //! use hydra_api::instruction::{CreateArgs, SchedMeta}; | |
| //! | |
| //! // Inside your user-facing instruction's handler: | |
| //! hydra_cpi::create( | |
| //! payer_ai, crank_ai, system_program_ai, | |
| //! &CreateArgs { seed, authority: [0u8; 32], /* ... */ }, | |
| //! )?; | |
| //! ``` | |
| //! |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/hydra-api/src/cpi.rs` around lines 16 - 25, Update the doc example
under hydra_api::cpi::native so it matches the current CPI API: change the
import path to hydra_api::cpi::base::native and adjust the hydra_cpi::create
example to include the required signer_seeds argument. Keep the example aligned
with the current create signature in cpi.rs so the snippet copy-pastes and
compiles.
1 participant
Closes #8
Summary by CodeRabbit
New Features
makecommands and a new CI pipeline.Bug Fixes