feat: ephemeral cranks - #9
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds the ChangesEphemeral Rollup Crank Feature
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
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 |
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
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
programs/hydra-ephemeral/src/processor/trigger.rs (1)
88-94: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUse checked arithmetic for
executed.
executed + 1can overflow when the persisted counter reachesu64::MAX, aborting the trigger instead of returning a program error.Proposed fix
+ let next_executed = executed + .checked_add(1) + .ok_or(ProgramError::ArithmeticOverflow)?; { let mut data = crank_ai.try_borrow_mut()?; let s = unsafe { load_crank_mut(&mut data)? }; s.set_next_exec_slot(next_slot); - s.set_executed(executed + 1); + s.set_executed(next_executed);🤖 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-ephemeral/src/processor/trigger.rs` around lines 88 - 94, Update the counter increment in the crank state mutation block using checked arithmetic for executed + 1, and propagate the overflow as the processor’s existing program error type before calling set_executed. Preserve normal increment behavior for values below u64::MAX and leave the remaining counter logic unchanged.programs/hydra/src/processor/trigger.rs (1)
112-115: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
data_mut_ptr()for these writes.data_ptr()returns*const u8, so thiswrite_u64path needs a mutable pointer or mutable borrow instead.🤖 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 - 115, Update the unsafe write block in the trigger processing flow to obtain the pointer via crank_ai.data_mut_ptr() instead of data_ptr(), while preserving the existing OFF_NEXT_EXEC_SLOT and OFF_EXECUTED writes.Source: MCP tools
🤖 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.
Outside diff comments:
In `@programs/hydra-ephemeral/src/processor/trigger.rs`:
- Around line 88-94: Update the counter increment in the crank state mutation
block using checked arithmetic for executed + 1, and propagate the overflow as
the processor’s existing program error type before calling set_executed.
Preserve normal increment behavior for values below u64::MAX and leave the
remaining counter logic unchanged.
In `@programs/hydra/src/processor/trigger.rs`:
- Around line 112-115: Update the unsafe write block in the trigger processing
flow to obtain the pointer via crank_ai.data_mut_ptr() instead of data_ptr(),
while preserving the existing OFF_NEXT_EXEC_SLOT and OFF_EXECUTED writes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 088f3920-5be2-4fe7-a2be-c52d7ee287e5
📒 Files selected for processing (8)
programs/hydra-ephemeral/src/processor/cancel.rsprograms/hydra-ephemeral/src/processor/close.rsprograms/hydra-ephemeral/src/processor/create.rsprograms/hydra-ephemeral/src/processor/trigger.rsprograms/hydra/src/processor/cancel.rsprograms/hydra/src/processor/close.rsprograms/hydra/src/processor/create.rsprograms/hydra/src/processor/trigger.rs
7897dd1 to
07d584a
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/hydra-cranker/src/cache.rs (2)
172-196: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftProtect optimistic trigger advancement from stale subscription snapshots.
The cache advances before transaction confirmation, but subscription updates can still apply an older account snapshot. This can restore the previous schedule and submit the same crank again.
crates/hydra-cranker/src/cache.rs#L172-L196: Add monotonic snapshot validation or a pending-trigger barrier before replacing an optimistically advanced entry.crates/hydra-cranker/src/main.rs#L25-L31: Keep the one-slot cooldown only after stale snapshot reconciliation is enforced.crates/hydra-cranker/src/main.rs#L350-L353: Reconcile the optimistic update with subscription or confirmed RPC state before allowing another trigger.🤖 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-cranker/src/cache.rs` around lines 172 - 196, Protect advance_after_trigger in crates/hydra-cranker/src/cache.rs:172-196 from older subscription snapshots by adding monotonic snapshot validation or a pending-trigger barrier before replacing an optimistically advanced cache entry. In crates/hydra-cranker/src/main.rs:25-31, retain the one-slot cooldown only after stale-snapshot reconciliation is enforced. In crates/hydra-cranker/src/main.rs:350-353, reconcile the optimistic advancement with subscription or confirmed RPC state before permitting another trigger.
29-32: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep existing crank accounts compatible before changing the decoder layout.
CrankEntry::from_rawnow decodesinterval_slotsat[72..80], but the on-chainCrankstate plusTrigger/Closestill use the previous layout. If old crank accounts can remain on chain, add a layout version or migration; otherwise old data decodes with the wrong schedule.🤖 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-cranker/src/cache.rs` around lines 29 - 32, Update CrankEntry::from_raw and the associated on-chain Crank/Trigger/Close layout handling so existing crank accounts remain decodable after adding interval_slots at [72..80]. Add an explicit layout version or migration path that distinguishes legacy accounts and preserves their prior schedule, while using the new layout only for upgraded accounts.
🤖 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.
Outside diff comments:
In `@crates/hydra-cranker/src/cache.rs`:
- Around line 172-196: Protect advance_after_trigger in
crates/hydra-cranker/src/cache.rs:172-196 from older subscription snapshots by
adding monotonic snapshot validation or a pending-trigger barrier before
replacing an optimistically advanced cache entry. In
crates/hydra-cranker/src/main.rs:25-31, retain the one-slot cooldown only after
stale-snapshot reconciliation is enforced. In
crates/hydra-cranker/src/main.rs:350-353, reconcile the optimistic advancement
with subscription or confirmed RPC state before permitting another trigger.
- Around line 29-32: Update CrankEntry::from_raw and the associated on-chain
Crank/Trigger/Close layout handling so existing crank accounts remain decodable
after adding interval_slots at [72..80]. Add an explicit layout version or
migration path that distinguishes legacy accounts and preserves their prior
schedule, while using the new layout only for upgraded accounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: df214b72-aae9-4490-baa2-8beb6bb72021
📒 Files selected for processing (2)
crates/hydra-cranker/src/cache.rscrates/hydra-cranker/src/main.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/hydra-cranker/src/main.rs (1)
350-356: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDefer cache advancement to signature confirmation.
fire_triggeronly pollsget_signature_status()whenskip_preflightis true; when preflight is enabled, itsOk(())path can run aftersend_transaction_with_configacceptance rather than confirmation. Advanceentry.pubkeyinadvance_after_triggeronly after an authoritative success/status, or defer until the subscription echo arrives.🤖 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-cranker/src/main.rs` around lines 350 - 356, Remove the immediate cache::advance_after_trigger call from the fire_trigger success path and defer advancing entry.pubkey until an authoritative successful signature status is observed or the programSubscribe echo arrives. Ensure send_transaction_with_config returning Ok(()) alone cannot advance the cache or clear the failure record.
🤖 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 `@tests/e2e/tests/ephemeral_cranks.rs`:
- Line 52: Update the cleanup flow in the ephemeral crank tests after wait_until
joins the reader thread: explicitly release the ManuallyDrop-wrapped _sub with
bounded cleanup so each scenario closes its WebSocket and subscription instead
of retaining PubsubLogsClientSubscription until process exit.
---
Outside diff comments:
In `@crates/hydra-cranker/src/main.rs`:
- Around line 350-356: Remove the immediate cache::advance_after_trigger call
from the fire_trigger success path and defer advancing entry.pubkey until an
authoritative successful signature status is observed or the programSubscribe
echo arrives. Ensure send_transaction_with_config returning Ok(()) alone cannot
advance the cache or clear the failure record.
🪄 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 Plus
Run ID: e355afd8-6189-48d9-81ae-3a883c7dabe9
📒 Files selected for processing (5)
Makefilecrates/hydra-api/src/consts.rscrates/hydra-cranker/src/cache.rscrates/hydra-cranker/src/main.rstests/e2e/tests/ephemeral_cranks.rs
💤 Files with no reviewable changes (1)
- crates/hydra-api/src/consts.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/hydra-cranker/src/main.rs (1)
352-355: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSuppress a terminal crank until the cache reconciles it.
If the final trigger lands but
programSubscribedoes not update the cache before the next interval, Line 355 advances onlynext_exec_slot. The entry still hasremaining == 1, as specified bycrates/hydra-cranker/src/cache.rsLines 295-304. The cranker then submits another trigger for an exhausted crank after the one-slot cooldown.Mark a final execution as pending and exclude it from eligibility until an account update or bounded RPC reconciliation resolves the submission. Keep a retry path for submissions that did not land.
🤖 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-cranker/src/main.rs` around lines 352 - 355, Update the trigger handling around cache::advance_after_trigger so a final execution with remaining == 1 is marked pending and excluded from eligibility until programSubscribe or bounded RPC reconciliation confirms the account state; preserve a retry path when the submission does not land, while leaving non-terminal interval advancement unchanged.crates/hydra-api/src/cpi.rs (1)
183-190: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass
signer_seedsin the ephemeral CPI example.
hydra_api::cpi::ephemeral::native::createrequiressigner_seeds, but the example stops afterCreateArgs.📝 Proposed fix
//! sponsor_ai, crank_ai, vault_ai, magic_program_ai, //! &CreateArgs { seed, authority: [0u8; 32], /* ... */ }, + //! &signer_seeds, //! )?;🤖 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 183 - 190, Update the ephemeral native create example around hydra_cpi::create to include the required signer_seeds argument after the CreateArgs argument, using the appropriate seed-slice representation while preserving the existing example flow.crates/hydra-api/src/instruction.rs (1)
75-182: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign the Create serializer with the stored crank tail layout.
create::write_toandcreate::body_lencurrently write each scheduled ix as[u8 num_accounts][u16 data_len][program_id][metas][data], butscheduled_ixs_from_crankreads the on-chain tail as[u16 num_accounts][metas][program_id][u16 data_len][data], andprocessor::measure_region/write_tailalso use that crank layout. This width/order mismatch breaksTrigger.followupbyte matching, so either the crank parser needs to match the serializer or the serializer needs to emit and countu16 num_accountsfirst.🤖 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/instruction.rs` around lines 75 - 182, Align CreateArgs::write_to and CreateArgs::body_len with the crank tail layout consumed by scheduled_ixs_from_crank, processor::measure_region, and write_tail: encode each scheduled instruction as a u16 account count, metas, program_id, u16 data length, then data. Update the size calculation to include the u16 count and place all fields in that same order so Trigger.followup byte matching remains consistent.
🤖 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.
Outside diff comments:
In `@crates/hydra-api/src/cpi.rs`:
- Around line 183-190: Update the ephemeral native create example around
hydra_cpi::create to include the required signer_seeds argument after the
CreateArgs argument, using the appropriate seed-slice representation while
preserving the existing example flow.
In `@crates/hydra-api/src/instruction.rs`:
- Around line 75-182: Align CreateArgs::write_to and CreateArgs::body_len with
the crank tail layout consumed by scheduled_ixs_from_crank,
processor::measure_region, and write_tail: encode each scheduled instruction as
a u16 account count, metas, program_id, u16 data length, then data. Update the
size calculation to include the u16 count and place all fields in that same
order so Trigger.followup byte matching remains consistent.
In `@crates/hydra-cranker/src/main.rs`:
- Around line 352-355: Update the trigger handling around
cache::advance_after_trigger so a final execution with remaining == 1 is marked
pending and excluded from eligibility until programSubscribe or bounded RPC
reconciliation confirms the account state; preserve a retry path when the
submission does not land, while leaving non-terminal interval advancement
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db449768-4719-479d-836a-92370d7037cf
📒 Files selected for processing (9)
crates/hydra-api/Cargo.tomlcrates/hydra-api/src/cpi.rscrates/hydra-api/src/instruction.rscrates/hydra-cranker/Cargo.tomlcrates/hydra-cranker/src/cache.rscrates/hydra-cranker/src/fire.rscrates/hydra-cranker/src/main.rstests/e2e/Cargo.tomltests/e2e/tests/ephemeral_cranks.rs
1 participant
Closes #8
Summary by CodeRabbit
New Features
--ephemeral.Bug Fixes
Documentation
Tests