feat: add suppport for ephemeral accounts#12
Conversation
WalkthroughUpdated a crate patch revision and moved account-access validation from pre-execution to post-execution; writable-account checks now allow delegated, undelegating, or ephemeral states and validation failures are recorded on the executed transaction with diagnostic logs instead of short-circuiting. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/transaction_processor.rs`:
- Around line 484-502: The post-execution access check
(`validate_accounts_access`) currently returns early after setting
`executed_tx.execution_details.status` and pushing the processed result, but it
skips extending `balances.post`, leaving `balances.pre` populated and
`balances.post` missing; to fix, ensure you extend `balances.post` for the same
accounts that were used pre-execution before the `continue` (e.g. call the same
routine that populated `balances.post` or move the `balances.post` extension
logic to happen before `validate_accounts_access`), so that
`executed_tx.balances.post` is populated even when `validate_accounts_access`
fails and you still push `ProcessedTransaction::Executed(Box::new(executed_tx))`
into `processing_results`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/access_permissions.rs`:
- Around line 39-47: The logic currently errors when mutation_allowed(acc) is
true, which is inverted; change the condition inside the loop that iterates
self.accounts (skipping index 0) so that it returns
Err(TransactionError::InvalidWritableAccount, *pk) when message.is_writable(i)
&& !mutation_allowed(acc) instead of when mutation_allowed(acc) is true; update
the check that references the mutation_allowed closure and message.is_writable
to negate mutation_allowed(acc) so protected accounts
(delegated/undelegating/ephemeral) cause the error.
bdbaac8 to
cd03eb3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/transaction_processor.rs (1)
483-529:⚠️ Potential issue | 🟠 MajorEnsure access-violation path updates account cache (fees/nonce/intra-batch state).
Line 508-528: when
validate_accounts_accessfails, the account cache is never updated. That leaves fee payer/nonce state stale and can let later txs in the batch see pre-fee balances or reuse nonces. Update the cache regardless; only gate program-cache merge on success.🔧 Suggested fix
- let result = if let Err((err, offender)) = result { + if let Err((err, offender)) = result { // If an account access violation was detected, we construct extra // log message, and replace the status, so that the transaction will // be persisted to the ledger with some useful debug information executed_tx.execution_details.status = Err(err); let logs = executed_tx .execution_details .log_messages .get_or_insert_default(); let msg = format!("Account {offender} was illegally used as writeable"); logs.push(msg); - ProcessedTransaction::Executed(Box::new(executed_tx)) - } else { - // Update loaded accounts cache with account states which might have changed. - // Also update local program cache with modifications made by the transaction, - // if it executed successfully. - account_loader.update_accounts_for_executed_tx(tx, &executed_tx); - if executed_tx.was_successful() { - program_cache_for_tx_batch.merge(&executed_tx.programs_modified_by_tx); - } - ProcessedTransaction::Executed(Box::new(executed_tx)) - }; + } + // Update loaded accounts cache with account states which might have changed. + account_loader.update_accounts_for_executed_tx(tx, &executed_tx); + // Only merge program cache on successful execution. + if executed_tx.was_successful() { + program_cache_for_tx_batch.merge(&executed_tx.programs_modified_by_tx); + } + let result = ProcessedTransaction::Executed(Box::new(executed_tx));
This PR moves the pre-execution check to the post - execution. This is necessary to allow the ephemeral accounts (which are not delegated) to be mutated (created) first. In addition ephmeral accounts are also alowed to be writeable (while not being delegated).
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores