Skip to content

Commit

Permalink
Do not eagerly commit transactions to the standalone engine DB
Browse files Browse the repository at this point in the history
  • Loading branch information
@birchmd
birchmd committed Aug 18, 2023
1 parent 880da84 commit f625301
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 22 deletions.
58 changes: 43 additions & 15 deletions engine-standalone-storage/src/sync/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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};

Expand Down Expand Up @@ -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<M: ModExpAlgorithm + 'static>(
storage: &mut Storage,
message: Message,
Expand Down Expand Up @@ -255,19 +260,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,
|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,
Expand All @@ -291,12 +293,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,
|x| x.get_transaction_diff(),
)
});
let (tx_hash, diff, maybe_result) = result.result;
Expand All @@ -309,17 +312,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 =
Expand Down Expand Up @@ -390,7 +399,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,
Expand All @@ -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)
}
Expand All @@ -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<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>>],
Expand Down Expand Up @@ -709,6 +718,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,
Expand All @@ -717,6 +735,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>),
Expand Down
23 changes: 16 additions & 7 deletions engine-tests/src/tests/standalone/sync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) => {
Expand All @@ -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);

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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!(
Expand Down

0 comments on commit f625301

Please sign in to comment.