Skip to content

Commit

Permalink
add context to errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Fraser999 committed Sep 30, 2024
1 parent 3c47298 commit fe21bcb
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 69 deletions.
15 changes: 9 additions & 6 deletions crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,9 @@ impl App {
.get_block_height()
.await
.expect("block height must be set, as `put_block_height` was already called");
// No need to add context as this method already reports sufficient context on error.
state.put_storage_version_by_height(height, new_version)?;
state
.put_storage_version_by_height(height, new_version)
.wrap_err("failed to put storage version by height")?;
debug!(
height,
version = new_version,
Expand Down Expand Up @@ -975,10 +976,12 @@ impl App {
) -> Result<Vec<abci::Event>> {
let mut state_tx = StateDelta::new(self.state.clone());

// No need to add context as this method already reports sufficient context on error.
state_tx.put_block_height(begin_block.header.height.into())?;
// No need to add context as this method already reports sufficient context on error.
state_tx.put_block_timestamp(begin_block.header.time)?;
state_tx
.put_block_height(begin_block.header.height.into())
.wrap_err("failed to put block height")?;
state_tx
.put_block_timestamp(begin_block.header.time)
.wrap_err("failed to put block timestamp")?;

// call begin_block on all components
let mut arc_state_tx = Arc::new(state_tx);
Expand Down
40 changes: 21 additions & 19 deletions crates/astria-sequencer/src/authority/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,26 +126,28 @@ impl ActionHandler for FeeChangeAction {
.wrap_err("failed to get sudo address from state")?;
ensure!(sudo_address == from, "signer is not the sudo key");

// No need to add context to any of these `put` calls, as they already report sufficient
// context on error.
match self.fee_change {
FeeChange::TransferBaseFee => state.put_transfer_base_fee(self.new_value),
FeeChange::SequenceBaseFee => state.put_sequence_action_base_fee(self.new_value),
FeeChange::SequenceByteCostMultiplier => {
state.put_sequence_action_byte_cost_multiplier(self.new_value)
}
FeeChange::InitBridgeAccountBaseFee => {
state.put_init_bridge_account_base_fee(self.new_value)
}
FeeChange::BridgeLockByteCostMultiplier => {
state.put_bridge_lock_byte_cost_multiplier(self.new_value)
}
FeeChange::BridgeSudoChangeBaseFee => {
state.put_bridge_sudo_change_base_fee(self.new_value)
}
FeeChange::Ics20WithdrawalBaseFee => {
state.put_ics20_withdrawal_base_fee(self.new_value)
}
FeeChange::TransferBaseFee => state
.put_transfer_base_fee(self.new_value)
.wrap_err("failed to put transfer base fee"),
FeeChange::SequenceBaseFee => state
.put_sequence_action_base_fee(self.new_value)
.wrap_err("failed to put sequence action base fee"),
FeeChange::SequenceByteCostMultiplier => state
.put_sequence_action_byte_cost_multiplier(self.new_value)
.wrap_err("failed to put sequence action byte cost multiplier"),
FeeChange::InitBridgeAccountBaseFee => state
.put_init_bridge_account_base_fee(self.new_value)
.wrap_err("failed to put init bridge account base fee"),
FeeChange::BridgeLockByteCostMultiplier => state
.put_bridge_lock_byte_cost_multiplier(self.new_value)
.wrap_err("failed to put bridge lock byte cost multiplier"),
FeeChange::BridgeSudoChangeBaseFee => state
.put_bridge_sudo_change_base_fee(self.new_value)
.wrap_err("failed to put bridge sudo change base fee"),
FeeChange::Ics20WithdrawalBaseFee => state
.put_ics20_withdrawal_base_fee(self.new_value)
.wrap_err("failed to put ics20 withdrawal base fee"),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,15 @@ impl ActionHandler for BridgeSudoChangeAction {
.wrap_err("failed to decrease balance for bridge sudo change fee")?;

if let Some(sudo_address) = self.new_sudo_address {
// No need to add context as this method already reports sufficient context on error.
state.put_bridge_account_sudo_address(&self.bridge_address, sudo_address)?;
state
.put_bridge_account_sudo_address(&self.bridge_address, sudo_address)
.wrap_err("failed to put bridge account sudo address")?;
}

if let Some(withdrawer_address) = self.new_withdrawer_address {
// No need to add context as this method already reports sufficient context on error.
state
.put_bridge_account_withdrawer_address(&self.bridge_address, withdrawer_address)?;
.put_bridge_account_withdrawer_address(&self.bridge_address, withdrawer_address)
.wrap_err("failed to put bridge account withdrawer address")?;
}

Ok(())
Expand Down
20 changes: 13 additions & 7 deletions crates/astria-sequencer/src/bridge/component.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::sync::Arc;

use astria_core::protocol::genesis::v1alpha1::GenesisAppState;
use astria_eyre::eyre::Result;
use astria_eyre::eyre::{
Result,
WrapErr,
};
use tendermint::abci::request::{
BeginBlock,
EndBlock,
Expand All @@ -20,12 +23,15 @@ impl Component for BridgeComponent {

#[instrument(name = "BridgeComponent::init_chain", skip_all)]
async fn init_chain<S: StateWriteExt>(mut state: S, app_state: &Self::AppState) -> Result<()> {
// No need to add context as these `put` methods already report sufficient context on error.
state.put_init_bridge_account_base_fee(app_state.fees().init_bridge_account_base_fee)?;
state.put_bridge_lock_byte_cost_multiplier(
app_state.fees().bridge_lock_byte_cost_multiplier,
)?;
state.put_bridge_sudo_change_base_fee(app_state.fees().bridge_sudo_change_fee)
state
.put_init_bridge_account_base_fee(app_state.fees().init_bridge_account_base_fee)
.wrap_err("failed to put init bridge account base fee")?;
state
.put_bridge_lock_byte_cost_multiplier(app_state.fees().bridge_lock_byte_cost_multiplier)
.wrap_err("failed to put bridge lock byte cost multiplier")?;
state
.put_bridge_sudo_change_base_fee(app_state.fees().bridge_sudo_change_fee)
.wrap_err("failed to put bridge sudo change base fee")
}

#[instrument(name = "BridgeComponent::begin_block", skip_all)]
Expand Down
24 changes: 12 additions & 12 deletions crates/astria-sequencer/src/bridge/init_bridge_account_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,21 +93,21 @@ impl ActionHandler for InitBridgeAccountAction {
"insufficient funds for bridge account initialization",
);

// No need to add context as this method already reports sufficient context on error.
state.put_bridge_account_rollup_id(&from, self.rollup_id)?;
state
.put_bridge_account_rollup_id(&from, self.rollup_id)
.wrap_err("failed to put bridge account rollup id")?;
state
.put_bridge_account_ibc_asset(&from, &self.asset)
.wrap_err("failed to put asset ID")?;
// No need to add context as this method already reports sufficient context on error.
state.put_bridge_account_sudo_address(
&from,
self.sudo_address.map_or(from, Address::bytes),
)?;
// No need to add context as this method already reports sufficient context on error.
state.put_bridge_account_withdrawer_address(
&from,
self.withdrawer_address.map_or(from, Address::bytes),
)?;
state
.put_bridge_account_sudo_address(&from, self.sudo_address.map_or(from, Address::bytes))
.wrap_err("failed to put bridge account sudo address")?;
state
.put_bridge_account_withdrawer_address(
&from,
self.withdrawer_address.map_or(from, Address::bytes),
)
.wrap_err("failed to put bridge account withdrawer address")?;
state
.get_and_increase_block_fees(&self.fee_asset, fee, Self::full_name())
.await
Expand Down
19 changes: 14 additions & 5 deletions crates/astria-sequencer/src/grpc/state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,22 @@ async fn get_sequencer_block_by_hash<S: StateRead + ?Sized>(
state: &S,
hash: &[u8; 32],
) -> Result<SequencerBlock> {
// No need to add context as these `get` methods already report sufficient context on error.
let header = state.get_sequencer_block_header_by_hash(hash).await?;
let rollup_ids = state.get_rollup_ids_by_block_hash(hash).await?;
let header = state
.get_sequencer_block_header_by_hash(hash)
.await
.wrap_err("failed to get sequencer block header by hash")?;
let rollup_ids = state
.get_rollup_ids_by_block_hash(hash)
.await
.wrap_err("failed to get rollup ids by block hash")?;
let rollup_transactions_proof = state
.get_rollup_transactions_proof_by_block_hash(hash)
.await?;
let rollup_ids_proof = state.get_rollup_ids_proof_by_block_hash(hash).await?;
.await
.wrap_err("failed to get rollup transactions proof by block hash")?;
let rollup_ids_proof = state
.get_rollup_ids_proof_by_block_hash(hash)
.await
.wrap_err("failed to get rollup ids proof by block hash")?;

// allow: want to avoid explicitly importing `index_map` crate to sequencer crate.
#[allow(clippy::default_trait_access)]
Expand Down
5 changes: 3 additions & 2 deletions crates/astria-sequencer/src/ibc/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ impl Component for IbcComponent {
.wrap_err("failed to set IBC sudo key")?;

for address in app_state.ibc_relayer_addresses() {
// No need to add context as this method already reports sufficient context on error.
state.put_ibc_relayer_address(address)?;
state
.put_ibc_relayer_address(address)
.wrap_err("failed to put IBC relayer address")?;
}

state
Expand Down
6 changes: 3 additions & 3 deletions crates/astria-sequencer/src/ibc/ibc_relayer_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ impl ActionHandler for IbcRelayerChangeAction {

match self {
IbcRelayerChangeAction::Addition(address) => {
// No need to add context as this method already reports sufficient context on
// error.
state.put_ibc_relayer_address(address)?;
state
.put_ibc_relayer_address(address)
.wrap_err("failed to put IBC relayer address")?;
}
IbcRelayerChangeAction::Removal(address) => {
state.delete_ibc_relayer_address(address);
Expand Down
18 changes: 12 additions & 6 deletions crates/astria-sequencer/src/sequence/component.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::sync::Arc;

use astria_core::protocol::genesis::v1alpha1::GenesisAppState;
use astria_eyre::eyre::Result;
use astria_eyre::eyre::{
Result,
WrapErr,
};
use tendermint::abci::request::{
BeginBlock,
EndBlock,
Expand All @@ -20,11 +23,14 @@ impl Component for SequenceComponent {

#[instrument(name = "SequenceComponent::init_chain", skip_all)]
async fn init_chain<S: StateWriteExt>(mut state: S, app_state: &Self::AppState) -> Result<()> {
// No need to add context as these `put` methods already report sufficient context on error.
state.put_sequence_action_base_fee(app_state.fees().sequence_base_fee)?;
state.put_sequence_action_byte_cost_multiplier(
app_state.fees().sequence_byte_cost_multiplier,
)
state
.put_sequence_action_base_fee(app_state.fees().sequence_base_fee)
.wrap_err("failed to put sequence action base fee")?;
state
.put_sequence_action_byte_cost_multiplier(
app_state.fees().sequence_byte_cost_multiplier,
)
.wrap_err("failed to put sequence action byte cost multiplier")
}

#[instrument(name = "SequenceComponent::begin_block", skip_all)]
Expand Down
11 changes: 6 additions & 5 deletions crates/astria-sequencer/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,12 @@ impl ActionHandler for SignedTransaction {
.wrap_err("failed to check account rollup id")?
.is_some()
{
// No need to add context as this method already reports sufficient context on error.
state.put_last_transaction_id_for_bridge_account(
&self,
transaction_context.transaction_id,
)?;
state
.put_last_transaction_id_for_bridge_account(
&self,
transaction_context.transaction_id,
)
.wrap_err("failed to put last transaction id for bridge account")?;
}

let from_nonce = state
Expand Down

0 comments on commit fe21bcb

Please sign in to comment.