Skip to content

Commit

Permalink
Enforce max_block_size
Browse files Browse the repository at this point in the history
Omit transactions that exceed max_block_size from payload. Use mocked
`max_block_size` for now.

  * add test `enforce_max_block_size`

This hasn't really be tested yet, but it is how I plan on testing this.

  * remove hotshot-web-server
  • Loading branch information
tbro committed May 6, 2024
1 parent bf13357 commit b9d4f48
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 34 deletions.
21 changes: 10 additions & 11 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,18 @@ dotenvy = "0.15"
ethers = { version = "2.0", features = ["solc"] }
futures = "0.3"

hotshot = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot = { git = "https://github.com/EspressoSystems/hotshot", rev = "6d293ae9b" }
# Hotshot imports
hotshot-builder-api = { git = "https://github.com/EspressoSystems/HotShot.git", tag = "0.5.48" }
hotshot-builder-core = { git = "https://github.com/EspressoSystems/hotshot-builder-core", tag = "0.1.17" }
hotshot-events-service = { git = "https://github.com/EspressoSystems/hotshot-events-service.git", tag = "0.1.18" }
hotshot-orchestrator = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot-query-service = { git = "https://github.com/EspressoSystems/hotshot-query-service", tag = "0.1.16" }
hotshot-stake-table = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot-builder-api = { git = "https://github.com/EspressoSystems/HotShot.git", rev = "6d293ae9b" }
hotshot-builder-core = { git = "https://github.com/EspressoSystems/hotshot-builder-core", branch = "instance_state_by_ref" }
hotshot-events-service = { git = "https://github.com/EspressoSystems/hotshot-events-service.git", branch = "tbro/update-hotshot" }
hotshot-orchestrator = { git = "https://github.com/EspressoSystems/hotshot", rev = "6d293ae9b" }
hotshot-query-service = { git = "https://github.com/EspressoSystems/hotshot-query-service", branch = "update-hotshot" }
hotshot-stake-table = { git = "https://github.com/EspressoSystems/hotshot", rev = "6d293ae9b" }
hotshot-state-prover = { version = "0.1.0", path = "hotshot-state-prover" }
hotshot-task = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot-testing = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot-types = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot-web-server = { git = "https://github.com/EspressoSystems/hotshot", tag = "0.5.48" }
hotshot-task = { git = "https://github.com/EspressoSystems/hotshot", rev = "6d293ae9b" }
hotshot-testing = { git = "https://github.com/EspressoSystems/hotshot", rev = "6d293ae9b" }
hotshot-types = { git = "https://github.com/EspressoSystems/hotshot", rev = "6d293ae9b" }

