Skip to content

Conversation

@taco-paco
Copy link
Contributor

@taco-paco taco-paco commented Oct 2, 2025

Reason

Prior, to form Base layer action users had to include in ER transaction list of accounts they would use in it(Base layer action), now users can form actions using meta information which they could pass as arguments or store for actions in some other account.

Initial apporach had a flaw:
Say user wanted to perform an action: transfer from A to B on base chain. Now say, B is not delegated. This would mean that user would have to explicitly send account B in ER transaction as writable. This would result in him getting an error since we allow only delegated accounts to be writable

Right now user can write:
When validator will send this action to Base chain send account B as writable.

Implementation details

To avoid false hopes from the user instead of AccountMeta we use ShortAccountMeta, where latter doesn't have is_signer field.
Since we can only sign on behalf of validator we don't allow user to provide is_signer flag, meaning that it is always false.

Additionally, some unnecessary copying in magic-program was removed

Greptile Overview

Updated On: 2025-10-02 16:07:56 UTC

Summary

This PR introduces a significant architectural improvement to the MagicBlock validator by allowing users to directly specify account metadata (`ShortAccountMeta`) for base layer actions instead of requiring all accounts to be included in Ephemeral Runtime (ER) transactions. Previously, users faced a critical limitation: when performing actions like transfers to non-delegated accounts, they had to include the destination account as writable in the ER transaction, which would fail validation since only delegated accounts can be writable in ER transactions.

The solution introduces ShortAccountMeta, a simplified version of AccountMeta that excludes the is_signer field (since the validator always signs on behalf of users). This allows users to specify "when you execute this action on the base chain, include account B as writable" without having to make account B writable in the ER transaction itself.

The changes span multiple components:

  • Core API changes: BaseActionArgs now uses Vec<ShortAccountMeta> and Pubkey for destination program instead of indices
  • Argument handling: New ShortAccountMeta struct with proper conversion traits from AccountMeta and AccountInfo
  • Transaction processing: Updated logic to handle direct account metadata instead of account references
  • Code cleanup: Removal of unnecessary copying and consolidation of schedule processing logic
  • Test updates: Integration tests updated to use the new API patterns

The refactoring also includes consolidation of the schedule base intent processing logic, moving functionality from a separate processor module directly into the main processing flow, and cleanup of debug statements and typo fixes in test programs.

Important Files Changed

Changed Files
Filename Score Overview
magicblock-magic-program-api/src/args.rs 4/5 Core API change introducing ShortAccountMeta struct and updating BaseActionArgs to use direct metadata instead of indices
programs/magicblock/src/magic_scheduled_base_intent.rs 4/5 Major refactoring to accept ShortAccountMeta directly and remove account validation during construction
programs/magicblock/src/schedule_transactions/process_schedule_base_intent.rs 4/5 Significant update to handle new account metadata approach and inline undelegation logic
test-integration/programs/flexi-counter/src/processor/schedule_intent.rs 5/5 Updated to use ShortAccountMeta conversions for better base layer action flexibility
test-integration/programs/flexi-counter/src/processor/schedule_redelegation_intent.rs 4/5 Refactored to use ShortAccountMeta instead of AccountInfo clones for redelegation actions
programs/magicblock/src/utils/accounts.rs 4/5 Removed utility function as ShortAccountMeta functionality moved to args module
magicblock-committor-service/src/tasks/mod.rs 5/5 Simple import restructuring to use ShortAccountMeta from args module
test-integration/test-committor-service/tests/test_transaction_preparator.rs 5/5 Updated imports to reflect new ShortAccountMeta location in args module
programs/magicblock/src/schedule_transactions/mod.rs 4/5 Removed schedule base intent processor module as functionality consolidated elsewhere
programs/magicblock/src/schedule_transactions/schedule_base_intent_processor.rs 4/5 Entire file deleted as functionality moved to main processing flow
test-integration/Cargo.toml 4/5 Updated ephemeral-rollups-sdk dependency to support new ShortAccountMeta functionality
test-integration/programs/flexi-counter/src/instruction.rs 5/5 Fixed typo in enum variant name from 'CreateRedelegationIntont' to 'CreateRedelegationIntent'
test-integration/programs/flexi-counter/src/processor.rs 5/5 Fixed matching typo for 'CreateRedelegationIntent' enum variant
test-integration/test-schedule-intent/tests/test_schedule_intents.rs 5/5 Removed debug print statement cleanup

Confidence score: 4/5

  • This PR provides significant UX improvements by solving a critical limitation in base layer actions without introducing breaking changes
  • Score reflects solid architectural design with proper security boundaries (removing is_signer field) and comprehensive test coverage
  • Pay close attention to magicblock-magic-program-api/src/args.rs and programs/magicblock/src/magic_scheduled_base_intent.rs for core API changes

