Skip to content

Commit

Permalink
Fix transaction finalization after cancel
Browse files Browse the repository at this point in the history
  • Loading branch information
anton-lisanin committed Nov 14, 2024
1 parent 6ed60b1 commit f8a24eb
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 24 deletions.
8 changes: 7 additions & 1 deletion evm_loader/program/src/account/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,13 @@ impl<'a> StateAccount<'a> {
}

pub fn finish_scheduled_tx(self, program_id: &Pubkey) -> Result<()> {
super::validate_tag(program_id, &self.account, TAG_SCHEDULED_STATE_FINALIZED)?;
let tag = super::tag(program_id, &self.account)?;
let is_finalized = tag == TAG_SCHEDULED_STATE_FINALIZED;
let is_canceled = tag == TAG_SCHEDULED_STATE_CANCELLED;
if !(is_finalized || is_canceled) {
return Err(Error::StorageAccountInvalidTag(*self.account.key, tag));
}

debug_print!(
"Finalize State {} for scheduled transaction",
self.account.key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ pub fn process<'a>(
let system = System::from_account(&accounts[5])?;

// Validate Transaction
let tx = ScheduledTxShell::from_rlp(messsage).map_err(|_| Error::TreeAccountTxInvalidType)?;
let tx = ScheduledTxShell::from_rlp(messsage)?;
let tx_hash = tx.hash;

let payer = Address::from_solana_address(signer.key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ fn validate<'a>(
let trx_tree_account = state
.tree_account()
.expect("Unreachable code path: validation in the State Account contains a bug.");

let actual_tree_pubkey = *tree.info().key;
if trx_tree_account != actual_tree_pubkey {
return Err(Error::ScheduledTxInvalidTreeAccount(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ pub fn do_scheduled_start<'a>(

let origin = storage.trx_origin();

storage.trx().validate(origin, &account_storage)?;
storage
.trx()
.validate(origin, &account_storage, Some(&transaction_tree))?;

// Increment origin's nonce only once for the whole execution tree.
let mut origin_account = account_storage.origin(origin, storage.trx())?;
Expand Down
4 changes: 2 additions & 2 deletions evm_loader/program/src/instruction/transaction_execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub fn execute(
let mut account_storage = ProgramAccountStorage::new(accounts)?;
let mut backend_data = ExecutorStateData::new(&account_storage);

trx.validate(origin, &account_storage)?;
trx.validate(origin, &account_storage, None)?;

account_storage.origin(origin, &trx)?.increment_nonce()?;

Expand Down Expand Up @@ -62,7 +62,7 @@ pub fn execute_with_solana_call(
) -> Result<()> {
let mut account_storage = ProgramAccountStorage::new(accounts)?;

trx.validate(origin, &account_storage)?;
trx.validate(origin, &account_storage, None)?;

account_storage.origin(origin, &trx)?.increment_nonce()?;

Expand Down
2 changes: 1 addition & 1 deletion evm_loader/program/src/instruction/transaction_step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn do_begin<'a>(

let origin = storage.trx_origin();

storage.trx().validate(origin, &account_storage)?;
storage.trx().validate(origin, &account_storage, None)?;

// Increment origin nonce in the first iteration
// This allows us to run multiple iterative transactions from the same sender in parallel
Expand Down
35 changes: 17 additions & 18 deletions evm_loader/program/src/types/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize};
use solana_program::instruction::{get_stack_height, TRANSACTION_LEVEL_STACK_HEIGHT};
use std::convert::TryInto;

use crate::account::TransactionTree;
use crate::types::vector::VectorVecExt;
use crate::{
account_storage::AccountStorage, config::GAS_LIMIT_MULTIPLIER_NO_CHAINID, error::Error, vector,
Expand Down Expand Up @@ -1074,6 +1075,7 @@ impl Transaction {
&self,
origin: Address,
backend: &impl AccountStorage,
tree: Option<&TransactionTree<'_>>,
) -> Result<(), crate::error::Error> {
let chain_id = self
.chain_id()
Expand All @@ -1083,36 +1085,33 @@ impl Transaction {
return Err(Error::InvalidChainId(chain_id));
}

if tree.is_some() != self.is_scheduled_tx() {
return Err(Error::TreeAccountTxInvalidType);
}

// Nonce validation is slightly different for classic and scheduled transactions.
//
// Classic transactions:
// origin's nonce should be equal to txn's nonce because it's validated during
// the first iteration and then incremented.
//
// Scheduled transactions:
// payer's nonce (origin) can be greater than what's stored in the transaction because payer
// is the same for the whole scheduled execution tree. However, payer's nonce is also
// incremented only once - at the start of the first scheduled txn.
// payer's nonce (origin) validated only for the first transaction in the tree
let origin_nonce = backend.nonce(origin, chain_id).await;
#[allow(clippy::collapsible_else_if)]
if self.is_scheduled_tx() {
if origin_nonce < self.nonce() {
let error = Error::InvalidTransactionNonce(origin, origin_nonce, self.nonce());
return Err(error);
}
} else {
if origin_nonce != self.nonce() {
let error = Error::InvalidTransactionNonce(origin, origin_nonce, self.nonce());
return Err(error);
}

let validate_nonce = tree.map_or(true, TransactionTree::is_not_started);
if validate_nonce && (origin_nonce != self.nonce()) {
let error = Error::InvalidTransactionNonce(origin, origin_nonce, self.nonce());
return Err(error);
}

// The reason to forbid the calls for DynamicFee transactions - priority fee calculation
// uses get_processed_sibling_instruction syscall which doesn't work well for CPI.
if self.tx_type() == 2 && get_stack_height() != TRANSACTION_LEVEL_STACK_HEIGHT {
return Err(Error::Custom(
"CPI calls of Neon EVM are forbidden for DynamicFee transaction type.".to_owned(),
));
let is_root_transaction = get_stack_height() == TRANSACTION_LEVEL_STACK_HEIGHT;
if matches!(self.tx_type(), 2 | 0x80) && !is_root_transaction {
return Err(
"CPI calls of Neon EVM are forbidden for DynamicFee transaction type.".into(),
);
}

Ok(())
Expand Down

0 comments on commit f8a24eb

Please sign in to comment.