Skip to content

Conversation

@snawaz
Copy link

@snawaz snawaz commented Oct 29, 2025

This PR adds commit_diff_and_undelegate_accounts() that invokes ScheduleCommitDiffAndUndelegate in the magic program.

I think this PR will eventually be reverted or at least this function will be removed, once I worked on top of magicblock-labs/magicblock-validator#575 and redesign the work eventually removing ScheduleCommitDiffAndUndelegate. But for now, this works great and can stay till the redesign.

Summary by CodeRabbit

  • New Features

    • Introduced a configurable commit policy system enabling users to choose between diff-based and full-byte commit strategies based on operational needs.
    • Added new diff-based commit functionality with undelegation support for enhanced flexibility.
  • Chores

    • Updated workspace dependencies to use local path references.
    • Added development dependencies for testing infrastructure.

Copy link
Author

snawaz commented Oct 29, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@coderabbitai
Copy link

coderabbitai bot commented Oct 29, 2025

Walkthrough

Updates workspace dependencies to use path-based references for two Magicblock crates, introduces a new CommitPolicy enum to control commit strategies (diff-based vs. full-byte), adds a new commit function for diff-based operations with undelegation, and modifies existing commit functions to route through the new policy system.

Changes

Cohort / File(s) Change Summary
Dependency Management
rust/Cargo.toml
Updated magicblock-delegation-program from version 1.1.2 to path ../../delegation-program; updated magicblock-magic-program-api from version 0.2.1 to path ../../magicblock-validator/magicblock-magic-program-api
Commit Policy Implementation
rust/sdk/src/ephem.rs
Introduced CommitPolicy enum (UseDiff, UseFullBytes); added commit_diff_and_undelegate_accounts function; extended create_schedule_commit_ix with commit_policy parameter for instruction variant routing (ScheduleCommitDiffAndUndelegate vs ScheduleCommitAndUndelegate); updated existing commit_accounts and commit_and_undelegate_accounts to pass CommitPolicy::UseFullBytes
Development Dependencies
ts/web3js/package.json
Added ts-jest ^29.1.1 and typescript 5.3.0 as devDependencies

Sequence Diagram

sequenceDiagram
    participant caller as Caller
    participant commit as Commit Functions
    participant policy as CommitPolicy Router
    participant instr as MagicBlockInstruction

    caller->>commit: commit_diff_and_undelegate_accounts()
    commit->>policy: create_schedule_commit_ix(CommitPolicy::UseDiff, allow_undelegation=true)
    activate policy
    policy->>instr: Select ScheduleCommitDiffAndUndelegate
    deactivate policy
    instr-->>caller: Instruction

    caller->>commit: commit_accounts() or commit_and_undelegate_accounts()
    commit->>policy: create_schedule_commit_ix(CommitPolicy::UseFullBytes, ...)
    activate policy
    policy->>instr: Select ScheduleCommitAndUndelegate
    deactivate policy
    instr-->>caller: Instruction

    note over policy: panic if UseDiff + allow_undelegation=false
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • rust/sdk/src/ephem.rs: Verify CommitPolicy enum design and routing logic across create_schedule_commit_ix; ensure panic condition for unsupported UseDiff+non-undelegate path is intentional; confirm all call sites correctly pass the new policy parameter
  • Dependency path references: Validate that path-based dependency references resolve correctly and maintain feature compatibility
  • New public API surface: Review CommitPolicy enum variants and commit_diff_and_undelegate_accounts function signature for consistency with existing patterns

Possibly related issues

Possibly related PRs

Suggested reviewers

  • GabrielePicco

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Add commit_diff_and_undelegate_accounts()" directly and accurately describes the primary change in the pull request. The main objective is to add a new public function called commit_diff_and_undelegate_accounts() that invokes ScheduleCommitDiffAndUndelegate in the magic program, which is exactly what the title conveys. Supporting infrastructure changes like the new CommitPolicy enum and updates to create_schedule_commit_ix are secondary to this main feature addition. The title is concise, specific, and clear enough that a teammate scanning the commit history would immediately understand the primary change being introduced.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch snawaz/commit-diff-and-undelegate

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@snawaz snawaz force-pushed the snawaz/commit-diff-and-undelegate branch from 5a2cfb3 to e8efc4c Compare October 29, 2025 10:35
@snawaz snawaz marked this pull request as ready for review October 31, 2025 04:10
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a903543 and e65be56.

⛔ Files ignored due to path filters (1)
  • rust/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • rust/Cargo.toml (1 hunks)
  • rust/sdk/src/ephem.rs (4 hunks)
  • ts/web3js/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-28T14:28:30.386Z
Learnt from: GabrielePicco
Repo: magicblock-labs/ephemeral-rollups-sdk PR: 69
File: rust/pinocchio/src/instruction/undelegate.rs:22-0
Timestamp: 2025-10-28T14:28:30.386Z
Learning: In the undelegate function in rust/pinocchio/src/instruction/undelegate.rs, the buffer account is intentionally required to be a signer to enforce CPI (Cross-Program Invocation) authorization as part of the security model.

Applied to files:

  • rust/sdk/src/ephem.rs

@snawaz snawaz marked this pull request as draft November 2, 2025 14:41
@snawaz
Copy link
Author

snawaz commented Nov 2, 2025

We do not need this PR anymore, as magicblock-labs/magicblock-validator#575 is redesigned to not introduce ScheduleCommitDiffAndUndelegate instruction.

@snawaz snawaz closed this Nov 2, 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

Development

Successfully merging this pull request may close these issues.

3 participants