Skip to content

Commit

Permalink
#4039 remove stake_table check in libp2p, rename stake_table to membe…
Browse files Browse the repository at this point in the history
…rship
  • Loading branch information
pls148 committed Feb 4, 2025
1 parent 6ef49d1 commit fcd3722
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 78 deletions.
2 changes: 1 addition & 1 deletion crates/hotshot/src/traits/networking/libp2p_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ impl<T: NodeType> Libp2pNetwork<T> {

// Set the auth message and stake table
config_builder
.stake_table(Some(quorum_membership))
.membership(Some(quorum_membership))
.auth_message(Some(auth_message));

// The replication factor is the minimum of [the default and 2/3 the number of nodes]
Expand Down
4 changes: 2 additions & 2 deletions crates/libp2p-networking/src/network/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,10 @@ impl<T: NodeType, D: DhtPersistentStorage> NetworkNode<T, D> {
// Get the `PeerId` from the `KeyPair`
let peer_id = PeerId::from(keypair.public());

// Generate the transport from the keypair, stake table, and auth message
// Generate the transport from the keypair, membership, and auth message
let transport: BoxedTransport = gen_transport::<T>(
keypair.clone(),
config.stake_table.clone(),
config.membership.clone(),
config.auth_message.clone(),
)
.await?;
Expand Down
4 changes: 2 additions & 2 deletions crates/libp2p-networking/src/network/node/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub struct NetworkNodeConfig<T: NodeType> {
/// The stake table. Used for authenticating other nodes. If not supplied
/// we will not check other nodes against the stake table
#[builder(default)]
pub stake_table: Option<Arc<RwLock<T::Membership>>>,
pub membership: Option<Arc<RwLock<T::Membership>>>,

/// The path to the file to save the DHT to
#[builder(default)]
Expand All @@ -81,7 +81,7 @@ impl<T: NodeType> Clone for NetworkNodeConfig<T> {
to_connect_addrs: self.to_connect_addrs.clone(),
republication_interval: self.republication_interval,
ttl: self.ttl,
stake_table: self.stake_table.as_ref().map(Arc::clone),
membership: self.membership.as_ref().map(Arc::clone),
dht_file_path: self.dht_file_path.clone(),
auth_message: self.auth_message.clone(),
dht_timeout: self.dht_timeout,
Expand Down
81 changes: 22 additions & 59 deletions crates/libp2p-networking/src/network/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use std::{
use anyhow::{ensure, Context, Result as AnyhowResult};
use async_lock::RwLock;
use futures::{future::poll_fn, AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use hotshot_types::traits::{
election::Membership, node_implementation::NodeType, signature_key::SignatureKey,
};
use hotshot_types::traits::{node_implementation::NodeType, signature_key::SignatureKey};
use libp2p::{
core::{
muxing::StreamMuxerExt,
Expand Down Expand Up @@ -45,7 +43,7 @@ pub struct StakeTableAuthentication<T: Transport, Types: NodeType, C: StreamMuxe
pub inner: T,

/// The stake table we check against to authenticate connections
pub stake_table: Arc<Option<Arc<RwLock<Types::Membership>>>>,
pub membership: Arc<Option<Arc<RwLock<Types::Membership>>>>,

/// A pre-signed message that we send to the remote peer for authentication
pub auth_message: Arc<Option<Vec<u8>>>,
Expand All @@ -63,12 +61,12 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
/// and authenticates connections against the stake table.
pub fn new(
inner: T,
stake_table: Option<Arc<RwLock<Types::Membership>>>,
membership: Option<Arc<RwLock<Types::Membership>>>,
auth_message: Option<Vec<u8>>,
) -> Self {
Self {
inner,
stake_table: Arc::from(stake_table),
membership: Arc::from(membership),
auth_message: Arc::from(auth_message),
pd: std::marker::PhantomData,
}
Expand Down Expand Up @@ -108,11 +106,11 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
/// - The signature is invalid
pub async fn verify_peer_authentication<R: AsyncReadExt + Unpin>(
stream: &mut R,
stake_table: Arc<Option<Arc<RwLock<Types::Membership>>>>,
membership: Arc<Option<Arc<RwLock<Types::Membership>>>>,
required_peer_id: &PeerId,
) -> AnyhowResult<()> {
// If we have a stake table, check if the remote peer is in it
if let Some(stake_table) = stake_table.as_ref() {
// If we have a membership, read message and validate
if membership.is_some() {
// Read the length-delimited message from the remote peer
let message = read_length_delimited(stream, MAX_AUTH_MESSAGE_SIZE).await?;

Expand All @@ -121,7 +119,7 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
.with_context(|| "Failed to deserialize auth message")?;

// Verify the signature on the public keys
let public_key = auth_message
auth_message
.validate()
.with_context(|| "Failed to verify authentication message")?;

Expand All @@ -133,11 +131,6 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
if peer_id != *required_peer_id {
return Err(anyhow::anyhow!("Peer ID mismatch"));
}

// Check if the public key is in the stake table
if !stake_table.read().await.has_stake(&public_key, None) {
return Err(anyhow::anyhow!("Peer not in stake table"));
}
}

Ok(())
Expand All @@ -150,7 +143,7 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
fn gen_handshake<F: Future<Output = Result<T::Output, T::Error>> + Send + 'static>(
original_future: F,
outgoing: bool,
stake_table: Arc<Option<Arc<RwLock<Types::Membership>>>>,
membership: Arc<Option<Arc<RwLock<Types::Membership>>>>,
auth_message: Arc<Option<Vec<u8>>>,
) -> UpgradeFuture<T>
where
Expand Down Expand Up @@ -186,7 +179,7 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
// Verify the remote peer's authentication
Self::verify_peer_authentication(
&mut substream,
stake_table,
membership,
stream.as_peer_id(),
)
.await
Expand All @@ -198,7 +191,7 @@ impl<T: Transport, Types: NodeType, C: StreamMuxer + Unpin> StakeTableAuthentica
// If it is incoming, verify the remote peer's authentication first
Self::verify_peer_authentication(
&mut substream,
stake_table,
membership,
stream.as_peer_id(),
)
.await
Expand Down Expand Up @@ -324,11 +317,11 @@ where

// Clone the necessary fields
let auth_message = Arc::clone(&self.auth_message);
let stake_table = Arc::clone(&self.stake_table);
let membership = Arc::clone(&self.membership);

// If the dial was successful, perform the authentication handshake on top
match res {
Ok(dial) => Ok(Self::gen_handshake(dial, true, stake_table, auth_message)),
Ok(dial) => Ok(Self::gen_handshake(dial, true, membership, auth_message)),
Err(err) => Err(err),
}
}
Expand All @@ -352,11 +345,11 @@ where
} => {
// Clone the necessary fields
let auth_message = Arc::clone(&self.auth_message);
let stake_table = Arc::clone(&self.stake_table);
let membership = Arc::clone(&self.membership);

// Generate the handshake upgrade future (inbound)
let auth_upgrade =
Self::gen_handshake(upgrade, false, stake_table, auth_message);
Self::gen_handshake(upgrade, false, membership, auth_message);

// Return the new event
TransportEvent::Incoming {
Expand Down Expand Up @@ -498,7 +491,9 @@ mod test {

use hotshot_example_types::node_types::TestTypes;
use hotshot_types::{
light_client::StateVerKey, signature_key::BLSPubKey, traits::signature_key::SignatureKey,
light_client::StateVerKey,
signature_key::BLSPubKey,
traits::{election::Membership, signature_key::SignatureKey},
PeerConfig,
};
use libp2p::{core::transport::dummy::DummyTransport, quic::Connection};
Expand Down Expand Up @@ -617,13 +612,13 @@ mod test {
stake_table_entry: keypair.0.stake_table_entry(1),
state_ver_key: StateVerKey::default(),
};
let stake_table =
let membership =
<TestTypes as NodeType>::Membership::new(vec![peer_config.clone()], vec![peer_config]);

// Verify the authentication message
let result = MockStakeTableAuth::verify_peer_authentication(
&mut stream,
Arc::new(Some(Arc::new(RwLock::new(stake_table)))),
Arc::new(Some(Arc::new(RwLock::new(membership)))),
&peer_id,
)
.await;
Expand All @@ -634,38 +629,6 @@ mod test {
);
}

#[tokio::test(flavor = "multi_thread")]
async fn key_not_in_stake_table() {
// Create a new identity
let (_, peer_id, auth_message) = new_identity!();

// Create a stream and write the message to it
let mut stream = cursor_from!(auth_message);

// Create an empty stake table
let stake_table = Arc::new(RwLock::new(<TestTypes as NodeType>::Membership::new(
vec![],
vec![],
)));

// Verify the authentication message
let result = MockStakeTableAuth::verify_peer_authentication(
&mut stream,
Arc::new(Some(stake_table)),
&peer_id,
)
.await;

// Make sure it errored for the right reason
assert!(
result
.expect_err("Should have failed authentication but did not")
.to_string()
.contains("Peer not in stake table"),
"Did not fail with the correct error"
);
}

#[tokio::test(flavor = "multi_thread")]
async fn peer_id_mismatch() {
// Create a new identity and authentication message
Expand All @@ -682,15 +645,15 @@ mod test {
stake_table_entry: keypair.0.stake_table_entry(1),
state_ver_key: StateVerKey::default(),
};
let stake_table = Arc::new(RwLock::new(<TestTypes as NodeType>::Membership::new(
let membership = Arc::new(RwLock::new(<TestTypes as NodeType>::Membership::new(
vec![peer_config.clone()],
vec![peer_config],
)));

// Check against the malicious peer ID
let result = MockStakeTableAuth::verify_peer_authentication(
&mut stream,
Arc::new(Some(stake_table)),
Arc::new(Some(membership)),
&malicious_peer_id,
)
.await;
Expand Down
3 changes: 1 addition & 2 deletions crates/task-impls/src/da.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ use async_broadcast::{Receiver, Sender};
use async_lock::RwLock;
use async_trait::async_trait;
use hotshot_task::task::TaskState;
use hotshot_types::simple_vote::HasEpoch;
use hotshot_types::{
consensus::{Consensus, OuterConsensus},
data::{DaProposal2, PackedBundle},
event::{Event, EventType},
message::{Proposal, UpgradeLock},
simple_certificate::DaCertificate2,
simple_vote::{DaData2, DaVote2},
simple_vote::{DaData2, DaVote2, HasEpoch},
traits::{
block_contents::vid_commitment,
election::Membership,
Expand Down
22 changes: 11 additions & 11 deletions crates/task-impls/src/quorum_vote/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,17 @@

use std::{collections::btree_map::Entry, sync::Arc};

use super::QuorumVoteTaskState;
use crate::{
events::HotShotEvent,
helpers::{
broadcast_event, decide_from_proposal, decide_from_proposal_2, fetch_proposal,
LeafChainTraversalOutcome,
},
quorum_vote::Versions,
};
use async_broadcast::{InactiveReceiver, Sender};
use async_lock::RwLock;
use chrono::Utc;
use committable::Committable;
use hotshot_types::simple_vote::HasEpoch;
use hotshot_types::{
consensus::OuterConsensus,
data::{Leaf2, QuorumProposalWrapper, VidDisperseShare},
drb::{compute_drb_result, DrbResult},
event::{Event, EventType},
message::{convert_proposal, Proposal, UpgradeLock},
simple_vote::{QuorumData2, QuorumVote2},
simple_vote::{HasEpoch, QuorumData2, QuorumVote2},
traits::{
block_contents::BlockHeader,
election::Membership,
Expand All @@ -46,6 +36,16 @@ use tracing::instrument;
use utils::anytrace::*;
use vbs::version::StaticVersionType;

use super::QuorumVoteTaskState;
use crate::{
events::HotShotEvent,
helpers::{
broadcast_event, decide_from_proposal, decide_from_proposal_2, fetch_proposal,
LeafChainTraversalOutcome,
},
quorum_vote::Versions,
};

/// Store the DRB result from the computation task to the shared `results` table.
///
/// Returns the result if it exists.
Expand Down
3 changes: 2 additions & 1 deletion crates/testing/tests/tests_6/test_epochs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// You should have received a copy of the MIT License
// along with the HotShot repository. If not, see <https://mit-license.org/>.

use std::{collections::HashMap, time::Duration};

use hotshot_example_types::{
node_types::{
CombinedImpl, EpochUpgradeTestVersions, EpochsTestVersions, Libp2pImpl, MemoryImpl,
Expand All @@ -22,7 +24,6 @@ use hotshot_testing::{
view_sync_task::ViewSyncTaskDescription,
};
use hotshot_types::{data::ViewNumber, traits::node_implementation::ConsensusTime};
use std::{collections::HashMap, time::Duration};

cross_tests!(
TestName: test_success_with_epochs,
Expand Down

0 comments on commit fcd3722

Please sign in to comment.