From 06ea6d207c007b888fed4704ed5707fe6b0827ea Mon Sep 17 00:00:00 2001 From: elizabeth Date: Fri, 27 Sep 2024 16:47:10 -0400 Subject: [PATCH] move execution_results and finalize_block from App to object ephemeral store --- crates/astria-sequencer/src/app/mod.rs | 87 +++++++++++++++++--------- 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index bb2d96137..f599c0e82 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -48,6 +48,7 @@ use cnidarium::{ StagedWriteBatch, StateDelta, StateRead, + StateWrite, Storage, }; use prost::Message as _; @@ -120,6 +121,14 @@ use crate::{ transaction::InvalidNonce, }; +// ephemeral store key for the cache of results of executing of transactions in `prepare_proposal`. +// cleared in `process_proposal` if we're the proposer. +const EXECUTION_RESULTS_KEY: &str = "execution_results"; + +// ephemeral store key for the cache of results of executing of transactions in `process_proposal`. +// cleared at the end of the block. +const POST_TRANSACTION_EXECUTION_RESULT_KEY: &str = "post_transaction_execution_result"; + /// The inter-block state being written to by the application. type InterBlockState = Arc>; @@ -162,14 +171,6 @@ pub(crate) struct App { // to the mempool to recost all transactions. recost_mempool: bool, - // cache of results of executing of transactions in `prepare_proposal`. - // cleared in `process_proposal` if we're the proposer. - execution_results: Option>, - - // cache of results of executing of transactions in `process_proposal`. - // cleared at the end of the block. - finalize_block: Option, - // the current `StagedWriteBatch` which contains the rocksdb write batch // of the current block being executed, created from the state delta, // and set after `finalize_block`. @@ -213,8 +214,6 @@ impl App { mempool, validator_address: None, executed_proposal_hash: Hash::default(), - execution_results: None, - finalize_block: None, recost_mempool: false, write_batch: None, app_hash, @@ -290,11 +289,11 @@ impl App { // but `self.state` was changed due to executing the previous round's data. // // if the previous round was committed, then the state stays the same. + // + // this also clears the ephemeral storage. self.state = Arc::new(StateDelta::new(storage.latest_snapshot())); - // clear the cache of transaction execution results - self.execution_results = None; - self.finalize_block = None; + // clear the cached executed proposal hash self.executed_proposal_hash = Hash::default(); } @@ -378,7 +377,7 @@ impl App { // `SequencerBlock` and to set `self.finalize_block`. // // we can't run this in `prepare_proposal` as we don't know the block hash there. - let Some(tx_results) = self.execution_results.take() else { + let Some(tx_results) = self.state.object_get(EXECUTION_RESULTS_KEY) else { bail!("execution results must be present after executing transactions") }; @@ -647,7 +646,11 @@ impl App { self.metrics .set_transactions_in_mempool_total(self.mempool.len().await); - self.execution_results = Some(execution_results); + let mut state_tx = Arc::try_begin_transaction(&mut self.state) + .expect("state Arc should not be referenced elsewhere"); + state_tx.object_put(EXECUTION_RESULTS_KEY, execution_results); + let _ = state_tx.apply(); + Ok((validated_txs, included_signed_txs)) } @@ -857,21 +860,26 @@ impl App { .put_sequencer_block(sequencer_block) .wrap_err("failed to write sequencer block to state")?; - // events that occur after end_block are ignored here; - // there should be none anyways. - let _ = self.apply(state_tx); - - // use a dummy app hash here - the actual app hash will be filled out in finalize_block. - let dummy_app_hash = AppHash::default(); - - let finalize_block = abci::response::FinalizeBlock { + let result = PostTransactionExecutionResult { events: end_block.events, validator_updates: end_block.validator_updates, consensus_param_updates: end_block.consensus_param_updates, - app_hash: dummy_app_hash, tx_results: finalize_block_tx_results, }; - self.finalize_block = Some(finalize_block); + + state_tx.object_put(POST_TRANSACTION_EXECUTION_RESULT_KEY, result); + + // events that occur after end_block are ignored here; + // there should be none anyways. + let _ = self.apply(state_tx); + + assert!( + self.state + .object_get::(POST_TRANSACTION_EXECUTION_RESULT_KEY) + .is_some(), + "failed" + ); + Ok(()) } @@ -964,11 +972,6 @@ impl App { .wrap_err("failed to run post execute transactions handler")?; } - let mut finalize_block = self.finalize_block.take().expect( - "finalize_block result must be present, as txs were already executed just now or \ - during the proposal phase", - ); - // update the priority of any txs in the mempool based on the updated app state if self.recost_mempool { self.metrics.increment_mempool_recosted(); @@ -976,12 +979,26 @@ impl App { update_mempool_after_finalization(&mut self.mempool, &self.state, self.recost_mempool) .await; + let post_transaction_execution_result: PostTransactionExecutionResult = self + .state + .object_get(POST_TRANSACTION_EXECUTION_RESULT_KEY) + .expect( + "post_transaction_execution_result must be present, as txs were already executed \ + just now or during the proposal phase", + ); + // prepare the `StagedWriteBatch` for a later commit. let app_hash = self .prepare_commit(storage) .await .wrap_err("failed to prepare commit")?; - finalize_block.app_hash = app_hash; + let finalize_block = abci::response::FinalizeBlock { + events: post_transaction_execution_result.events, + validator_updates: post_transaction_execution_result.validator_updates, + consensus_param_updates: post_transaction_execution_result.consensus_param_updates, + app_hash, + tx_results: post_transaction_execution_result.tx_results, + }; Ok(finalize_block) } @@ -1237,3 +1254,11 @@ fn signed_transaction_from_bytes(bytes: &[u8]) -> Result { Ok(tx) } + +#[derive(Clone, Debug)] +struct PostTransactionExecutionResult { + events: Vec, + tx_results: Vec, + validator_updates: Vec, + consensus_param_updates: Option, +}