Skip to content

Commit

Permalink
WIP: There is no way to test topic_weight since libp2p-gossipsub has …
Browse files Browse the repository at this point in the history
…no getter function to obtain `TopicScoreParams`
  • Loading branch information
ackintosh committed Jul 22, 2023
1 parent 5623e45 commit 152d1f0
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 51 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

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

10 changes: 10 additions & 0 deletions beacon_node/lighthouse_network/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,16 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
}
}

/// Returns a scoring parameters for a topic if existent.
pub fn get_topic_params(&self, _topic: GossipTopic) -> Option<TopicScoreParams> {
// See https://github.com/libp2p/rust-libp2p/pull/4231
// self.swarm
// .behaviour()
// .gossipsub
// .get_topic_params(&topic.into())
todo!()
}

/// Subscribes to a gossipsub topic.
///
/// Returns `true` if the subscription was successful and `false` otherwise.
Expand Down
1 change: 0 additions & 1 deletion beacon_node/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ matches = "0.1.8"
exit-future = "0.2.0"
slog-term = "2.6.0"
slog-async = "2.5.0"
tempfile = "3.1.0"

[dependencies]
beacon_chain = { path = "../beacon_chain" }
Expand Down
8 changes: 8 additions & 0 deletions beacon_node/network/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,14 @@ impl<T: BeaconChainTypes> NetworkService<T> {

core_topics.is_subset(&subscribed_topics)
}

#[cfg(test)]
fn get_topic_params(
&self,
topic: GossipTopic,
) -> Option<lighthouse_network::libp2p::gossipsub::TopicScoreParams> {
self.libp2p.get_topic_params(topic)
}
}

/// Returns a `Sleep` that triggers after the next change in the beacon chain fork version.
Expand Down
82 changes: 33 additions & 49 deletions beacon_node/network/src/service/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(not(debug_assertions))]
// #[cfg(not(debug_assertions))]
#[cfg(test)]
mod tests {
use crate::persisted_dht::load_dht;
Expand All @@ -11,13 +11,9 @@ mod tests {
use lighthouse_network::types::{GossipEncoding, GossipKind};
use lighthouse_network::{Enr, GossipTopic};
use slog::{o, Drain, Level, Logger};
use sloggers::file::FileLoggerBuilder;
use sloggers::types::Severity;
use sloggers::{null::NullLoggerBuilder, Build};
use std::io::Read;
use std::str::FromStr;
use std::sync::Arc;
use tempfile::NamedTempFile;
use tokio::{runtime::Runtime, sync::mpsc};
use types::{Epoch, EthSpec, ForkName, MinimalEthSpec, SubnetId};

Expand Down Expand Up @@ -112,7 +108,6 @@ mod tests {
#[test]
fn test_removing_topic_weight_on_old_topics() {
let runtime = Arc::new(Runtime::new().unwrap());
let mut logfile = NamedTempFile::new().unwrap();

// Capella spec
let mut spec = MinimalEthSpec::default_spec();
Expand All @@ -128,23 +123,17 @@ mod tests {
.mock_execution_layer()
.build()
.chain;

let old_fork_digest = beacon_chain.enr_fork_id().fork_digest;
let (next_fork_name, _) = beacon_chain.duration_to_next_fork().expect("next fork");
assert_eq!(next_fork_name, ForkName::Capella);

// Build network service.
let (mut network_service, network_globals, _network_senders) = runtime.block_on(async {
let logger = FileLoggerBuilder::new(logfile.path().clone())
.level(Severity::Debug)
.build()
.expect("should build logger");
let (_, exit) = exit_future::signal();
let (shutdown_tx, _) = futures::channel::mpsc::channel(1);
let executor = task_executor::TaskExecutor::new(
Arc::downgrade(&runtime),
exit,
logger.clone(),
get_logger(false),
shutdown_tx,
);

Expand Down Expand Up @@ -174,6 +163,17 @@ mod tests {
// Topics the service should be subscribed to during the current epoch (before the fork) are:
// - /eth2/(old_fork_digest)/beacon_attestation_1/ssz_snappy
// - /eth2/(old_fork_digest)/beacon_attestation_2/ssz_snappy
let old_fork_digest = beacon_chain.enr_fork_id().fork_digest;
let old_topic1 = GossipTopic::new(
GossipKind::Attestation(SubnetId::new(1)),
GossipEncoding::SSZSnappy,
old_fork_digest,
);
let old_topic2 = GossipTopic::new(
GossipKind::Attestation(SubnetId::new(2)),
GossipEncoding::SSZSnappy,
old_fork_digest,
);
runtime.block_on(async {
while network_globals.gossipsub_subscriptions.read().len() < 2 {
if let Some(msg) = network_service.attestation_service.next().await {
Expand All @@ -183,16 +183,8 @@ mod tests {
});
let subscriptions = network_globals.gossipsub_subscriptions.read().clone();
assert_eq!(2, subscriptions.len());
assert!(subscriptions.contains(&GossipTopic::new(
GossipKind::Attestation(SubnetId::new(1)),
GossipEncoding::SSZSnappy,
old_fork_digest
)));
assert!(subscriptions.contains(&GossipTopic::new(
GossipKind::Attestation(SubnetId::new(2)),
GossipEncoding::SSZSnappy,
old_fork_digest
)));
assert!(subscriptions.contains(&old_topic1));
assert!(subscriptions.contains(&old_topic2));

// Advance slot to the next fork
for _ in 0..MinimalEthSpec::slots_per_epoch() {
Expand All @@ -204,31 +196,23 @@ mod tests {
network_service.update_next_fork();
});

// Ideally we should check TopicScoreParams::topic_weight on the old topics to be zero,
// however there's no way to get TopicScoreParams since libp2p doesn't provide a getter to
// do so.
// As a workaround, checking logs.
let mut buf = vec![];
logfile.read_to_end(&mut buf).unwrap();
let logs = String::from_utf8(buf).unwrap();

// Ensure that the fork has happened.
assert!(logs.contains("Transitioned to new fork, new_fork: Capella"));

// Test that the topic weight has been removed.
assert!(logs.contains(
format!(
"Removed topic weight, topic: /eth2/{}/beacon_attestation_1/ssz_snappy",
hex::encode(old_fork_digest)
)
.as_str()
));
assert!(logs.contains(
format!(
"Removed topic weight, topic: /eth2/{}/beacon_attestation_2/ssz_snappy",
hex::encode(old_fork_digest)
)
.as_str()
));
// Check that topic_weight on the old topics has been zeroed.
// WIP:
// There is no way to test topic_weight since libp2p-gossipsub has no getter function to
// obtain `TopicScoreParams`. Now I'm working on the PR to add a getter to do that.
//
// feat(gossipsub): Add getter function to obtain TopicScoreParams #4231
// https://github.com/libp2p/rust-libp2p/pull/4231
//
// Once the PR has been merged, we can do test topic_weight like below.
// let old_topic_params1 = network_service
// .get_topic_params(old_topic1)
// .expect("topic score params");
// assert_eq!(0.0, old_topic_params1.topic_weight);
//
// let old_topic_params2 = network_service
// .get_topic_params(old_topic2)
// .expect("topic score params");
// assert_eq!(0.0, old_topic_params2.topic_weight);
}
}

0 comments on commit 152d1f0

Please sign in to comment.