Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Aug 23, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f625301
Do not eagerly commit transactions to the standalone engine DB
birchmd Aug 18, 2023
6fd24af
Clippy fix
birchmd Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 44 additions & 15 deletions engine-standalone-storage/src/sync/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::engine_state::EngineStateAccess;
use aurora_engine::parameters::SubmitArgs;
use aurora_engine::pausables::{
EnginePrecompilesPauser, PausedPrecompilesManager, PrecompileFlags,
Expand All @@ -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,
Expand All @@ -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};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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 =
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand All @@ -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>>],
Expand Down Expand Up @@ -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,
Expand All @@ -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>),
Expand Down
23 changes: 16 additions & 7 deletions engine-tests/src/tests/standalone/sync.rs
View file
Open in desktop
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
Loading