Sequence Diagram

sequenceDiagram
    participant User
    participant "ER Program"
    participant "MagicBlock Program"
    participant "Base Layer"
    participant "DLP (Delegation Program)"

    User->>+"ER Program": "Create intent with ShortAccountMetas"
    "ER Program"->>+"MagicBlock Program": "process_schedule_base_intent(args with ShortAccountMetas)"
    
    "MagicBlock Program"->>+"MagicBlock Program": "BaseAction::try_from_args()"
    Note over "MagicBlock Program": "Validate escrow authority signature"
    Note over "MagicBlock Program": "Create BaseAction with ShortAccountMetas"
    "MagicBlock Program"-->>-"MagicBlock Program": "BaseAction created"
    
    "MagicBlock Program"->>+"MagicBlock Program": "ScheduledBaseIntent::try_new()"
    Note over "MagicBlock Program": "Create scheduled intent with base actions"
    "MagicBlock Program"-->>-"MagicBlock Program": "Intent scheduled"
    
    alt Undelegate Intent
        "MagicBlock Program"->>+DLP: "set_account_owner_to_delegation_program()"
        Note over DLP: "Make accounts immutable in validator"
        DLP-->>-"MagicBlock Program": "Ownership changed"
    end
    
    "MagicBlock Program"->>+"MagicContext": "add_scheduled_action()"
    "MagicContext"-->>-"MagicBlock Program": "Action added"
    "MagicBlock Program"-->>-"ER Program": "Intent scheduled"
    "ER Program"-->>-User: "Transaction confirmed"
    
    Note over "Base Layer": "Later, committor service processes intent"
    
    "Base Layer"->>+"Base Layer": "ArgsTask::BaseAction execution"
    Note over "Base Layer": "Convert ShortAccountMetas to AccountMetas"
    Note over "Base Layer": "is_signer always set to false"
    "Base Layer"->>+"Target Program": "dlp::call_handler with AccountMetas"
    "Target Program"-->>-"Base Layer": "Action executed"
    "Base Layer"-->>-"Base Layer": "Transaction completed"
Loading

@taco-paco taco-paco changed the title Feat: allow users to directly set AccountMeta(shorter version) for base layer action fix: allow users to directly set AccountMeta(shorter version) for base layer action Oct 2, 2025
@github-actions
Copy link

github-actions bot commented Oct 2, 2025

Manual Deploy Available

You can trigger a manual deploy of this PR branch to testnet:

Deploy to Testnet 🚀

Alternative: Comment /deploy on this PR to trigger deployment directly.

⚠️ Note: Manual deploy requires authorization. Only authorized users can trigger deployments.

Comment updated automatically when the PR is synchronized.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

14 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Contributor

@thlorenz thlorenz left a comment

Choose a reason for hiding this comment

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

LGTM aside from the minor comment nit.
I trust that there are enough tests that guarantee the new behavior.

committee.into(),
ShortAccountMeta {
pubkey: *transfer_destination.key,
is_writable: true, // writable on base chain
Copy link
Contributor

@thlorenz thlorenz Oct 2, 2025

Choose a reason for hiding this comment

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

I'm not sure those comments are needed, just put a Rust doc on top of ShortAccountMeta::is_writable that explains that this refers to main chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done

committee.into(),
ShortAccountMeta {
pubkey: *destination_program.key,
is_writable: true, // writable on base chain
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto redundant comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@taco-paco taco-paco merged commit e17d745 into master Oct 3, 2025
6 of 7 checks passed
@taco-paco taco-paco deleted the feat/base-layer-ix/actions-with-account-metas branch October 3, 2025 13:48
thlorenz added a commit that referenced this pull request Oct 6, 2025
* thlorenz/chainlink: (59 commits)
  fix: sync accountsdb slot after ledger replay
  fix: for failed txns use meta defaults that are explorer compatible
  chore: update progress
  chore: rename test to proper name
  chore: allow overriding ephem port while running single test
  chore: all non-skipped ledger tests passing in parallel
  chore: true random port picking for ledger tests
  chore: initial changes to allow running ledger tests in parallel
  feat: ledger test validators start at different ports
  chore: fix compile issue due to merge
  chore: fix merge issues for test-integration
  chore: fix more merge issues to build validator
  chore: fix merge conflicts in cargo lock
  chore: ignore undelegate between restarts test for now
  chore: fix resume strategies test
  chore: fix restore ledger with new keypair test
  chore: fix timestamp ledger test
  chore: fix ledger account flush test
  fix: allow users to directly set AccountMeta(shorter version) for base layer action (#561)
  Hotfix: don't clone and cache not existing accounts (#560)
  ...
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.

4 participants