Skip to content

Commit

Permalink
Requested changes from Fraser
Browse files Browse the repository at this point in the history
  • Loading branch information
ethanoroshiba committed Aug 2, 2024
1 parent 219ee9e commit d92a373
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 68 deletions.
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/celestia/block_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ mod test {

#[tokio::test]
async fn validate_sequencer_blob_with_chain_ids() {
let test_tx = b"test-tx".to_vec();
let test_tx = Bytes::from_static(b"test-tx");
let rollup_id = RollupId::from_unhashed_bytes(b"test-chain");
let grouped_txs = BTreeMap::from([(rollup_id, vec![test_tx.clone()])]);
let rollup_transactions_tree =
Expand Down
3 changes: 2 additions & 1 deletion crates/astria-conductor/src/celestia/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use astria_eyre::eyre::{
bail,
WrapErr as _,
};
use bytes::Bytes;
use celestia_types::nmt::Namespace;
use futures::{
future::{
Expand Down Expand Up @@ -101,7 +102,7 @@ pub(crate) struct ReconstructedBlock {
pub(crate) celestia_height: u64,
pub(crate) block_hash: [u8; 32],
pub(crate) header: SequencerBlockHeader,
pub(crate) transactions: Vec<Vec<u8>>,
pub(crate) transactions: Vec<Bytes>,
}

impl ReconstructedBlock {
Expand Down
8 changes: 1 addition & 7 deletions crates/astria-conductor/src/celestia/reconstruct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use astria_core::{
SubmittedRollupData,
},
};
use bytes::Bytes;
use telemetry::display::base64;
use tracing::{
info,
Expand Down Expand Up @@ -57,12 +56,7 @@ pub(super) fn reconstruct_blocks_from_verified_blobs(
celestia_height,
block_hash: header_blob.block_hash(),
header: header_blob.into_unchecked().header,
transactions: rollup
.into_unchecked()
.transactions
.into_iter()
.map(Bytes::into)
.collect(),
transactions: rollup.into_unchecked().transactions,
});
} else {
let reason = if header_blobs.contains_key(&rollup.sequencer_block_hash()) {
Expand Down
4 changes: 2 additions & 2 deletions crates/astria-conductor/src/executor/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ impl Client {
pub(super) async fn execute_block_with_retry(
&mut self,
prev_block_hash: Bytes,
transactions: Vec<Vec<u8>>,
transactions: Vec<Bytes>,
timestamp: Timestamp,
) -> eyre::Result<Block> {
use prost::Message;

let transactions = transactions
.into_iter()
.map(|tx| RollupData::decode(tx.as_slice()))
.map(|tx| RollupData::decode(tx.as_ref()))
.collect::<Result<_, _>>()
.wrap_err("failed to decode tx bytes as RollupData")?;

Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ struct ExecutableBlock {
hash: [u8; 32],
height: SequencerHeight,
timestamp: pbjson_types::Timestamp,
transactions: Vec<Vec<u8>>,
transactions: Vec<Bytes>,
}

impl ExecutableBlock {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ source: crates/astria-conductor/src/sequencer/reporting.rs
expression: ReportRollups(block.rollup_transactions())
---
{
"KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKio=": 2,
"RUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUU=": 1
"KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKio=": 3,
"RUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUVFRUU=": 2
}
2 changes: 1 addition & 1 deletion crates/astria-core/src/primitive/v1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ impl std::fmt::Display for Address {
pub fn derive_merkle_tree_from_rollup_txs<'a, T, U>(rollup_ids_to_txs: T) -> merkle::Tree
where
T: IntoIterator<Item = (&'a RollupId, &'a U)>,
U: AsRef<[Vec<u8>]> + 'a + ?Sized,
U: AsRef<[Bytes]> + 'a + ?Sized,
{
let mut tree = merkle::Tree::new();
for (rollup_id, txs) in rollup_ids_to_txs {
Expand Down
30 changes: 9 additions & 21 deletions crates/astria-core/src/protocol/bridge/v1alpha1/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use bytes::{
Buf as _,
Bytes,
};
use bytes::Bytes;

use super::raw;
use crate::primitive::v1::{
Expand All @@ -27,21 +24,19 @@ impl BridgeAccountLastTxHashResponse {
///
/// - if the transaction hash is not 32 bytes
pub fn try_from_raw(
raw: raw::BridgeAccountLastTxHashResponse,
raw: &raw::BridgeAccountLastTxHashResponse,
) -> Result<Self, BridgeAccountLastTxHashResponseError> {
Ok(Self {
height: raw.height,
tx_hash: raw
.tx_hash
.clone()
.map(|bytes| bytes.chunk().try_into())
.transpose()
.map_err(|_| {
let Some(tx_hash) = raw.tx_hash else {
return BridgeAccountLastTxHashResponseError::missing_tx_hash();
};
BridgeAccountLastTxHashResponseError::invalid_tx_hash(tx_hash.len())
})?,
.map(|bytes| {
<[u8; 32]>::try_from(bytes.as_ref()).map_err(|_| {
BridgeAccountLastTxHashResponseError::invalid_tx_hash(bytes.len())
})
})
.transpose()?,
})
}

Expand All @@ -64,7 +59,7 @@ impl raw::BridgeAccountLastTxHashResponse {
pub fn try_into_native(
self,
) -> Result<BridgeAccountLastTxHashResponse, BridgeAccountLastTxHashResponseError> {
BridgeAccountLastTxHashResponse::try_from_raw(self)
BridgeAccountLastTxHashResponse::try_from_raw(&self)
}

#[must_use]
Expand All @@ -86,19 +81,12 @@ impl BridgeAccountLastTxHashResponseError {
bytes,
))
}

#[must_use]
pub fn missing_tx_hash() -> Self {
Self(BridgeAccountLastTxHashResponseErrorKind::MissingTxHash())
}
}

#[derive(Debug, thiserror::Error)]
enum BridgeAccountLastTxHashResponseErrorKind {
#[error("invalid tx hash; must be 32 bytes, got {0} bytes")]
InvalidTxHash(usize),
#[error("tx hash was not set")]
MissingTxHash(),
}

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down
10 changes: 6 additions & 4 deletions crates/astria-core/src/protocol/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use bytes::Bytes;
use indexmap::IndexMap;
use transaction::v1alpha1::SignedTransaction;

Expand All @@ -19,7 +20,7 @@ pub mod test_utils;
/// TODO: This can all be done in-place once <https://github.com/rust-lang/rust/issues/80552> is stabilized.
pub fn group_sequence_actions_in_signed_transaction_transactions_by_rollup_id(
signed_transactions: &[SignedTransaction],
) -> IndexMap<RollupId, Vec<Vec<u8>>> {
) -> IndexMap<RollupId, Vec<Bytes>> {
use prost::Message as _;

use crate::sequencerblock::v1alpha1::block::RollupData;
Expand All @@ -30,9 +31,10 @@ pub fn group_sequence_actions_in_signed_transaction_transactions_by_rollup_id(
.flat_map(SignedTransaction::actions)
{
if let Some(action) = action.as_sequence() {
let txs_for_rollup: &mut Vec<Vec<u8>> = map.entry(action.rollup_id).or_insert(vec![]);
let rollup_data = RollupData::SequencedData(action.data.to_vec());
txs_for_rollup.push(rollup_data.into_raw().encode_to_vec());
let txs_for_rollup: &mut Vec<Bytes> =
map.entry(action.rollup_id).or_insert(vec![Bytes::new()]);
let rollup_data = RollupData::SequencedData(action.data.clone());
txs_for_rollup.push(rollup_data.into_raw().encode_to_vec().into());
}
}
map.sort_unstable_keys();
Expand Down
7 changes: 6 additions & 1 deletion crates/astria-core/src/protocol/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use std::collections::HashMap;

use bytes::Bytes;
use prost::Message as _;

use super::{
Expand Down Expand Up @@ -132,6 +133,7 @@ impl ConfigureSequencerBlock {
RollupData::Deposit(Box::new(deposit))
.into_raw()
.encode_to_vec()
.into()
}));
}
rollup_transactions.sort_unstable_keys();
Expand All @@ -148,7 +150,10 @@ impl ConfigureSequencerBlock {
rollup_ids_root.to_vec(),
];
data.extend(txs.into_iter().map(|tx| tx.into_raw().encode_to_vec()));

let data = data
.into_iter()
.map(|data| Bytes::copy_from_slice(&data))
.collect();
SequencerBlock::try_from_block_info_and_data(
block_hash,
chain_id.try_into().unwrap(),
Expand Down
47 changes: 22 additions & 25 deletions crates/astria-core/src/sequencerblock/v1alpha1/block.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use std::collections::HashMap;

use bytes::{
Buf as _,
Bytes,
};
use bytes::Bytes;
use indexmap::IndexMap;
use sha2::Sha256;
use tendermint::{
Expand Down Expand Up @@ -72,7 +69,7 @@ enum RollupTransactionsErrorKind {
#[derive(Clone, Debug, PartialEq)]
pub struct RollupTransactionsParts {
pub rollup_id: RollupId,
pub transactions: Vec<Vec<u8>>,
pub transactions: Vec<Bytes>,
pub proof: merkle::Proof,
}

Expand All @@ -82,7 +79,7 @@ pub struct RollupTransactions {
/// The 32 bytes identifying a rollup. Usually the sha256 hash of a plain rollup name.
rollup_id: RollupId,
/// The block data for this rollup in the form of encoded [`RollupData`].
transactions: Vec<Vec<u8>>,
transactions: Vec<Bytes>,
/// Proof that this set of transactions belongs in the rollup datas merkle tree
proof: merkle::Proof,
}
Expand All @@ -96,7 +93,7 @@ impl RollupTransactions {

/// Returns the block data for this rollup.
#[must_use]
pub fn transactions(&self) -> &[Vec<u8>] {
pub fn transactions(&self) -> &[Bytes] {
&self.transactions
}

Expand Down Expand Up @@ -202,7 +199,7 @@ impl SequencerBlockError {
Self(SequencerBlockErrorKind::NoRollupTransactionsRoot)
}

fn incorrect_rollup_transactions_root_length(len: usize) -> Self {
fn incorrect_rollup_tx_root_length(len: usize) -> Self {
Self(SequencerBlockErrorKind::IncorrectRollupTransactionsRootLength(len))
}

Expand Down Expand Up @@ -454,13 +451,13 @@ impl SequencerBlockHeader {
.map_err(SequencerBlockHeaderError::time)?;

let rollup_transactions_root =
rollup_transactions_root.chunk().try_into().map_err(|_| {
rollup_transactions_root.as_ref().try_into().map_err(|_| {
SequencerBlockHeaderError::incorrect_rollup_transactions_root_length(
rollup_transactions_root.len(),
)
})?;

let data_hash = data_hash.chunk().try_into().map_err(|_| {
let data_hash = data_hash.as_ref().try_into().map_err(|_| {
SequencerBlockHeaderError::incorrect_rollup_transactions_root_length(data_hash.len())
})?;

Expand Down Expand Up @@ -712,7 +709,7 @@ impl SequencerBlock {
height: tendermint::block::Height,
time: Time,
proposer_address: account::Id,
data: Vec<Vec<u8>>,
data: Vec<Bytes>,
deposits: HashMap<RollupId, Vec<Deposit>>,
) -> Result<Self, SequencerBlockError> {
use prost::Message as _;
Expand All @@ -724,16 +721,16 @@ impl SequencerBlock {
let rollup_transactions_root: [u8; 32] = data_list
.next()
.ok_or(SequencerBlockError::no_rollup_transactions_root())?
.as_ref()
.try_into()
.map_err(|e: Vec<_>| {
SequencerBlockError::incorrect_rollup_transactions_root_length(e.len())
})?;
.map_err(|_| SequencerBlockError::incorrect_rollup_tx_root_length(data_list.len()))?;

let rollup_ids_root: [u8; 32] = data_list
.next()
.ok_or(SequencerBlockError::no_rollup_ids_root())?
.as_ref()
.try_into()
.map_err(|e: Vec<_>| SequencerBlockError::incorrect_rollup_ids_root_length(e.len()))?;
.map_err(|_| SequencerBlockError::incorrect_rollup_ids_root_length(data_list.len()))?;

let mut rollup_datas = IndexMap::new();
for elem in data_list {
Expand All @@ -752,10 +749,11 @@ impl SequencerBlock {
fee_asset: _,
}) = action
{
let elem = rollup_datas.entry(rollup_id).or_insert(vec![]);
let data = RollupData::SequencedData(data.to_vec())
let elem = rollup_datas.entry(rollup_id).or_insert(vec![Bytes::new()]);
let data = RollupData::SequencedData(data)
.into_raw()
.encode_to_vec();
.encode_to_vec()
.into();
elem.push(data);
}
}
Expand All @@ -768,6 +766,7 @@ impl SequencerBlock {
RollupData::Deposit(Box::new(deposit))
.into_raw()
.encode_to_vec()
.into()
}));
}

Expand Down Expand Up @@ -854,7 +853,7 @@ impl SequencerBlock {
} = raw;

let block_hash = block_hash
.chunk()
.as_ref()
.try_into()
.map_err(|_| SequencerBlockError::invalid_block_hash(block_hash.len()))?;

Expand Down Expand Up @@ -1076,7 +1075,7 @@ impl FilteredSequencerBlock {
} = raw;

let block_hash = block_hash
.chunk()
.as_ref()
.try_into()
.map_err(|_| FilteredSequencerBlockError::invalid_block_hash(block_hash.len()))?;

Expand Down Expand Up @@ -1439,7 +1438,7 @@ enum DepositErrorKind {
/// and must decode it accordingly.
#[derive(Debug, Clone, PartialEq)]
pub enum RollupData {
SequencedData(Vec<u8>),
SequencedData(Bytes),
Deposit(Box<Deposit>),
}

Expand All @@ -1448,7 +1447,7 @@ impl RollupData {
pub fn into_raw(self) -> raw::RollupData {
match self {
Self::SequencedData(data) => raw::RollupData {
value: Some(raw::rollup_data::Value::SequencedData(data.into())),
value: Some(raw::rollup_data::Value::SequencedData(data)),
},
Self::Deposit(deposit) => raw::RollupData {
value: Some(raw::rollup_data::Value::Deposit(deposit.into_raw())),
Expand All @@ -1467,9 +1466,7 @@ impl RollupData {
value,
} = raw;
match value {
Some(raw::rollup_data::Value::SequencedData(data)) => {
Ok(Self::SequencedData(data.to_vec()))
}
Some(raw::rollup_data::Value::SequencedData(data)) => Ok(Self::SequencedData(data)),
Some(raw::rollup_data::Value::Deposit(deposit)) => Deposit::try_from_raw(deposit)
.map(Box::new)
.map(Self::Deposit)
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-core/src/sequencerblock/v1alpha1/celestia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ impl UncheckedSubmittedMetadata {
}?;

let block_hash = block_hash
.chunk()
.as_ref()
.try_into()
.map_err(|_| SubmittedMetadataError::block_hash(block_hash.len()))?;

Expand Down
2 changes: 1 addition & 1 deletion crates/astria-sequencer/src/app/tests_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() {
for (_, rollup_data) in block.rollup_transactions() {
for tx in rollup_data.transactions() {
let rollup_data =
RollupData::try_from_raw(RawRollupData::decode(tx.as_slice()).unwrap()).unwrap();
RollupData::try_from_raw(RawRollupData::decode(tx.as_ref()).unwrap()).unwrap();
if let RollupData::Deposit(deposit) = rollup_data {
deposits.push(deposit);
}
Expand Down
Loading

0 comments on commit d92a373

Please sign in to comment.