Skip to content

Conversation

@GabrielePicco
Copy link
Collaborator

@GabrielePicco GabrielePicco commented Nov 19, 2025

Summary by CodeRabbit

  • New Features

    • Configurable inclusion of inner instructions in transaction simulation responses.
    • Added an observable auto-airdrop toggle so automatic lamport airdrops can be enabled or disabled.
  • Bug Fixes

    • When auto-airdrop is disabled, fee-related failures are now recorded inside execution details (rather than turning the whole simulation into a hard error), improving error visibility.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Walkthrough

Adds a new boolean flag is_auto_airdrop_lamports_enabled to scheduler and executor state, threads it from scheduler into TransactionExecutor, changes gasless processing to record an invalid-fee error inside Executed outcomes only when the flag is disabled, and makes simulate_transaction's RPC response omit inner_instructions unless configured to include them.

Changes

Cohort / File(s) Summary
Simulate transaction RPC
magicblock-aperture/src/requests/http/simulate_transaction.rs
Make inner_instructions conditional on config.inner_instructions: when true map/collect inner instructions into a Vec, otherwise set None; use the computed variable when constructing RpcSimulateTransactionResult.
Scheduler state & wiring
magicblock-api/src/magic_validator.rs, magicblock-processor/src/scheduler/state.rs, test-kit/src/lib.rs
Add public is_auto_airdrop_lamports_enabled: bool to TransactionSchedulerState; initialize it from config.accounts.clone().auto_airdrop_lamports > 0 (or set false in test env).
Executor surface & propagation
magicblock-processor/src/executor/mod.rs
Add public field is_auto_airdrop_lamports_enabled: bool to TransactionExecutor and initialize it from scheduler state in TransactionExecutor::new.
Processing logic (gasless case)
magicblock-processor/src/executor/processing.rs
When undelegated_feepayer_was_modified in gasless mode, only set the status inside an Executed ProcessedTransaction to Err(InvalidAccountForFee) if is_auto_airdrop_lamports_enabled is false; previously the whole result was set to Err(InvalidAccountForFee) unconditionally.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant Aperture as SimulateTransaction handler
  participant Executor
  participant SchedulerState

  Note over Aperture: Simulate RPC flow (changed)
  Client->>Aperture: simulate_request
  Aperture->>Executor: run simulation (collect result)
  alt config.inner_instructions == true
    Executor-->>Aperture: result with inner_instructions Vec
  else
    Executor-->>Aperture: result with inner_instructions = None
  end
  Aperture-->>Client: RpcSimulateTransactionResult
Loading
sequenceDiagram
  autonumber
  participant Scheduler
  participant Executor
  participant Processing

  Note over Scheduler,Executor: New boolean wiring
  Scheduler->>Executor: state (is_auto_airdrop_lamports_enabled)
  Executor->>Processing: process transaction (gasless path)
  alt undelegated_feepayer_was_modified
    alt is_auto_airdrop_lamports_enabled == false
      Processing-->>Executor: Executed(result with status Err(InvalidAccountForFee))
    else
      Processing-->>Executor: Executed(result unchanged) or other result
    end
  else
    Processing-->>Executor: normal outcome
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Check propagation and initialization of the new public flag across modules and tests.
  • Verify the gasless processing change preserves expected semantics for non-Executed variants and doesn't inadvertently hide errors.
  • Validate simulate_transaction types and serialization when inner_instructions is None vs Some(Vec).

Possibly related PRs

Suggested reviewers

  • thlorenz

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 clearly describes both main changes: enabling auto-airdrop to cover rent payments and fixing inner instructions in the RPC simulation response.
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 picco/allow-paying-for-rent-and-rpc-fix

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.

@github-actions
Copy link

github-actions bot commented Nov 19, 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

@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 d0eafe4 and 43652f4.

📒 Files selected for processing (5)
  • magicblock-api/src/magic_validator.rs (1 hunks)
  • magicblock-processor/src/executor/mod.rs (2 hunks)
  • magicblock-processor/src/executor/processing.rs (1 hunks)
  • magicblock-processor/src/scheduler/state.rs (1 hunks)
  • test-kit/src/lib.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-07T13:09:52.253Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: test-kit/src/lib.rs:275-0
Timestamp: 2025-11-07T13:09:52.253Z
Learning: In test-kit, the transaction scheduler in ExecutionTestEnv is not expected to shut down during tests. Therefore, using `.unwrap()` in test helper methods like `schedule_transaction` is acceptable and will not cause issues in the test environment.

Applied to files:

  • test-kit/src/lib.rs
📚 Learning: 2025-10-21T14:00:54.642Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/requests/websocket/account_subscribe.rs:18-27
Timestamp: 2025-10-21T14:00:54.642Z
Learning: In magicblock-aperture account_subscribe handler (src/requests/websocket/account_subscribe.rs), the RpcAccountInfoConfig fields data_slice, commitment, and min_context_slot are currently ignored—only encoding is applied. This is tracked as technical debt in issue #579: https://github.com/magicblock-labs/magicblock-validator/issues/579

