Skip to content

Conversation

@ong-jonas
Copy link
Collaborator

@ong-jonas ong-jonas commented Oct 30, 2023

Network v0.6

  1. Configuration - Message ID is no longer overridden, back to rust-libp2p default (“concatenate the source peer id with a sequence number”).
  2. Change State Machine Replication Topic hash:
    "consensus" -> "hotstuff_rs"
    base64url(public_addr) -> "hotstuff_rs/" + base64url(public_addr)
  3. Each replica now subscribes to individual topics of closest replicas in kademlia k bucket. However, messages under these topics are not processed and just propagated to the rest of the network via gossipsub.
  4. Dropped Transaction message type removed.
  5. Protocol name is now a configurable string.
  6. Added unsubscribe function to unsubscribe from individual topic of replicas when they disconnect.

@ong-jonas ong-jonas marked this pull request as ready for review December 5, 2023 01:22
@ong-jonas ong-jonas changed the base branch from v0.5 to main December 5, 2023 01:30
@ong-jonas ong-jonas requested a review from AlvinHon December 5, 2023 01:30
@ong-jonas ong-jonas changed the base branch from main to v0.5 December 5, 2023 06:14

/// Check the distance between 2 peers. Subscribe to new peer's individual topic
/// if the distance is below [MAX_REPLICA_DISTANCE]
pub(crate) fn is_close_peer(peer_1: &PeerId, peer_2: &PeerId) -> bool {
Copy link
Collaborator

@lyulka lyulka Dec 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this should be in the conversions module.

Any reason why this function should be in the "conversions" module?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will shift this to peer module.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at PeerStartError::SystemConfigError. Its itemdoc says that the variant corresponds to the case where we "Failed to read from system configuration path". However, I don't see anywhere in the code where we read a file at a path.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated it with the correct error variant.

@manngayin612 manngayin612 changed the base branch from v0.5 to main December 11, 2023 06:00
@lyulka lyulka self-requested a review December 11, 2023 08:59
@lyulka lyulka merged commit eb02587 into main Dec 11, 2023
@lyulka lyulka deleted the mailbox/feature branch April 7, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants