Skip to content

Commit

Permalink
Simplify narwhal-sui interface (#686)
Browse files Browse the repository at this point in the history
  • Loading branch information
asonnino authored Aug 5, 2022
1 parent 5fe1489 commit bc6b4dd
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 37 deletions.
13 changes: 2 additions & 11 deletions narwhal/executor/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::{
state::ExecutionIndices,
ExecutionState, ExecutorOutput, SerializedTransaction,
};
use config::Committee;
use consensus::ConsensusOutput;
use std::{fmt::Debug, sync::Arc};
use store::Store;
Expand Down Expand Up @@ -178,15 +177,7 @@ where
.await;

let (bail, result) = match result {
Ok((outcome, committee)) => {
if let Some(_committee) = committee {
// TODO: Do we really need to receive back the committee here? We will know
// once we try to integrate Narwhal with Sui. #580
// todo!();
}

(None, Ok(outcome))
}
outcome @ Ok(..) => (None, outcome),

// We may want to log the errors that are the user's fault (i.e., that are neither
// our fault or the fault of consensus) for debug purposes. It is safe to continue
Expand Down Expand Up @@ -224,7 +215,7 @@ where
serialized: SerializedTransaction,
total_transactions: usize,
total_batches: usize,
) -> SubscriberResult<(<State as ExecutionState>::Outcome, Option<Committee>)> {
) -> SubscriberResult<<State as ExecutionState>::Outcome> {
// Compute the next expected indices. Those will be persisted upon transaction execution
// and are only used for crash-recovery.
self.execution_indices
Expand Down
5 changes: 1 addition & 4 deletions narwhal/executor/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,9 @@ impl From<Box<bincode::ErrorKind>> for SubscriberError {

/// Trait do separate execution errors in two categories: (i) errors caused by a bad client, (ii)
/// errors caused by a fault in the authority.
pub trait ExecutionStateError: Debug {
pub trait ExecutionStateError: std::error::Error {
/// Whether the error is due to a fault in the authority (eg. internal storage error).
fn node_error(&self) -> bool;

/// Convert the error message in to a string.
fn to_string(&self) -> String;
}

impl<T: ExecutionStateError> From<T> for SubscriberError {
Expand Down
4 changes: 2 additions & 2 deletions narwhal/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub use state::ExecutionIndices;

use crate::{batch_loader::BatchLoader, core::Core, subscriber::Subscriber};
use async_trait::async_trait;
use config::{Committee, SharedCommittee};
use config::SharedCommittee;
use consensus::{ConsensusOutput, ConsensusSyncRequest};
use crypto::PublicKey;
use serde::de::DeserializeOwned;
Expand Down Expand Up @@ -67,7 +67,7 @@ pub trait ExecutionState {
consensus_output: &ConsensusOutput,
execution_indices: ExecutionIndices,
transaction: Self::Transaction,
) -> Result<(Self::Outcome, Option<Committee>), Self::Error>;
) -> Result<Self::Outcome, Self::Error>;

/// Simple guardrail ensuring there is a single instance using the state
/// to call `handle_consensus_transaction`. Many instances may read the state,
Expand Down
9 changes: 2 additions & 7 deletions narwhal/executor/src/tests/execution_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0
use crate::{ExecutionIndices, ExecutionState, ExecutionStateError};
use async_trait::async_trait;
use config::Committee;
use consensus::ConsensusOutput;

use futures::executor::block_on;
Expand Down Expand Up @@ -48,7 +47,7 @@ impl ExecutionState for TestState {
_consensus_output: &ConsensusOutput,
execution_indices: ExecutionIndices,
transaction: Self::Transaction,
) -> Result<(Self::Outcome, Option<Committee>), Self::Error> {
) -> Result<Self::Outcome, Self::Error> {
if transaction == MALFORMED_TRANSACTION {
Err(Self::Error::ClientError)
} else if transaction == KILLER_TRANSACTION {
Expand All @@ -57,7 +56,7 @@ impl ExecutionState for TestState {
self.store
.write(Self::INDICES_ADDRESS, execution_indices)
.await;
Ok((Vec::default(), None))
Ok(Vec::default())
}
}

Expand Down Expand Up @@ -115,8 +114,4 @@ impl ExecutionStateError for TestStateError {
Self::ClientError => false,
}
}

fn to_string(&self) -> String {
ToString::to_string(&self)
}
}
9 changes: 2 additions & 7 deletions narwhal/node/src/execution_state.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0
use async_trait::async_trait;
use config::Committee;
use consensus::ConsensusOutput;
use executor::{ExecutionIndices, ExecutionState, ExecutionStateError};
use thiserror::Error;
Expand All @@ -20,8 +19,8 @@ impl ExecutionState for SimpleExecutionState {
_consensus_output: &ConsensusOutput,
_execution_indices: ExecutionIndices,
_transaction: Self::Transaction,
) -> Result<(Self::Outcome, Option<Committee>), Self::Error> {
Ok((Vec::default(), None))
) -> Result<Self::Outcome, Self::Error> {
Ok(Vec::default())
}

fn ask_consensus_write_lock(&self) -> bool {
Expand Down Expand Up @@ -59,8 +58,4 @@ impl ExecutionStateError for SimpleExecutionError {
Self::ClientError => false,
}
}

fn to_string(&self) -> String {
ToString::to_string(&self)
}
}
8 changes: 2 additions & 6 deletions narwhal/node/tests/reconfigure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl ExecutionState for SimpleExecutionState {
_consensus_output: &ConsensusOutput,
execution_indices: ExecutionIndices,
transaction: Self::Transaction,
) -> Result<(Self::Outcome, Option<Committee>), Self::Error> {
) -> Result<Self::Outcome, Self::Error> {
// Change epoch every few certificates. Note that empty certificates are not provided to
// this function (they are immediately skipped).
let mut epoch = self.committee.lock().unwrap().epoch();
Expand All @@ -80,7 +80,7 @@ impl ExecutionState for SimpleExecutionState {
.unwrap();
}

Ok((epoch, None))
Ok(epoch)
}

fn ask_consensus_write_lock(&self) -> bool {
Expand Down Expand Up @@ -112,10 +112,6 @@ impl ExecutionStateError for SimpleExecutionError {
Self::ClientError => false,
}
}

fn to_string(&self) -> String {
ToString::to_string(&self)
}
}

async fn run_client(name: PublicKey, committee: Committee, mut rx_reconfigure: Receiver<u64>) {
Expand Down

0 comments on commit bc6b4dd

Please sign in to comment.