From f62530155c2b777679e63da256fe97cacde4e076 Mon Sep 17 00:00:00 2001 From: Michael Birch Date: Fri, 18 Aug 2023 15:11:44 +0200 Subject: [PATCH] Do not eagerly commit transactions to the standalone engine DB --- engine-standalone-storage/src/sync/mod.rs | 58 +++++++++++++++++------ engine-tests/src/tests/standalone/sync.rs | 23 ++++++--- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/engine-standalone-storage/src/sync/mod.rs b/engine-standalone-storage/src/sync/mod.rs index 407804dc8..6e04723b8 100644 --- a/engine-standalone-storage/src/sync/mod.rs +++ b/engine-standalone-storage/src/sync/mod.rs @@ -8,7 +8,10 @@ use aurora_engine::{ state, xcc, }; use aurora_engine_modexp::ModExpAlgorithm; -use aurora_engine_sdk::env::{self, Env, DEFAULT_PREPAID_GAS}; +use aurora_engine_sdk::{ + env::{self, Env, DEFAULT_PREPAID_GAS}, + io::IO, +}; use aurora_engine_transactions::EthTransactionKind; use aurora_engine_types::{ account_id::AccountId, @@ -21,7 +24,6 @@ use std::{io, str::FromStr}; pub mod types; -use crate::engine_state::EngineStateAccess; use crate::{error::ParseTransactionKindError, BlockMetadata, Diff, Storage}; use types::{Message, TransactionKind, TransactionKindTag, TransactionMessage}; @@ -226,6 +228,9 @@ pub fn parse_transaction_kind( Ok(tx_kind) } +/// Note: this function does not automatically commit transaction messages to the storage. +/// If you want the transaction diff committed then you must call the `commit` method on +/// the outcome of this function. pub fn consume_message( storage: &mut Storage, message: Message, @@ -255,19 +260,16 @@ pub fn consume_message( let (tx_hash, diff, result) = storage .with_engine_access(block_height, transaction_position, &[], |io| { - execute_transaction::( + execute_transaction::<_, M, _>( transaction_message.as_ref(), block_height, &block_metadata, engine_account_id, io, + |x| x.get_transaction_diff(), ) }) .result; - match result.as_ref() { - Err(_) | Ok(Some(TransactionExecutionResult::Submit(Err(_)))) => (), // do not persist if Engine encounters an error - _ => storage.set_transaction_included(tx_hash, &transaction_message, &diff)?, - } let outcome = TransactionIncludedOutcome { hash: tx_hash, info: *transaction_message, @@ -291,12 +293,13 @@ pub fn execute_transaction_message( let block_metadata = storage.get_block_metadata(block_hash)?; let engine_account_id = storage.get_engine_account_id()?; let result = storage.with_engine_access(block_height, transaction_position, &[], |io| { - execute_transaction::( + execute_transaction::<_, M, _>( &transaction_message, block_height, &block_metadata, engine_account_id, io, + |x| x.get_transaction_diff(), ) }); let (tx_hash, diff, maybe_result) = result.result; @@ -309,17 +312,23 @@ pub fn execute_transaction_message( Ok(outcome) } -fn execute_transaction<'db, M: ModExpAlgorithm + 'static>( +pub fn execute_transaction( transaction_message: &TransactionMessage, block_height: u64, block_metadata: &BlockMetadata, engine_account_id: AccountId, - io: EngineStateAccess<'db, 'db, 'db>, + io: I, + get_diff: F, ) -> ( H256, Diff, Result, error::Error>, -) { +) +where + I: IO + Copy, + M: ModExpAlgorithm + 'static, + F: FnOnce(&I) -> Diff, +{ let signer_account_id = transaction_message.signer.clone(); let predecessor_account_id = transaction_message.caller.clone(); let relayer_address = @@ -390,7 +399,7 @@ fn execute_transaction<'db, M: ModExpAlgorithm + 'static>( (tx_hash, result) } other => { - let result = non_submit_execute::( + let result = non_submit_execute::( other, io, env, @@ -401,7 +410,7 @@ fn execute_transaction<'db, M: ModExpAlgorithm + 'static>( } }; - let diff = io.get_transaction_diff(); + let diff = get_diff(&io); (tx_hash, diff, result) } @@ -410,9 +419,9 @@ fn execute_transaction<'db, M: ModExpAlgorithm + 'static>( /// The `submit` transaction kind is special because it is the only one where the transaction hash /// differs from the NEAR receipt hash. #[allow(clippy::too_many_lines)] -fn non_submit_execute<'db, M: ModExpAlgorithm + 'static>( +fn non_submit_execute( transaction: &TransactionKind, - mut io: EngineStateAccess<'db, 'db, 'db>, + mut io: I, env: env::Fixed, relayer_address: Address, promise_data: &[Option>], @@ -709,6 +718,15 @@ pub enum ConsumeMessageOutcome { TransactionIncluded(Box), } +impl ConsumeMessageOutcome { + pub fn commit(&self, storage: &mut Storage) -> Result<(), crate::error::Error> { + if let Self::TransactionIncluded(x) = self { + x.commit(storage)?; + } + Ok(()) + } +} + #[derive(Debug)] pub struct TransactionIncludedOutcome { pub hash: aurora_engine_types::H256, @@ -717,6 +735,16 @@ pub struct TransactionIncludedOutcome { pub maybe_result: Result, error::Error>, } +impl TransactionIncludedOutcome { + pub fn commit(&self, storage: &mut Storage) -> Result<(), crate::error::Error> { + match self.maybe_result.as_ref() { + Err(_) | Ok(Some(TransactionExecutionResult::Submit(Err(_)))) => (), // do not persist if Engine encounters an error + _ => storage.set_transaction_included(self.hash, &self.info, &self.diff)?, + }; + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum TransactionExecutionResult { Submit(engine::EngineResult), diff --git a/engine-tests/src/tests/standalone/sync.rs b/engine-tests/src/tests/standalone/sync.rs index 9371bdb57..681ecc252 100644 --- a/engine-tests/src/tests/standalone/sync.rs +++ b/engine-tests/src/tests/standalone/sync.rs @@ -67,6 +67,7 @@ fn test_consume_deposit_message() { sync::ConsumeMessageOutcome::TransactionIncluded(outcome) => outcome, other => panic!("Unexpected outcome {other:?}"), }; + outcome.commit(&mut runner.storage).unwrap(); let finish_deposit_args = match outcome.maybe_result.unwrap().unwrap() { sync::TransactionExecutionResult::Promise(promise_args) => { @@ -99,6 +100,7 @@ fn test_consume_deposit_message() { sync::ConsumeMessageOutcome::TransactionIncluded(outcome) => outcome, other => panic!("Unexpected outcome {other:?}"), }; + outcome.commit(&mut runner.storage).unwrap(); let ft_on_transfer_args = match outcome.maybe_result.unwrap().unwrap() { sync::TransactionExecutionResult::Promise(promise_args) => { @@ -120,11 +122,12 @@ fn test_consume_deposit_message() { promise_data: Vec::new(), }; - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); assert_eq!(runner.get_balance(&recipient_address), deposit_amount); @@ -150,11 +153,12 @@ fn test_consume_deploy_message() { promise_data: Vec::new(), }; - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); let diff = runner .storage @@ -203,11 +207,12 @@ fn test_consume_deploy_erc20_message() { }; // Deploy ERC-20 (this would be the flow for bridging a new NEP-141 to Aurora) - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); let erc20_address = runner .storage @@ -241,11 +246,12 @@ fn test_consume_deploy_erc20_message() { }; // Mint new tokens (via ft_on_transfer flow, same as the bridge) - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); // Check balance is correct let deployed_token = ERC20( @@ -297,11 +303,12 @@ fn test_consume_ft_on_transfer_message() { promise_data: Vec::new(), }; - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); assert_eq!( runner.get_balance(&dest_address).raw().low_u128(), @@ -341,11 +348,12 @@ fn test_consume_call_message() { promise_data: Vec::new(), }; - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); assert_eq!(runner.get_balance(&recipient_address), transfer_amount); assert_eq!( @@ -391,11 +399,12 @@ fn test_consume_submit_message() { promise_data: Vec::new(), }; - sync::consume_message::( + let outcome = sync::consume_message::( &mut runner.storage, sync::types::Message::Transaction(Box::new(transaction_message)), ) .unwrap(); + outcome.commit(&mut runner.storage).unwrap(); assert_eq!(runner.get_balance(&recipient_address), transfer_amount); assert_eq!(