Skip to content

Commit

Permalink
p2p: use precalculated topic hash (#2378)
Browse files Browse the repository at this point in the history
## Linked Issues/PRs
None

## Description
One of the main target of `GossipsubTopics`struct is to precalculate the
topic hashes to avoid calculating them on each outgoing message. With
the current implementation and usage of `get_gossipsub_topic()` method,
`Topic` (more accurately `Sha256Topic`) is retrieved and then passed
down to publish method of the gossipsub, where the conversion to
`TopicHash` happens. Meaning that precalculated topic hashes are
actually unused.

This PR rewrites `get_gossipsub_topic()` method to
`get_gossipsub_topic_hash()`, which allows benefiting from the
precalculated topic hashes.

## Checklist
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?

---------

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
  • Loading branch information
yaziciahmet and xgreenx authored Oct 22, 2024
1 parent 293667c commit 48344ca
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 37 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2366](https://github.com/FuelLabs/fuel-core/pull/2366): The `importer_gas_price_for_block` metric is properly collected.
- [2369](https://github.com/FuelLabs/fuel-core/pull/2369): The `transaction_insertion_time_in_thread_pool_milliseconds` metric is properly collected.

### Changed

- [2378](https://github.com/FuelLabs/fuel-core/pull/2378): Use cached hash of the topic instead of calculating it on each publishing gossip message.

### Added
- [2321](https://github.com/FuelLabs/fuel-core/pull/2321): New metrics for the txpool: "The size of transactions in the txpool" (`txpool_tx_size`), "The time spent by a transaction in the txpool in seconds" (`txpool_tx_time_in_txpool_seconds`), The number of transactions in the txpool (`txpool_number_of_transactions`), "The number of transactions pending verification before entering the txpool" (`txpool_number_of_transactions_pending_verification`), "The number of executable transactions in the txpool" (`txpool_number_of_executable_transactions`), "The time it took to select transactions for inclusion in a block in nanoseconds" (`txpool_select_transaction_time_nanoseconds`), The time it took to insert a transaction in the txpool in milliseconds (`txpool_insert_transaction_time_milliseconds`).
- [2362](https://github.com/FuelLabs/fuel-core/pull/2362): Added a new request_response protocol version `/fuel/req_res/0.0.2`. In comparison with `/fuel/req/0.0.1`, which returns an empty response when a request cannot be fulfilled, this version returns more meaningful error codes. Nodes still support the version `0.0.1` of the protocol to guarantee backward compatibility with fuel-core nodes. Empty responses received from nodes using the old protocol `/fuel/req/0.0.1` are automatically converted into an error `ProtocolV1EmptyResponse` with error code 0, which is also the only error code implemented. More specific error codes will be added in the future.
Expand Down
10 changes: 4 additions & 6 deletions crates/services/p2p/src/behavior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ use crate::{
},
config::Config,
discovery,
gossipsub::{
config::build_gossipsub_behaviour,
topics::GossipTopic,
},
gossipsub::config::build_gossipsub_behaviour,
heartbeat,
peer_report,
request_response::messages::{
Expand All @@ -24,6 +21,7 @@ use libp2p::{
MessageAcceptance,
MessageId,
PublishError,
TopicHash,
},
identify,
request_response::{
Expand Down Expand Up @@ -150,10 +148,10 @@ impl FuelBehaviour {

pub fn publish_message(
&mut self,
topic: GossipTopic,
topic_hash: TopicHash,
encoded_data: Vec<u8>,
) -> Result<MessageId, PublishError> {
self.gossipsub.publish(topic, encoded_data)
self.gossipsub.publish(topic_hash, encoded_data)
}

pub fn send_request_msg(
Expand Down
8 changes: 3 additions & 5 deletions crates/services/p2p/src/gossipsub/config.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use super::topics::{
GossipTopic,
NEW_TX_GOSSIP_TOPIC,
};
use super::topics::NEW_TX_GOSSIP_TOPIC;
use crate::{
config::{
Config,
Expand All @@ -18,6 +15,7 @@ use libp2p::gossipsub::{
MetricsConfig,
PeerScoreParams,
PeerScoreThresholds,
Sha256Topic,
Topic,
TopicScoreParams,
};
Expand Down Expand Up @@ -228,7 +226,7 @@ fn initialize_gossipsub(gossipsub: &mut gossipsub::Behaviour, p2p_config: &Confi

// subscribe to gossipsub topics with the network name suffix
for (topic, weight) in topics {
let t: GossipTopic = Topic::new(format!("{}/{}", topic, p2p_config.network_name));
let t: Sha256Topic = Topic::new(format!("{}/{}", topic, p2p_config.network_name));

gossipsub
.set_topic_params(t.clone(), initialize_topic_score_params(weight))
Expand Down
32 changes: 15 additions & 17 deletions crates/services/p2p/src/gossipsub/topics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,23 @@ use super::messages::{
GossipsubBroadcastRequest,
};

pub type GossipTopic = Sha256Topic;
pub const NEW_TX_GOSSIP_TOPIC: &str = "new_tx";

/// Holds used Gossipsub Topics
/// Each field contains TopicHash and GossipTopic itself
/// in order to avoid converting GossipTopic to TopicHash on each received message
/// Each field contains TopicHash of existing topics
/// in order to avoid converting topics to TopicHash on each received message
#[derive(Debug)]
pub struct GossipsubTopics {
new_tx_topic: (TopicHash, GossipTopic),
new_tx_topic: TopicHash,
}

impl GossipsubTopics {
pub fn new(network_name: &str) -> Self {
let new_tx_topic = Topic::new(format!("{NEW_TX_GOSSIP_TOPIC}/{network_name}"));
let new_tx_topic: Sha256Topic =
Topic::new(format!("{NEW_TX_GOSSIP_TOPIC}/{network_name}"));

Self {
new_tx_topic: (new_tx_topic.hash(), new_tx_topic),
new_tx_topic: new_tx_topic.hash(),
}
}

Expand All @@ -34,22 +34,20 @@ impl GossipsubTopics {
&self,
incoming_topic: &TopicHash,
) -> Option<GossipTopicTag> {
let GossipsubTopics { new_tx_topic } = &self;

match incoming_topic {
hash if hash == &new_tx_topic.0 => Some(GossipTopicTag::NewTx),
hash if hash == &self.new_tx_topic => Some(GossipTopicTag::NewTx),
_ => None,
}
}

/// Given a `GossipsubBroadcastRequest` returns a `GossipTopic`
/// Given a `GossipsubBroadcastRequest` returns a `TopicHash`
/// which is broadcast over the network with the serialized inner value of `GossipsubBroadcastRequest`
pub fn get_gossipsub_topic(
pub fn get_gossipsub_topic_hash(
&self,
outgoing_request: &GossipsubBroadcastRequest,
) -> GossipTopic {
) -> TopicHash {
match outgoing_request {
GossipsubBroadcastRequest::NewTx(_) => self.new_tx_topic.1.clone(),
GossipsubBroadcastRequest::NewTx(_) => self.new_tx_topic.clone(),
}
}
}
Expand All @@ -64,25 +62,25 @@ mod tests {
#[test]
fn test_gossipsub_topics() {
let network_name = "fuel_test_network";
let new_tx_topic: GossipTopic =
let new_tx_topic: Sha256Topic =
Topic::new(format!("{NEW_TX_GOSSIP_TOPIC}/{network_name}"));

let gossipsub_topics = GossipsubTopics::new(network_name);

// Test matching Topic Hashes
assert_eq!(gossipsub_topics.new_tx_topic.0, new_tx_topic.hash());
assert_eq!(gossipsub_topics.new_tx_topic, new_tx_topic.hash());

// Test given a TopicHash that `get_gossipsub_tag()` returns matching `GossipTopicTag`
assert_eq!(
gossipsub_topics.get_gossipsub_tag(&new_tx_topic.hash()),
Some(GossipTopicTag::NewTx)
);

// Test given a `GossipsubBroadcastRequest` that `get_gossipsub_topic()` returns matching `Topic`
// Test given a `GossipsubBroadcastRequest` that `get_gossipsub_topic_hash()` returns matching `TopicHash`
let broadcast_req =
GossipsubBroadcastRequest::NewTx(Arc::new(Transaction::default_test_tx()));
assert_eq!(
gossipsub_topics.get_gossipsub_topic(&broadcast_req).hash(),
gossipsub_topics.get_gossipsub_topic_hash(&broadcast_req),
new_tx_topic.hash()
);
}
Expand Down
18 changes: 9 additions & 9 deletions crates/services/p2p/src/p2p_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,17 +375,17 @@ impl FuelP2PService {
&mut self,
message: GossipsubBroadcastRequest,
) -> Result<MessageId, PublishError> {
let topic = self
let topic_hash = self
.network_metadata
.gossipsub_data
.topics
.get_gossipsub_topic(&message);
.get_gossipsub_topic_hash(&message);

match self.network_codec.encode(message) {
Ok(encoded_data) => self
.swarm
.behaviour_mut()
.publish_message(topic, encoded_data),
.publish_message(topic_hash, encoded_data),
Err(e) => Err(PublishError::TransformFailed(e)),
}
}
Expand Down Expand Up @@ -839,10 +839,7 @@ mod tests {
GossipsubBroadcastRequest,
GossipsubMessage,
},
topics::{
GossipTopic,
NEW_TX_GOSSIP_TOPIC,
},
topics::NEW_TX_GOSSIP_TOPIC,
},
p2p_service::FuelP2PEvent,
peer_manager::PeerInfo,
Expand Down Expand Up @@ -881,7 +878,10 @@ mod tests {
StreamExt,
};
use libp2p::{
gossipsub::Topic,
gossipsub::{
Sha256Topic,
Topic,
},
identity::Keypair,
swarm::{
ListenError,
Expand Down Expand Up @@ -1528,7 +1528,7 @@ mod tests {
) {
let mut p2p_config = Config::default_initialized("gossipsub_exchanges_messages");

let selected_topic: GossipTopic = {
let selected_topic: Sha256Topic = {
let topic = match broadcast_request {
GossipsubBroadcastRequest::NewTx(_) => NEW_TX_GOSSIP_TOPIC,
};
Expand Down

0 comments on commit 48344ca

Please sign in to comment.