# Push CDN imports
cdn-broker = { git = "https://github.com/EspressoSystems/Push-CDN", features = [
Expand Down
5 changes: 2 additions & 3 deletions builder/src/non_permissioned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ impl BuilderConfig {
// builder api request channel
let (req_sender, req_receiver) = broadcast::<MessageType<SeqTypes>>(channel_capacity.get());

let (genesis_payload, genesis_ns_table) =
Payload::from_transactions([], Arc::new(instance_state.clone()))
.expect("genesis payload construction failed");
let (genesis_payload, genesis_ns_table) = Payload::from_transactions([], &instance_state)
.expect("genesis payload construction failed");

let builder_commitment = genesis_payload.builder_commitment(&genesis_ns_table);

Expand Down
5 changes: 2 additions & 3 deletions builder/src/permissioned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,8 @@ impl<N: network::Type, P: SequencerPersistence, Ver: StaticVersionType + 'static
// builder api request channel
let (req_sender, req_receiver) = broadcast::<MessageType<SeqTypes>>(channel_capacity.get());

let (genesis_payload, genesis_ns_table) =
Payload::from_transactions([], Arc::new(instance_state.clone()))
.expect("genesis payload construction failed");
let (genesis_payload, genesis_ns_table) = Payload::from_transactions([], &instance_state)
.expect("genesis payload construction failed");

let builder_commitment = genesis_payload.builder_commitment(&genesis_ns_table);

Expand Down
1 change: 0 additions & 1 deletion sequencer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ hotshot-task = { workspace = true }
# Dependencies for feature `testing`
hotshot-testing = { workspace = true, optional = true }
hotshot-types = { workspace = true }
hotshot-web-server = { workspace = true }
include_dir = "0.7"
itertools = { workspace = true }

Expand Down
12 changes: 6 additions & 6 deletions sequencer/src/block.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use std::sync::Arc;

use crate::{BlockBuildingSnafu, NodeState, Transaction};
use committable::{Commitment, Committable};
use hotshot_query_service::availability::QueryablePayload;
use hotshot_types::traits::{states::InstanceState, BlockPayload};
use hotshot_types::traits::BlockPayload;
use hotshot_types::utils::BuilderCommitment;
use serde::{Deserialize, Serialize};
use sha2::Digest;
use snafu::OptionExt;
use std::sync::Arc;

pub mod entry;
pub mod payload;
Expand All @@ -24,6 +23,7 @@ pub type NsTable = NameSpaceTable<TxTableEntryWord>;
impl BlockPayload for Payload<TxTableEntryWord> {
type Error = crate::Error;
type Transaction = Transaction;
type Instance = NodeState;
type Metadata = NsTable;

/// Returns (Self, metadata).
Expand All @@ -48,9 +48,9 @@ impl BlockPayload for Payload<TxTableEntryWord> {
/// TODO(746) refactor and make pretty "table" code for tx, namespace tables?
fn from_transactions(
txs: impl IntoIterator<Item = Self::Transaction>,
_state: Arc<dyn InstanceState>,
instance_state: &Self::Instance,
) -> Result<(Self, Self::Metadata), Self::Error> {
let payload = Payload::from_txs(txs)?;
let payload = Payload::from_txs(txs, &instance_state.chain_config)?;
let ns_table = payload.get_ns_table().clone(); // TODO don't clone ns_table
Some((payload, ns_table)).context(BlockBuildingSnafu)
}
Expand All @@ -69,7 +69,7 @@ impl BlockPayload for Payload<TxTableEntryWord> {
// use the mock NodeState. A future update to HotShot should
// make a change there to remove the need for this workaround.

Self::from_transactions([], Arc::new(NodeState::mock())).unwrap()
Self::from_transactions([], &NodeState::mock()).unwrap()
}

fn encode(&self) -> Result<Arc<[u8]>, Self::Error> {
Expand Down
78 changes: 68 additions & 10 deletions sequencer/src/block/payload.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::block::entry::{TxTableEntry, TxTableEntryWord};
use crate::block::payload;
use crate::{BlockBuildingSnafu, Error, NamespaceId, NodeState, Transaction};
use crate::block::tables::NameSpaceTable;
use crate::block::tables::TxTable;
use crate::{BlockBuildingSnafu, ChainConfig, Error, NamespaceId, Transaction};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use derivative::Derivative;
use hotshot::traits::BlockPayload;
Expand All @@ -15,14 +17,9 @@ use num_traits::PrimInt;
use serde::{Deserialize, Serialize};
use snafu::OptionExt;
use std::default::Default;
use std::sync::Arc;
use std::{collections::HashMap, fmt::Display};

use crate::block::tables::NameSpaceTable;
use trait_set::trait_set;

use crate::block::tables::TxTable;

trait_set! {

pub trait TableWordTraits = CanonicalSerialize
Expand Down Expand Up @@ -155,13 +152,19 @@ impl<TableWord: TableWordTraits> Payload<TableWord> {

pub fn from_txs(
txs: impl IntoIterator<Item = <payload::Payload<TxTableEntryWord> as BlockPayload>::Transaction>,
chain_config: &ChainConfig,
) -> Result<Self, Error> {
let mut namespaces: HashMap<NamespaceId, NamespaceInfo> = Default::default();
let mut structured_payload = Self {
raw_payload: vec![],
ns_table: NameSpaceTable::default(),
};
let mut block_size = 0u64;
for tx in txs.into_iter() {
block_size += tx.payload().len() as u64;
if block_size >= chain_config.max_block_size() {
break;
}
Payload::<TableWord>::update_namespace_with_tx(&mut namespaces, tx);
}

Expand Down Expand Up @@ -309,7 +312,7 @@ impl hotshot_types::traits::block_contents::TestableBlock
for Payload<crate::block::entry::TxTableEntryWord>
{
fn genesis() -> Self {
BlockPayload::from_transactions([], Arc::new(NodeState::mock()))
BlockPayload::from_transactions([], &Default::default())
.unwrap()
.0
}
Expand All @@ -332,7 +335,7 @@ mod test {
tx_iterator::TxIndex,
},
transaction::NamespaceId,
Transaction,
NodeState, Transaction,
};
use async_compatibility_layer::logging::{setup_backtrace, setup_logging};
use helpers::*;
Expand All @@ -344,6 +347,62 @@ mod test {

const NUM_STORAGE_NODES: usize = 10;

#[test]
fn enforce_max_block_size() {
let mut rng = jf_utils::test_rng();
let max_block_size = 1000u64;
let payload_size = 10;

let len = max_block_size;
// let mut txs: Vec<Transaction> = vec![];
let mut txs = (0..max_block_size / payload_size)
.map(|_| Transaction::of_size(&mut rng, payload_size))
.collect::<Vec<Transaction>>();

// should panic b/c txs size > max_block_size
txns.push(Transaction::of_size(&mut rng, payload_size));
Payload::<TxTableEntryWord>::from_txs(txs, max_block_size).unwrap();

// should panic b/c txs size = max_block_size
txns.pop(Transaction::of_size(&mut rng, payload_size));
Payload::<TxTableEntryWord>::from_txs(txs, max_block_size).unwrap();

// should succeed b/c txs size < max_block_size
txns.pop(Transaction::of_size(&mut rng, payload_size));
Payload::<TxTableEntryWord>::from_txs(txs, max_block_size).unwrap();

// let mut block_size = 0u64;
// loop {
// if block_size < max_block_size {
// let tx = Transaction::of_size(&mut rng, max_block_size / 100);
// block_size += tx.payload().len() as u64;
// txs.push(tx);
// } else {
// break;
// }
// }
Payload::<TxTableEntryWord>::from_txs(txs, max_block_size).unwrap();
}

#[test]
fn basic_correctness() {
check_basic_correctness::<TxTableEntryWord>()
}

fn check_basic_correctness<TableWord: TableWordTraits>() {
// let mut block_size = 0u64;
// loop {
// if block_size < max_block_size {
// let tx = Transaction::of_size(&mut rng, max_block_size / 100);
// block_size += tx.payload().len() as u64;
// txs.push(tx);
// } else {
// break;
// }
// }
Payload::<TxTableEntryWord>::from_txs(txs, max_block_size).unwrap();
}

#[test]
fn basic_correctness() {
check_basic_correctness::<TxTableEntryWord>()
Expand Down Expand Up @@ -464,8 +523,7 @@ mod test {
.iter()
.flat_map(|(_ns_id, ns)| ns.txs.iter().cloned());
let (block, actual_ns_table) =
Payload::from_transactions(all_txs_iter, Arc::new(crate::NodeState::mock()))
.unwrap();
Payload::from_transactions(all_txs_iter, Arc::new(NodeState::mock())).unwrap();
let disperse_data = vid.disperse(&block.raw_payload).unwrap();

// TEST ACTUAL STUFF AGAINST DERIVED STUFF
Expand Down
14 changes: 14 additions & 0 deletions sequencer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,20 @@ impl NodeState {
}
}

// This allows us to turn on `Default` on InstanceState trait
// which is used in `HotShot` by `TestBuilderImplementation`.
#[cfg(any(test, feature = "testing"))]
impl Default for NodeState {
fn default() -> Self {
Self::new(
1u64,
ChainConfig::default(),
L1Client::new("http://localhost:3331".parse().unwrap(), Address::default()),
catchup::mock::MockStateCatchup::default(),
)
}
}

impl InstanceState for NodeState {}

impl NodeType for SeqTypes {
Expand Down
9 changes: 9 additions & 0 deletions sequencer/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ impl Transaction {
(0..len).map(|_| rand::random::<u8>()).collect::<Vec<_>>(),
)
}
#[cfg(any(test, feature = "testing"))]
/// Useful for when we want to test size of transaction(s)
pub fn of_size(rng: &mut dyn rand::RngCore, len: usize) -> Self {
use rand::Rng;
Self::new(
NamespaceId(rng.gen_range(0..10)),
(0..len).map(|_| rand::random::<u8>()).collect::<Vec<_>>(),
)
}
}

impl HotShotTransaction for Transaction {}
Expand Down

0 comments on commit b9d4f48

Please sign in to comment.