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

chore(sequencer): remove references to outdated ABCI methods #1826

Open
wants to merge 4 commits into
base: ENG-995/refactor_sequencer_app
Choose a base branch
from
+184 −212
Open

chore(sequencer): remove references to outdated ABCI methods #1826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1662c72
rename begin/end_block, remove deprecated ABCI types
ethanoroshiba Nov 19, 2024
d0169bc
fix test, remove proposer address from begin_block, name changes
ethanoroshiba Nov 20, 2024
6aecbcd
move fee payment to fees component
ethanoroshiba Nov 20, 2024
0629d4f
Merge branch 'ENG-995/refactor_sequencer_app' into ENG-1006/begin_and…
ethanoroshiba Dec 5, 2024
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
20 changes: 9 additions & 11 deletions crates/astria-sequencer/src/accounts/component.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@ use astria_eyre::eyre::{
Result,
WrapErr as _,
};
use tendermint::abci::request::{
BeginBlock,
EndBlock,
};
use tracing::instrument;

use crate::{
accounts,
assets,
component::Component,
component::{
Component,
PrepareStateInfo,
},
};

#[derive(Default)]
Expand Down Expand Up @@ -48,18 +47,17 @@ impl Component for AccountsComponent {
Ok(())
}

#[instrument(name = "AccountsComponent::begin_block", skip_all)]
async fn begin_block<S: accounts::StateWriteExt + 'static>(
#[instrument(name = "AccountsComponent::prepare_state_for_tx_execution", skip_all)]
async fn prepare_state_for_tx_execution<S: accounts::StateWriteExt + 'static>(
_state: &mut Arc<S>,
_begin_block: &BeginBlock,
_prepare_state_info: &PrepareStateInfo,
) -> Result<()> {
Ok(())
}

