Skip to content

Commit

Permalink
[Narwhal] remove NarwhalManager, LazyNarwhalClient, TransactionValida…
Browse files Browse the repository at this point in the history
…tor and a few other integrations (#19089)

## Description 

Remove a few integration points between Narwhal and Sui.
Remove some usages of Narwhal types in Sui.

## Test plan 

CI

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
mwtian authored Aug 26, 2024
1 parent f3f3014 commit d7c986c
Show file tree
Hide file tree
Showing 29 changed files with 199 additions and 966 deletions.
9 changes: 0 additions & 9 deletions Cargo.lock

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

29 changes: 14 additions & 15 deletions consensus/core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,8 @@ impl VerifiedBlock {
}
}

#[cfg(test)]
pub(crate) fn new_for_test(block: Block) -> Self {
/// This method is public for testing in other crates.
pub fn new_for_test(block: Block) -> Self {
// Use empty signature in test.
let signed_block = SignedBlock {
inner: block,
Expand All @@ -462,7 +462,7 @@ impl VerifiedBlock {
}

/// Returns reference to the block.
pub(crate) fn reference(&self) -> BlockRef {
pub fn reference(&self) -> BlockRef {
BlockRef {
round: self.round(),
author: self.author(),
Expand Down Expand Up @@ -544,15 +544,14 @@ pub(crate) fn genesis_blocks(context: Arc<Context>) -> Vec<VerifiedBlock> {
}

/// Creates fake blocks for testing.
#[cfg(test)]
/// This struct is public for testing in other crates.
#[derive(Clone)]
pub(crate) struct TestBlock {
pub struct TestBlock {
block: BlockV1,
}

#[cfg(test)]
impl TestBlock {
pub(crate) fn new(round: Round, author: u32) -> Self {
pub fn new(round: Round, author: u32) -> Self {
Self {
block: BlockV1 {
round,
Expand All @@ -562,42 +561,42 @@ impl TestBlock {
}
}

pub(crate) fn set_epoch(mut self, epoch: Epoch) -> Self {
pub fn set_epoch(mut self, epoch: Epoch) -> Self {
self.block.epoch = epoch;
self
}

pub(crate) fn set_round(mut self, round: Round) -> Self {
pub fn set_round(mut self, round: Round) -> Self {
self.block.round = round;
self
}

pub(crate) fn set_author(mut self, author: AuthorityIndex) -> Self {
pub fn set_author(mut self, author: AuthorityIndex) -> Self {
self.block.author = author;
self
}

pub(crate) fn set_timestamp_ms(mut self, timestamp_ms: BlockTimestampMs) -> Self {
pub fn set_timestamp_ms(mut self, timestamp_ms: BlockTimestampMs) -> Self {
self.block.timestamp_ms = timestamp_ms;
self
}

pub(crate) fn set_ancestors(mut self, ancestors: Vec<BlockRef>) -> Self {
pub fn set_ancestors(mut self, ancestors: Vec<BlockRef>) -> Self {
self.block.ancestors = ancestors;
self
}

pub(crate) fn set_transactions(mut self, transactions: Vec<Transaction>) -> Self {
pub fn set_transactions(mut self, transactions: Vec<Transaction>) -> Self {
self.block.transactions = transactions;
self
}

pub(crate) fn set_commit_votes(mut self, commit_votes: Vec<CommitVote>) -> Self {
pub fn set_commit_votes(mut self, commit_votes: Vec<CommitVote>) -> Self {
self.block.commit_votes = commit_votes;
self
}

pub(crate) fn build(self) -> Block {
pub fn build(self) -> Block {
Block::V1(self.block)
}
}
Expand Down
20 changes: 7 additions & 13 deletions consensus/core/src/block_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,7 @@ impl BlockVerifier for SignedBlockVerifier {
let batch: Vec<_> = block.transactions().iter().map(|t| t.data()).collect();

let max_transaction_size_limit =
self.context
.protocol_config
.consensus_max_transaction_size_bytes() as usize;
self.context.protocol_config.max_transaction_size_bytes() as usize;
for t in &batch {
if t.len() > max_transaction_size_limit && max_transaction_size_limit > 0 {
return Err(ConsensusError::TransactionTooLarge {
Expand All @@ -166,10 +164,10 @@ impl BlockVerifier for SignedBlockVerifier {
});
}

let total_transactions_size_limit =
self.context
.protocol_config
.consensus_max_transactions_in_block_bytes() as usize;
let total_transactions_size_limit = self
.context
.protocol_config
.max_transactions_in_block_bytes() as usize;
if batch.iter().map(|t| t.len()).sum::<usize>() > total_transactions_size_limit
&& total_transactions_size_limit > 0
{
Expand All @@ -180,7 +178,7 @@ impl BlockVerifier for SignedBlockVerifier {
}

self.transaction_verifier
.verify_batch(&self.context.protocol_config, &batch)
.verify_batch(&batch)
.map_err(|e| ConsensusError::InvalidTransaction(format!("{e:?}")))
}

Expand Down Expand Up @@ -238,11 +236,7 @@ mod test {

impl TransactionVerifier for TxnSizeVerifier {
// Fails verification if any transaction is < 4 bytes.
fn verify_batch(
&self,
_protocol_config: &sui_protocol_config::ProtocolConfig,
transactions: &[&[u8]],
) -> Result<(), ValidationError> {
fn verify_batch(&self, transactions: &[&[u8]]) -> Result<(), ValidationError> {
for txn in transactions {
if txn.len() < 4 {
return Err(ValidationError::InvalidTransaction(format!(
Expand Down
6 changes: 3 additions & 3 deletions consensus/core/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ pub struct CommitRef {
}

impl CommitRef {
pub(crate) fn new(index: CommitIndex, digest: CommitDigest) -> Self {
pub fn new(index: CommitIndex, digest: CommitDigest) -> Self {
Self { index, digest }
}
}
Expand Down Expand Up @@ -305,8 +305,8 @@ pub struct CommittedSubDag {
}

impl CommittedSubDag {
/// Create new (empty) sub-dag.
pub(crate) fn new(
/// Creates a new committed sub dag.
pub fn new(
leader: BlockRef,
blocks: Vec<VerifiedBlock>,
timestamp_ms: BlockTimestampMs,
Expand Down
7 changes: 1 addition & 6 deletions consensus/core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1239,12 +1239,7 @@ mod test {
let transaction: String = bcs::from_bytes(transaction.data()).unwrap();
assert_eq!(format!("Transaction {i}"), transaction);
}
assert!(
total
<= context
.protocol_config
.consensus_max_transactions_in_block_bytes()
);
assert!(total <= context.protocol_config.max_transactions_in_block_bytes());

// genesis blocks should be referenced
let all_genesis = genesis_blocks(context);
Expand Down
13 changes: 8 additions & 5 deletions consensus/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ mod block_manager;
mod block_verifier;
mod broadcaster;
mod commit;
mod commit_consumer;
mod commit_observer;
mod commit_syncer;
mod commit_vote_monitor;
mod context;
mod core;
mod core_thread;
Expand All @@ -31,21 +33,22 @@ mod threshold_clock;
mod transaction;
mod universal_committer;

mod commit_consumer;
mod commit_vote_monitor;
#[cfg(test)]
#[path = "tests/randomized_tests.rs"]
mod randomized_tests;
#[cfg(test)]
mod test_dag;
#[cfg(test)]
mod test_dag_builder;
#[cfg(test)]
mod test_dag_parser;

/// Exported consensus API.
pub use authority_node::ConsensusAuthority;
pub use block::{BlockAPI, Round};
pub use commit::{CommitDigest, CommitIndex, CommitRef, CommittedSubDag};
pub use commit_consumer::{CommitConsumer, CommitConsumerMonitor};
pub use transaction::{ClientError, TransactionClient, TransactionVerifier, ValidationError};

#[cfg(test)]
#[path = "tests/randomized_tests.rs"]
mod randomized_tests;
/// Exported API for testing.
pub use block::{TestBlock, Transaction, VerifiedBlock};
46 changes: 10 additions & 36 deletions consensus/core/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use std::sync::Arc;

use mysten_metrics::monitored_mpsc::{channel, Receiver, Sender};
use sui_protocol_config::ProtocolConfig;
use tap::tap::TapFallible;
use thiserror::Error;
use tokio::sync::oneshot;
Expand Down Expand Up @@ -46,7 +45,7 @@ impl TransactionConsumer {
tx_receiver,
max_consumed_bytes_per_request: context
.protocol_config
.consensus_max_transactions_in_block_bytes(),
.max_transactions_in_block_bytes(),
max_consumed_transactions_per_request: context
.protocol_config
.max_num_transactions_in_block(),
Expand Down Expand Up @@ -158,9 +157,7 @@ impl TransactionClient {
(
Self {
sender,
max_transaction_size: context
.protocol_config
.consensus_max_transaction_size_bytes(),
max_transaction_size: context.protocol_config.max_transaction_size_bytes(),
},
receiver,
)
Expand Down Expand Up @@ -215,11 +212,7 @@ impl TransactionClient {
/// before acceptance of the block.
pub trait TransactionVerifier: Send + Sync + 'static {
/// Determines if this batch can be voted on
fn verify_batch(
&self,
protocol_config: &ProtocolConfig,
batch: &[&[u8]],
) -> Result<(), ValidationError>;
fn verify_batch(&self, batch: &[&[u8]]) -> Result<(), ValidationError>;
}

#[derive(Debug, Error)]
Expand All @@ -233,11 +226,7 @@ pub enum ValidationError {
pub(crate) struct NoopTransactionVerifier;

impl TransactionVerifier for NoopTransactionVerifier {
fn verify_batch(
&self,
_protocol_config: &ProtocolConfig,
_batch: &[&[u8]],
) -> Result<(), ValidationError> {
fn verify_batch(&self, _batch: &[&[u8]]) -> Result<(), ValidationError> {
Ok(())
}
}
Expand Down Expand Up @@ -338,14 +327,9 @@ mod tests {
// ensure their total size is less than `max_bytes_to_fetch`
let total_size: u64 = transactions.iter().map(|t| t.data().len() as u64).sum();
assert!(
total_size
<= context
.protocol_config
.consensus_max_transactions_in_block_bytes(),
total_size <= context.protocol_config.max_transactions_in_block_bytes(),
"Should have fetched transactions up to {}",
context
.protocol_config
.consensus_max_transactions_in_block_bytes()
context.protocol_config.max_transactions_in_block_bytes()
);
all_transactions.extend(transactions);

Expand All @@ -356,14 +340,9 @@ mod tests {
// ensure their total size is less than `max_bytes_to_fetch`
let total_size: u64 = transactions.iter().map(|t| t.data().len() as u64).sum();
assert!(
total_size
<= context
.protocol_config
.consensus_max_transactions_in_block_bytes(),
total_size <= context.protocol_config.max_transactions_in_block_bytes(),
"Should have fetched transactions up to {}",
context
.protocol_config
.consensus_max_transactions_in_block_bytes()
context.protocol_config.max_transactions_in_block_bytes()
);
all_transactions.extend(transactions);

Expand Down Expand Up @@ -435,14 +414,9 @@ mod tests {

let total_size: u64 = transactions.iter().map(|t| t.data().len() as u64).sum();
assert!(
total_size
<= context
.protocol_config
.consensus_max_transactions_in_block_bytes(),
total_size <= context.protocol_config.max_transactions_in_block_bytes(),
"Should have fetched transactions up to {}",
context
.protocol_config
.consensus_max_transactions_in_block_bytes()
context.protocol_config.max_transactions_in_block_bytes()
);

all_transactions.extend(transactions);
Expand Down
4 changes: 0 additions & 4 deletions crates/sui-benchmark/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ fastcrypto-zkp.workspace = true

move-core-types.workspace = true
mysten-metrics.workspace = true
narwhal-node.workspace = true
test-cluster.workspace = true
sysinfo.workspace = true

Expand All @@ -58,6 +57,3 @@ sui-framework-snapshot.workspace = true
sui-macros.workspace = true
sui-simulator.workspace = true
typed-store.workspace = true

[features]
benchmark = ["narwhal-node/benchmark"]
5 changes: 0 additions & 5 deletions crates/sui-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,7 @@ typed-store.workspace = true
mysten-metrics.workspace = true
narwhal-config.workspace = true
narwhal-crypto.workspace = true
narwhal-executor.workspace = true
narwhal-network.workspace = true
narwhal-node.workspace = true
narwhal-test-utils.workspace = true
narwhal-types.workspace = true
narwhal-worker.workspace = true
shared-crypto.workspace = true
sui-archival.workspace = true
sui-config.workspace = true
Expand Down
Loading

0 comments on commit d7c986c

Please sign in to comment.