Skip to content

Commit

Permalink
sequencer-types: type all errors (#376)
Browse files Browse the repository at this point in the history
## Summary
Replace all implicit eyre errors by context specific typed errors

## Background
Same reasoning as for #375 as
part of the work for #332:

`sequencer-types` has to be made a dependency of `sequencer-client`,
which only uses typed errors. Therefore `sequencer-types` must have
typed errors (because otherwise `sequencer-client` would be "infected"
by eyre, which would require changing all of sequencer-client to also
use eyre.

## Changes
- Introduced one error variant per distinct eyre error.

## Testing
N/A. Nothing changed on the code level. Testing that the errors are
actually triggered could be done in a follow-up PR, but big parts of the
code will be replaced by the work in
#355 and follow-up PRs to it,
moving the types exchanged over RPCs to the astria-proto crate.

## Breaking Changelist
- No observable breaking changes in the astria services. Breaking
changes in the library crates because their public API changed (errors),
but we are not considering them right now

## Related Issues
#12
#86
#366
#332
  • Loading branch information
SuperFluffy committed Sep 18, 2023
1 parent 57a3103 commit e696394
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 66 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions crates/astria-sequencer-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ sha2 = "0.10"
thiserror = "1.0"

base64 = { workspace = true }
eyre = { workspace = true }
hex = { workspace = true, features = ["serde"] }
prost = { workspace = true }
serde = { workspace = true, features = ["derive"] }
Expand All @@ -18,9 +17,9 @@ tendermint = { workspace = true, features = ["rust-crypto"] }
tendermint-proto = { workspace = true }
tracing = { workspace = true }

astria-celestia-jsonrpc-client = { path = "../astria-celestia-jsonrpc-client" }
astria-sequencer-validation = { path = "../astria-sequencer-validation" }
celestia-jsonrpc-client = { package = "astria-celestia-jsonrpc-client", path = "../astria-celestia-jsonrpc-client" }
proto = { package = "astria-proto", path = "../astria-proto" }
sequencer-validation = { package = "astria-sequencer-validation", path = "../astria-sequencer-validation" }

[dev-dependencies]
ct-merkle = { version = "0.1", features = ["serde"] }
2 changes: 1 addition & 1 deletion crates/astria-sequencer-types/src/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
ops::Deref,
};

use astria_celestia_jsonrpc_client::blob::NAMESPACE_ID_AVAILABLE_LEN;
use celestia_jsonrpc_client::blob::NAMESPACE_ID_AVAILABLE_LEN;
use serde::{
de::{self,},
Deserialize,
Expand Down
139 changes: 78 additions & 61 deletions crates/astria-sequencer-types/src/sequencer_block_data.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
use std::collections::BTreeMap;

use astria_sequencer_validation::{
InclusionProof,
MerkleTree,
use std::{
array::TryFromSliceError,
collections::BTreeMap,
};

use base64::{
engine::general_purpose,
Engine as _,
};
use eyre::{
bail,
ensure,
eyre,
WrapErr as _,
use proto::DecodeError;
use sequencer_validation::{
InclusionProof,
MerkleTree,
};
use serde::{
Deserialize,
Expand All @@ -33,10 +31,45 @@ use crate::tendermint::calculate_last_commit_hash;

#[derive(Error, Debug)]
pub enum Error {
#[error("failed converting bytes to action tree root: expected 32 bytes")]
ActionTreeRootConversion(#[source] TryFromSliceError),
#[error(
"data hash stored tendermint header does not match action tree root reconstructed from \
data"
)]
CometBftDataHashReconstructedHashMismatch,
#[error(
"block hash calculated from tendermint header does not match block hash stored in \
sequencer block"
)]
HashOfHeaderBlockHashMismatach,
#[error("failed to generate inclusion proof for action tree root")]
InclusionProof(sequencer_validation::IndexOutOfBounds),
#[error(
"last commit hash does not match the one calculated from the block's commit signatures"
)]
LastCommitHashMismatch,
#[error("the sequencer block contained neither action tree root nor transaction data")]
NoData,
#[error("block has no data hash")]
MissingDataHash,
#[error("block has no last commit hash")]
MissingLastCommitHash,
#[error("failed decoding bytes to protobuf signed transaction")]
SignedTransactionProtobufDecode(#[source] DecodeError),
#[error("failed constructing native signed transaction from raw protobuf signed transaction")]
RawSignedTransactionConversion(
#[source] proto::native::sequencer::v1alpha1::SignedTransactionError,
),
#[error("failed deserializing sequencer block data from json bytes")]
ReadingJson(#[source] serde_json::Error),
#[error(
"failed to verify data hash in cometbft header against inclusion proof and action tree \
root in sequencer block body"
)]
Verification(#[source] sequencer_validation::VerificationFailure),
#[error("failed writing sequencer block data as json")]
WritingJson(#[source] serde_json::Error),
}

#[derive(Deserialize, Serialize, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
Expand Down Expand Up @@ -87,7 +120,7 @@ impl SequencerBlockData {
/// - if the block's action tree root inclusion proof cannot be verified
/// - if the block's height is >1 and it does not contain a last commit or last commit hash
/// - if the block's last commit hash does not match the one calculated from the block's commit
pub fn try_from_raw(raw: RawSequencerBlockData) -> eyre::Result<Self> {
pub fn try_from_raw(raw: RawSequencerBlockData) -> Result<Self, Error> {
let RawSequencerBlockData {
block_hash,
header,
Expand All @@ -98,18 +131,14 @@ impl SequencerBlockData {
} = raw;

let calculated_block_hash = header.hash();
ensure!(
block_hash == calculated_block_hash,
"block hash calculated from tendermint header does not match block hash stored in \
sequencer block",
);
if block_hash != calculated_block_hash {
return Err(Error::HashOfHeaderBlockHashMismatach);
}

let Some(data_hash) = header.data_hash else {
bail!(Error::MissingDataHash);
};
let data_hash = header.data_hash.ok_or(Error::MissingDataHash)?;
action_tree_root_inclusion_proof
.verify(&action_tree_root, data_hash)
.wrap_err("failed to verify action tree root inclusion proof")?;
.map_err(Error::Verification)?;

// genesis and height 1 do not have a last commit
if header.height.value() <= 1 {
Expand All @@ -124,20 +153,18 @@ impl SequencerBlockData {
}

// calculate and verify last_commit_hash
let Some(last_commit_hash) = header.last_commit_hash else {
bail!(Error::MissingLastCommitHash);
};
let last_commit_hash = header
.last_commit_hash
.ok_or(Error::MissingLastCommitHash)?;

let calculated_last_commit_hash = calculate_last_commit_hash(
last_commit
.as_ref()
.expect("last_commit must be set if last_commit_hash is set"),
);
ensure!(
calculated_last_commit_hash == last_commit_hash,
"last commit hash does not match the one calculated from the block's commit signatures",
);

if calculated_last_commit_hash != last_commit_hash {
return Err(Error::LastCommitHashMismatch)?;
}
Ok(Self {
block_hash,
header,
Expand Down Expand Up @@ -195,21 +222,17 @@ impl SequencerBlockData {
/// # Errors
///
/// - if the data cannot be serialized into json
pub fn to_bytes(&self) -> eyre::Result<Vec<u8>> {
// TODO: don't use json, use our own serializer (or protobuf for now?)
serde_json::to_vec(self).wrap_err("failed serializing signed namespace data to json")
pub fn to_bytes(&self) -> Result<Vec<u8>, Error> {
serde_json::to_vec(self).map_err(Error::WritingJson)
}

/// Converts json-encoded bytes into a `SequencerBlockData`.
///
/// # Errors
///
/// - if the data cannot be deserialized from json
/// - if the block hash cannot be verified
pub fn from_bytes(bytes: &[u8]) -> eyre::Result<Self> {
let data: Self = serde_json::from_slice(bytes)
.wrap_err("failed deserializing signed namespace data from bytes")?;
Ok(data)
pub fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
serde_json::from_slice(bytes).map_err(Error::ReadingJson)
}

/// Converts a Tendermint block into a `SequencerBlockData`.
Expand All @@ -227,44 +250,40 @@ impl SequencerBlockData {
///
/// See `specs/sequencer-inclusion-proofs.md` for most details on the action tree root
/// and inclusion proof purpose.
pub fn from_tendermint_block(b: Block) -> eyre::Result<Self> {
pub fn from_tendermint_block(b: Block) -> Result<Self, Error> {
use proto::{
generated::sequencer::v1alpha1 as raw,
native::sequencer::v1alpha1::SignedTransaction,
Message as _,
};
let Some(data_hash) = b.header.data_hash else {
bail!(Error::MissingDataHash);
};
let data_hash = b.header.data_hash.ok_or(Error::MissingDataHash)?;

if b.data.is_empty() {
bail!("block has no transactions; ie action tree root is missing");
}
let mut datas = b.data.iter();

let action_tree_root: [u8; 32] = b.data[0]
.clone()
let action_tree_root: [u8; 32] = datas
.next()
.map(Vec::as_slice)
.ok_or(Error::NoData)?
.try_into()
.map_err(|_| eyre!("action tree root must be 32 bytes"))?;
.map_err(Error::ActionTreeRootConversion)?;

// we unwrap sequencer txs into rollup-specific data here,
// and namespace them correspondingly
let mut rollup_data = BTreeMap::new();

// the first transaction is skipped as it's the action tree root,
// not a user-submitted transaction.
for (index, tx) in b.data[1..].iter().enumerate() {
for (index, tx) in datas.enumerate() {
debug!(
index,
bytes = general_purpose::STANDARD.encode(tx.as_slice()),
"parsing data from tendermint block",
);

let raw_tx = raw::SignedTransaction::decode(&**tx)
.wrap_err("failed decoding bytes to protobuf signed transaction")?;
let tx = SignedTransaction::try_from_raw(raw_tx).wrap_err(
"failed constructing native signed transaction from raw protobuf signed \
transaction",
)?;
.map_err(Error::SignedTransactionProtobufDecode)?;
let tx = SignedTransaction::try_from_raw(raw_tx)
.map_err(Error::RawSignedTransactionConversion)?;
tx.actions().iter().for_each(|action| {
if let Some(action) = action.as_sequence() {
// TODO(https://github.com/astriaorg/astria/issues/318): intern
Expand All @@ -282,14 +301,12 @@ impl SequencerBlockData {
// generate the action tree root proof of inclusion in `Header::data_hash`
let tx_tree = MerkleTree::from_leaves(b.data);
let calculated_data_hash = tx_tree.root();
ensure!(
// this should never happen for a correctly-constructed block
calculated_data_hash == data_hash.as_bytes(),
"action tree root does not match the first transaction in the block",
);
if calculated_data_hash != data_hash.as_bytes() {
return Err(Error::CometBftDataHashReconstructedHashMismatch);
}
let action_tree_root_inclusion_proof = tx_tree
.prove_inclusion(0) // action tree root is always the first tx in a block
.wrap_err("failed to generate inclusion proof for action tree root")?;
.map_err(Error::InclusionProof)?;

let data = Self {
block_hash: b.header.hash(),
Expand Down Expand Up @@ -322,9 +339,9 @@ pub struct RawSequencerBlockData {
}

impl TryFrom<RawSequencerBlockData> for SequencerBlockData {
type Error = eyre::Error;
type Error = Error;

fn try_from(raw: RawSequencerBlockData) -> eyre::Result<Self> {
fn try_from(raw: RawSequencerBlockData) -> Result<Self, Self::Error> {
Self::try_from_raw(raw)
}
}
Expand All @@ -339,7 +356,7 @@ impl From<SequencerBlockData> for RawSequencerBlockData {
mod test {
use std::collections::BTreeMap;

use astria_sequencer_validation::MerkleTree;
use sequencer_validation::MerkleTree;
use tendermint::Hash;

use super::SequencerBlockData;
Expand Down
2 changes: 2 additions & 0 deletions crates/astria-sequencer-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ mod utils;

pub use proof::{
InclusionProof,
IndexOutOfBounds,
MerkleTree,
VerificationFailure,
};
pub use utils::generate_action_tree_leaves;

0 comments on commit e696394

Please sign in to comment.