Applied to files:

  • magicblock-api/src/magic_validator.rs
  • magicblock-processor/src/executor/processing.rs
📚 Learning: 2025-11-13T09:38:43.804Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/locks.rs:64-102
Timestamp: 2025-11-13T09:38:43.804Z
Learning: In magicblock-processor's TransactionScheduler (scheduler/mod.rs line 59), the executor count is clamped to MAX_SVM_EXECUTORS (63) at initialization time, and executor IDs are assigned sequentially from 0 to count-1. This architectural guarantee ensures that executor IDs used in the bitmask-based AccountLock (scheduler/locks.rs) will always be within valid bounds for bit shifting operations, making runtime bounds checks unnecessary.

Applied to files:

  • magicblock-processor/src/executor/mod.rs
  • magicblock-processor/src/executor/processing.rs
📚 Learning: 2025-10-21T10:34:59.140Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-accounts-db/src/lib.rs:63-72
Timestamp: 2025-10-21T10:34:59.140Z
Learning: In magicblock-validator, the AccountsDb "stop-the-world" synchronizer is managed at the processor/executor level, not at the AccountsDb API level. Transaction executors in magicblock-processor hold a read lock (sync.read()) for the duration of each slot and release it only at slot boundaries, ensuring all account writes happen under the read lock. Snapshot operations acquire a write lock, blocking until all executors release their read locks. This pattern ensures mutual exclusion between writes and snapshots without requiring read guards in AccountsDb write APIs.

Applied to files:

  • magicblock-processor/src/executor/mod.rs
📚 Learning: 2025-11-04T10:53:50.922Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/locks.rs:110-122
Timestamp: 2025-11-04T10:53:50.922Z
Learning: In magicblock-processor, the TransactionScheduler runs in a single, dedicated thread and will always remain single-threaded. The `next_transaction_id()` function in scheduler/locks.rs uses `unsafe static mut` which is safe given this architectural guarantee.

Applied to files:

  • magicblock-processor/src/executor/mod.rs
📚 Learning: 2025-10-14T09:56:14.047Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 564
File: test-integration/programs/flexi-counter/src/processor/call_handler.rs:122-125
Timestamp: 2025-10-14T09:56:14.047Z
Learning: The file test-integration/programs/flexi-counter/src/processor/call_handler.rs contains a test smart contract used for integration testing, not production code.

Applied to files:

  • magicblock-processor/src/executor/processing.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build Project
🔇 Additional comments (4)
magicblock-processor/src/scheduler/state.rs (1)

47-48: LGTM! Clear field addition.

The new boolean flag is well-documented and appropriately scoped as public for cross-crate visibility.

test-kit/src/lib.rs (1)

135-135: LGTM! Appropriate test default.

Disabling auto airdrop in the test environment is a conservative choice that ensures tests explicitly handle account funding scenarios.

magicblock-processor/src/executor/mod.rs (1)

60-61: LGTM! Clean field propagation.

The field is properly documented with helpful context and correctly initialized from the scheduler state.

Also applies to: 108-109

magicblock-processor/src/executor/processing.rs (1)

201-210: LGTM! Correct security guard with auto airdrop bypass.

The conditional logic properly gates the InvalidAccountForFee error based on the auto airdrop flag. When auto airdrop is enabled, undelegated feepayer modifications are permitted (to pay for rent). When disabled, such modifications are treated as security violations.

Note: This error will overwrite any existing status within execution_details, giving the security violation precedence over other potential errors. This is the correct behavior since unauthorized lamport transfers must be blocked regardless of other failure reasons.

Copy link
Contributor

@bmuddha bmuddha left a comment

Choose a reason for hiding this comment

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

LGTM

@GabrielePicco GabrielePicco merged commit 90ac7f8 into master Nov 19, 2025
19 checks passed
@GabrielePicco GabrielePicco deleted the picco/allow-paying-for-rent-and-rpc-fix branch November 19, 2025 15:11
thlorenz added a commit that referenced this pull request Nov 20, 2025
* master:
  fix: allow auto airdrop to pay for rent & simulate inner instructions response (#646)
Dodecahedr0x pushed a commit that referenced this pull request Nov 20, 2025
… response (#646)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Configurable inclusion of inner instructions in transaction simulation
responses.
* Added an observable auto-airdrop toggle so automatic lamport airdrops
can be enabled or disabled.

* **Bug Fixes**
* When auto-airdrop is disabled, fee-related failures are now recorded
inside execution details (rather than turning the whole simulation into
a hard error), improving error visibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
bmuddha pushed a commit that referenced this pull request Nov 20, 2025
… response (#646)

* **New Features**
* Configurable inclusion of inner instructions in transaction simulation
responses.
* Added an observable auto-airdrop toggle so automatic lamport airdrops
can be enabled or disabled.

* **Bug Fixes**
* When auto-airdrop is disabled, fee-related failures are now recorded
inside execution details (rather than turning the whole simulation into
a hard error), improving error visibility.
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