#[instrument(name = "AccountsComponent::end_block", skip_all)]
async fn end_block<S: accounts::StateWriteExt + 'static>(
#[instrument(name = "AccountsComponent::handle_post_tx_execution", skip_all)]
async fn handle_post_tx_execution<S: accounts::StateWriteExt + 'static>(
_state: &mut Arc<S>,
_end_block: &EndBlock,
) -> Result<()> {
Ok(())
}
Expand Down
161 changes: 52 additions & 109 deletions crates/astria-sequencer/src/app/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ use tendermint::{
Event,
},
account,
block::Header,
AppHash,
Hash,
};
Expand All @@ -87,10 +86,7 @@ pub(crate) use self::state_ext::{
StateWriteExt,
};
use crate::{
accounts::{
component::AccountsComponent,
StateWriteExt as _,
},
accounts::component::AccountsComponent,
action_handler::{
impls::transaction::InvalidNonce,
ActionHandler as _,
Expand All @@ -109,11 +105,11 @@ use crate::{
StateReadExt as _,
StateWriteExt as _,
},
component::Component as _,
fees::{
component::FeesComponent,
StateReadExt as _,
component::{
Component as _,
PrepareStateInfo,
},
fees::component::FeesComponent,
grpc::StateWriteExt as _,
ibc::component::IbcComponent,
mempool::{
Expand Down Expand Up @@ -206,8 +202,8 @@ pub(crate) struct App {

// This is set to the executed hash of the proposal during `process_proposal`
//
// If it does not match the hash given during `begin_block`, then we clear and
// reset the execution results cache + state delta. Transactions are re-executed.
// If it does not match the hash given during `prepare_state_for_tx_execution`, then we clear
// and reset the execution results cache + state delta. Transactions are re-executed.
// If it does match, we utilize cached results to reduce computation.
//
// Resets to default hash at the beginning of `prepare_proposal`, and `process_proposal` if
Expand Down Expand Up @@ -376,7 +372,6 @@ impl App {
height: prepare_proposal.height,
time: prepare_proposal.time,
next_validators_hash: prepare_proposal.next_validators_hash,
proposer_address: prepare_proposal.proposer_address,
};

self.pre_execute_transactions(block_data)
Expand Down Expand Up @@ -478,7 +473,6 @@ impl App {
height: process_proposal.height,
time: process_proposal.time,
next_validators_hash: process_proposal.next_validators_hash,
proposer_address: process_proposal.proposer_address,
};

self.pre_execute_transactions(block_data)
Expand Down Expand Up @@ -681,7 +675,8 @@ impl App {
}

/// sets up the state for execution of the block's transactions.
/// set the current height and timestamp, and calls `begin_block` on all components.
/// set the current height and timestamp, and calls `prepare_state_for_tx_execution` on all
/// components.
///
/// this *must* be called anytime before a block's txs are executed, whether it's
/// during the proposal phase, or finalize_block phase.
Expand All @@ -696,42 +691,18 @@ impl App {
// reset recost flag
self.recost_mempool = false;

// call begin_block on all components
// NOTE: the fields marked `unused` are not used by any of the components;
// however, we need to still construct a `BeginBlock` type for now as
// the penumbra IBC implementation still requires it as a parameter.
let begin_block: abci::request::BeginBlock = abci::request::BeginBlock {
hash: Hash::default(), // unused
byzantine_validators: block_data.misbehavior.clone(),
header: Header {
app_hash: self.app_hash.clone(),
chain_id: chain_id.clone(),
consensus_hash: Hash::default(), // unused
data_hash: Some(Hash::default()), // unused
evidence_hash: Some(Hash::default()), // unused
height: block_data.height,
last_block_id: None, // unused
last_commit_hash: Some(Hash::default()), // unused
last_results_hash: Some(Hash::default()), // unused
next_validators_hash: block_data.next_validators_hash,
proposer_address: block_data.proposer_address,
time: block_data.time,
validators_hash: Hash::default(), // unused
version: tendermint::block::header::Version {
// unused
app: 0,
block: 0,
},
},
last_commit_info: tendermint::abci::types::CommitInfo {
round: 0u16.into(), // unused
votes: vec![],
}, // unused
let prepare_state_info = PrepareStateInfo {
app_hash: self.app_hash.clone(),
byzantine_validators: block_data.misbehavior,
chain_id,
height: block_data.height,
next_validators_hash: block_data.next_validators_hash,
time: block_data.time,
};

self.begin_block(&begin_block)
self.start_block(&prepare_state_info)
.await
.wrap_err("begin_block failed")?;
.wrap_err("prepare_state_for_tx_execution failed")?;

Ok(())
}
Expand Down Expand Up @@ -759,13 +730,8 @@ impl App {
.get_chain_id()
.await
.wrap_err("failed to get chain ID from state")?;
let sudo_address = self
.state
.get_sudo_address()
.await
.wrap_err("failed to get sudo address from state")?;

let end_block = self.end_block(height.value(), &sudo_address).await?;
let (validator_updates, events) = self.component_post_execution_state_updates().await?;

// get deposits for this block from state's ephemeral cache and put them to storage.
let mut state_tx = StateDelta::new(self.state.clone());
Expand Down Expand Up @@ -803,15 +769,14 @@ impl App {
.wrap_err("failed to write sequencer block to state")?;

let result = PostTransactionExecutionResult {
events: end_block.events,
validator_updates: end_block.validator_updates,
consensus_param_updates: end_block.consensus_param_updates,
events,
validator_updates,
tx_results: finalize_block_tx_results,
};

state_tx.object_put(POST_TRANSACTION_EXECUTION_RESULT_KEY, result);

// events that occur after end_block are ignored here;
// events that occur after handle_post_tx_execution are ignored here;
// there should be none anyways.
let _ = self.apply(state_tx);

Expand Down Expand Up @@ -856,7 +821,6 @@ impl App {
height,
time,
next_validators_hash: finalize_block.next_validators_hash,
proposer_address,
};

self.pre_execute_transactions(block_data)
Expand Down Expand Up @@ -930,7 +894,7 @@ impl App {
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,
ethanoroshiba marked this conversation as resolved.
Show resolved Hide resolved
consensus_param_updates: None,
app_hash,
tx_results: post_transaction_execution_result.tx_results,
};
Expand Down Expand Up @@ -975,34 +939,34 @@ impl App {
Ok(app_hash)
}

#[instrument(name = "App::begin_block", skip_all)]
async fn begin_block(
#[instrument(name = "App::start_block", skip_all)]
async fn start_block(
&mut self,
begin_block: &abci::request::BeginBlock,
prepare_state_info: &PrepareStateInfo,
) -> Result<Vec<abci::Event>> {
let mut state_tx = StateDelta::new(self.state.clone());

state_tx
.put_block_height(begin_block.header.height.into())
.put_block_height(prepare_state_info.height.into())
.wrap_err("failed to put block height")?;
state_tx
.put_block_timestamp(begin_block.header.time)
.put_block_timestamp(prepare_state_info.time)
.wrap_err("failed to put block timestamp")?;

// call begin_block on all components
// call prepare_state_for_tx_execution on all components
let mut arc_state_tx = Arc::new(state_tx);
AccountsComponent::begin_block(&mut arc_state_tx, begin_block)
AccountsComponent::prepare_state_for_tx_execution(&mut arc_state_tx, prepare_state_info)
.await
.wrap_err("begin_block failed on AccountsComponent")?;
AuthorityComponent::begin_block(&mut arc_state_tx, begin_block)
.wrap_err("prepare_state_for_tx_execution failed on AccountsComponent")?;
AuthorityComponent::prepare_state_for_tx_execution(&mut arc_state_tx, prepare_state_info)
.await
.wrap_err("begin_block failed on AuthorityComponent")?;
IbcComponent::begin_block(&mut arc_state_tx, begin_block)
.wrap_err("prepare_state_for_tx_execution failed on AuthorityComponent")?;
IbcComponent::prepare_state_for_tx_execution(&mut arc_state_tx, prepare_state_info)
.await
.wrap_err("begin_block failed on IbcComponent")?;
FeesComponent::begin_block(&mut arc_state_tx, begin_block)
.wrap_err("prepare_state_for_tx_execution failed on IbcComponent")?;
FeesComponent::prepare_state_for_tx_execution(&mut arc_state_tx, prepare_state_info)
.await
.wrap_err("begin_block failed on FeesComponent")?;
.wrap_err("prepare_state_for_tx_execution failed on FeesComponent")?;

let state_tx = Arc::try_unwrap(arc_state_tx)
.expect("components should not retain copies of shared state");
Expand Down Expand Up @@ -1048,34 +1012,26 @@ impl App {
Ok(events)
}

#[instrument(name = "App::end_block", skip_all)]
async fn end_block(
#[instrument(name = "App::component_post_execution_state_updates", skip_all)]
async fn component_post_execution_state_updates(
&mut self,
height: u64,
fee_recipient: &[u8; 20],
) -> Result<abci::response::EndBlock> {
) -> Result<(Vec<tendermint::validator::Update>, Vec<Event>)> {
let state_tx = StateDelta::new(self.state.clone());
let mut arc_state_tx = Arc::new(state_tx);

let end_block = abci::request::EndBlock {
height: height
.try_into()
.expect("a block height should be able to fit in an i64"),
};

// call end_block on all components
AccountsComponent::end_block(&mut arc_state_tx, &end_block)
// call handle_post_tx_execution on all components
AccountsComponent::handle_post_tx_execution(&mut arc_state_tx)
.await
.wrap_err("end_block failed on AccountsComponent")?;
AuthorityComponent::end_block(&mut arc_state_tx, &end_block)
.wrap_err("handle_post_tx_execution failed on AccountsComponent")?;
AuthorityComponent::handle_post_tx_execution(&mut arc_state_tx)
.await
.wrap_err("end_block failed on AuthorityComponent")?;
FeesComponent::end_block(&mut arc_state_tx, &end_block)
.wrap_err("handle_post_tx_execution failed on AuthorityComponent")?;
FeesComponent::handle_post_tx_execution(&mut arc_state_tx)
.await
.wrap_err("end_block failed on FeesComponent")?;
IbcComponent::end_block(&mut arc_state_tx, &end_block)
.wrap_err("handle_post_tx_execution failed on FeesComponent")?;
IbcComponent::handle_post_tx_execution(&mut arc_state_tx)
.await
.wrap_err("end_block failed on IbcComponent")?;
.wrap_err("handle_post_tx_execution failed on IbcComponent")?;

let mut state_tx = Arc::try_unwrap(arc_state_tx)
.expect("components should not retain copies of shared state");
Expand All @@ -1090,24 +1046,13 @@ impl App {
// clear validator updates
state_tx.clear_validator_updates();

// gather block fees and transfer them to the block proposer
let fees = self.state.get_block_fees();

for fee in fees {
state_tx
.increase_balance(fee_recipient, fee.asset(), fee.amount())
.await
.wrap_err("failed to increase fee recipient balance")?;
}

let events = self.apply(state_tx);
Ok(abci::response::EndBlock {
validator_updates: validator_updates
Ok((
validator_updates
.try_into_cometbft()
.wrap_err("failed converting astria validators to cometbft compatible type")?,
events,
..Default::default()
})
))
}

#[instrument(name = "App::commit", skip_all)]
Expand Down Expand Up @@ -1176,7 +1121,6 @@ struct BlockData {
height: tendermint::block::Height,
time: tendermint::Time,
next_validators_hash: Hash,
proposer_address: account::Id,
}

fn signed_transaction_from_bytes(bytes: &[u8]) -> Result<Transaction> {
Expand All @@ -1193,7 +1137,6 @@ struct PostTransactionExecutionResult {
events: Vec<Event>,
tx_results: Vec<ExecTxResult>,
validator_updates: Vec<tendermint::validator::Update>,
consensus_param_updates: Option<tendermint::consensus::Params>,
}

#[derive(PartialEq)]
Expand Down
Loading
Loading