Fix(standalone): Do not eagerly commit transactions to the DB (#825)

## Description

The [Borealis
refiner](https://github.com/aurora-is-near/borealis-engine-lib/tree/main/refiner-lib)
was experiencing a performance issue where it would become very slow at
processing blocks. Profiling revealed the reason was increasing time to
look up the `ENGINE_STATE` key when replaying transactions (this replay
is necessary to correctly process batch transactions on Near). After
some investigation, we realized the cause of this slow lookup was due to
that key being constantly written and deleted by every transaction. The
reason for this churn is because the Engine logic was changed to
automatically migrate its state, but of course old transactions did not
have that logic and therefore the replay would compute an incorrect
state diff relative to what is reported in the Near block. In such cases
the replay changes to the DB are deleted and the correct diff from the
Near block is used instead.

To avoid this DB churn, this PR changes the standalone engine so that it
will not commit to the DB right away when consuming a block. Instead it
is now up to clients of the standalone engine to commit the changes
themselves (after performing any validation). A PR on Borealis Refiner
will make that change there after this PR is merged.

Note this does not address the larger issue of replay accuracy. In
theory the Borealis Refiner should use the code that existed at the time
when replaying an old transaction. However, this is not so easy to
accomplish which is why we are proposing this solution instead. It is an
immediate fix to the performance issue that can keep the Refiner running
while we address the more fundamental problem.

## Performance / NEAR gas cost considerations

No impact to on-chain Aurora contract; changes to standalone only.

## Testing

Updates to existing tests

engine-standalone-storage/src/sync/mod.rs

@@ -1,3 +1,4 @@
+use crate::engine_state::EngineStateAccess;
 use aurora_engine::parameters::SubmitArgs;
 use aurora_engine::pausables::{
     EnginePrecompilesPauser, PausedPrecompilesManager, PrecompileFlags,
@@ -8,7 +9,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 +25,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 +229,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<M: ModExpAlgorithm + 'static>(
     storage: &mut Storage,
     message: Message,
@@ -255,19 +261,16 @@ pub fn consume_message<M: ModExpAlgorithm + 'static>(
 
             let (tx_hash, diff, result) = storage
                 .with_engine_access(block_height, transaction_position, &[], |io| {
-                    execute_transaction::<M>(
+                    execute_transaction::<_, M, _>(
                         transaction_message.as_ref(),
                         block_height,
                         &block_metadata,
                         engine_account_id,
                         io,
+                        EngineStateAccess::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 +294,13 @@ pub fn execute_transaction_message<M: ModExpAlgorithm + 'static>(
     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::<M>(
+        execute_transaction::<_, M, _>(
             &transaction_message,
             block_height,
             &block_metadata,
             engine_account_id,
             io,
+            EngineStateAccess::get_transaction_diff,
         )
     });
     let (tx_hash, diff, maybe_result) = result.result;
@@ -309,17 +313,23 @@ pub fn execute_transaction_message<M: ModExpAlgorithm + 'static>(
     Ok(outcome)
 }
 
-fn execute_transaction<'db, M: ModExpAlgorithm + 'static>(
+pub fn execute_transaction<I, M, F>(
     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<Option<TransactionExecutionResult>, 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 +400,7 @@ fn execute_transaction<'db, M: ModExpAlgorithm + 'static>(
             (tx_hash, result)
         }
         other => {
-            let result = non_submit_execute::<M>(
+            let result = non_submit_execute::<I, M>(
                 other,
                 io,
                 env,
@@ -401,7 +411,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 +420,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<I: IO + Copy, M: ModExpAlgorithm + 'static>(
     transaction: &TransactionKind,
-    mut io: EngineStateAccess<'db, 'db, 'db>,
+    mut io: I,
     env: env::Fixed,
     relayer_address: Address,
     promise_data: &[Option<Vec<u8>>],
@@ -709,6 +719,15 @@ pub enum ConsumeMessageOutcome {
     TransactionIncluded(Box<TransactionIncludedOutcome>),
 }
 
+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 +736,16 @@ pub struct TransactionIncludedOutcome {
     pub maybe_result: Result<Option<TransactionExecutionResult>, 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<SubmitResult>),

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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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::<AuroraModExp>(
+    let outcome = sync::consume_message::<AuroraModExp>(
         &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!(