From d9517cc9667c81908fe565194ba8deddff6745fd Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Fri, 14 Oct 2022 18:51:53 +0200 Subject: [PATCH] node: add more tests for refs relay Signed-off-by: Alexis Sellier --- radicle-node/src/service.rs | 5 +- radicle-node/src/service/message.rs | 10 +++ radicle-node/src/test/peer.rs | 42 +++++++----- radicle-node/src/test/simulator.rs | 20 ++++-- radicle-node/src/test/tests.rs | 102 +++++++++++++++++++++++++++- 5 files changed, 152 insertions(+), 27 deletions(-) diff --git a/radicle-node/src/service.rs b/radicle-node/src/service.rs index 1c3b4ff8..a2f96b46 100644 --- a/radicle-node/src/service.rs +++ b/radicle-node/src/service.rs @@ -643,8 +643,9 @@ where return Ok(false); } // TODO: Check refs to see if we should try to fetch or not. - // FIXME: This code is wrong: we shouldn't be fetching from the connected peer, - // we should fetch from the origin. + // Refs are only supposed to be relayed by peers who are tracking + // the resource. Therefore, it's safe to fetch from the remote + // peer, even though it isn't the announcer. let updated = self.storage.fetch(message.id, git).unwrap(); let is_updated = !updated.is_empty(); diff --git a/radicle-node/src/service/message.rs b/radicle-node/src/service/message.rs index 836c29ad..1545a33b 100644 --- a/radicle-node/src/service/message.rs +++ b/radicle-node/src/service/message.rs @@ -122,6 +122,16 @@ pub struct Subscribe { pub until: Timestamp, } +impl Subscribe { + pub fn all() -> Self { + Self { + filter: Filter::default(), + since: Timestamp::MIN, + until: Timestamp::MAX, + } + } +} + /// Node announcing itself to the network. #[derive(Debug, Clone, PartialEq, Eq)] pub struct NodeAnnouncement { diff --git a/radicle-node/src/test/peer.rs b/radicle-node/src/test/peer.rs index 38ff71a5..47dd863c 100644 --- a/radicle-node/src/test/peer.rs +++ b/radicle-node/src/test/peer.rs @@ -7,8 +7,10 @@ use log::*; use crate::address; use crate::clock::{RefClock, Timestamp}; +use crate::crypto::Signer; use crate::git; use crate::git::Url; +use crate::identity::Id; use crate::node; use crate::prelude::NodeId; use crate::service; @@ -23,12 +25,12 @@ use crate::test::simulator; use crate::{Link, LocalTime}; /// Service instantiation used for testing. -pub type Service = service::Service; +pub type Service = service::Service; #[derive(Debug)] -pub struct Peer { +pub struct Peer { pub name: &'static str, - pub service: Service, + pub service: Service, pub ip: net::IpAddr, pub rng: fastrand::Rng, pub local_time: LocalTime, @@ -37,9 +39,10 @@ pub struct Peer { initialized: bool, } -impl simulator::Peer for Peer +impl simulator::Peer for Peer where S: WriteStorage + 'static, + G: Signer + 'static, { fn init(&mut self) { self.initialize() @@ -50,21 +53,21 @@ where } } -impl Deref for Peer { - type Target = Service; +impl Deref for Peer { + type Target = Service; fn deref(&self) -> &Self::Target { &self.service } } -impl DerefMut for Peer { +impl DerefMut for Peer { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.service } } -impl Peer +impl Peer where S: WriteStorage + 'static, { @@ -75,6 +78,8 @@ where ..git::Url::default() }; + let mut rng = fastrand::Rng::new(); + let signer = MockSigner::new(&mut rng); Self::config( name, @@ -84,20 +89,27 @@ where }, ip, storage, - fastrand::Rng::new(), + signer, + rng, ) } +} +impl Peer +where + S: WriteStorage + 'static, + G: Signer + 'static, +{ pub fn config( name: &'static str, config: Config, ip: impl Into, storage: S, - mut rng: fastrand::Rng, + signer: G, + rng: fastrand::Rng, ) -> Self { let local_time = LocalTime::now(); let clock = RefClock::from(local_time); - let signer = MockSigner::new(&mut rng); let routing = routing::Table::memory().unwrap(); let addrs = address::Book::memory().unwrap(); let service = Service::new(config, clock, routing, storage, addrs, signer, rng.clone()); @@ -170,9 +182,7 @@ where ) } - pub fn refs_announcement(&self) -> Message { - let inv = self.inventory().unwrap(); - let id = inv[self.rng.usize(..inv.len())]; + pub fn refs_announcement(&self, id: Id) -> Message { let refs = BTreeMap::new().into(); let ann = AnnouncementMessage::from(RefsAnnouncement { id, @@ -185,7 +195,7 @@ where } pub fn connect_from(&mut self, peer: &Self) { - let remote = simulator::Peer::::addr(peer); + let remote = simulator::Peer::::addr(peer); let local = net::SocketAddr::new(self.ip, self.rng.u16(..)); let git = format!("file:///{}.git", remote.ip()); let git = Url::from_bytes(git.as_bytes()).unwrap(); @@ -213,7 +223,7 @@ where } pub fn connect_to(&mut self, peer: &Self) { - let remote = simulator::Peer::::addr(peer); + let remote = simulator::Peer::::addr(peer); self.initialize(); self.service.attempted(&remote); diff --git a/radicle-node/src/test/simulator.rs b/radicle-node/src/test/simulator.rs index 3627397e..15897a00 100644 --- a/radicle-node/src/test/simulator.rs +++ b/radicle-node/src/test/simulator.rs @@ -14,6 +14,7 @@ use log::*; use nakamoto_net as nakamoto; use nakamoto_net::{Link, LocalDuration, LocalTime}; +use crate::crypto::Signer; use crate::service::reactor::Io; use crate::service::{DisconnectReason, Envelope, Event}; use crate::storage::WriteStorage; @@ -29,7 +30,9 @@ pub const MAX_EVENTS: usize = 2048; type NodeId = std::net::IpAddr; /// A simulated peer. Service instances have to be wrapped in this type to be simulated. -pub trait Peer: Deref> + DerefMut> + 'static { +pub trait Peer: + Deref> + DerefMut> + 'static +{ /// Initialize the peer. This should at minimum initialize the service with the /// current time. fn init(&mut self); @@ -166,7 +169,7 @@ impl Default for Options { } /// A peer-to-peer node simulation. -pub struct Simulation { +pub struct Simulation { /// Inbox of inputs to be delivered by the simulation. inbox: Inbox, /// Events emitted during simulation. @@ -191,9 +194,11 @@ pub struct Simulation { rng: fastrand::Rng, /// Storage type. storage: PhantomData, + /// Signer type. + signer: PhantomData, } -impl Simulation { +impl Simulation { /// Create a new simulation. pub fn new(time: LocalTime, rng: fastrand::Rng, opts: Options) -> Self { Self { @@ -211,6 +216,7 @@ impl Simulation { time, rng, storage: PhantomData, + signer: PhantomData, } } @@ -267,7 +273,7 @@ impl Simulation { /// Initialize peers. pub fn initialize<'a, P>(self, peers: impl IntoIterator) -> Self where - P: Peer, + P: Peer, { for peer in peers.into_iter() { peer.init(); @@ -281,7 +287,7 @@ impl Simulation { peers: impl IntoIterator, pred: impl Fn(&Self) -> bool, ) where - P: Peer, + P: Peer, { let mut nodes: BTreeMap<_, _> = peers.into_iter().map(|p| (p.addr().ip(), p)).collect(); @@ -295,12 +301,12 @@ impl Simulation { /// Process one scheduled input from the inbox, using the provided peers. /// This function should be called until it returns `false`, or some desired state is reached. /// Returns `true` if there are more messages to process. - pub fn step<'a, P: Peer>(&mut self, peers: impl IntoIterator) -> bool { + pub fn step<'a, P: Peer>(&mut self, peers: impl IntoIterator) -> bool { let mut nodes: BTreeMap<_, _> = peers.into_iter().map(|p| (p.addr().ip(), p)).collect(); self.step_(&mut nodes) } - fn step_>(&mut self, nodes: &mut BTreeMap) -> bool { + fn step_>(&mut self, nodes: &mut BTreeMap) -> bool { if !self.opts.latency.is_empty() { // Configure latencies. for (i, from) in nodes.keys().enumerate() { diff --git a/radicle-node/src/test/tests.rs b/radicle-node/src/test/tests.rs index 89dac792..503242a8 100644 --- a/radicle-node/src/test/tests.rs +++ b/radicle-node/src/test/tests.rs @@ -15,6 +15,7 @@ use crate::service::ServiceState as _; use crate::service::*; use crate::storage::git::Storage; use crate::storage::ReadStorage; +use crate::test::arbitrary; use crate::test::assert_matches; use crate::test::fixtures; #[allow(unused)] @@ -77,14 +78,21 @@ fn test_inbound_connection() { #[test] fn test_persistent_peer_connect() { - let rng = fastrand::Rng::new(); + let mut rng = fastrand::Rng::new(); let bob = Peer::new("bob", [8, 8, 8, 8], MockStorage::empty()); let eve = Peer::new("eve", [9, 9, 9, 9], MockStorage::empty()); let config = Config { connect: vec![bob.address(), eve.address()], ..Config::default() }; - let mut alice = Peer::config("alice", config, [7, 7, 7, 7], MockStorage::empty(), rng); + let mut alice = Peer::config( + "alice", + config, + [7, 7, 7, 7], + MockStorage::empty(), + MockSigner::new(&mut rng), + rng, + ); alice.initialize(); @@ -158,6 +166,7 @@ fn test_tracking() { }, [7, 7, 7, 7], MockStorage::empty(), + MockSigner::default(), fastrand::Rng::new(), ); let proj_id: identity::Id = test::arbitrary::gen(1); @@ -334,6 +343,94 @@ fn test_announcement_relay() { ); } +#[test] +fn test_refs_announcement_relay() { + let tmp = tempfile::tempdir().unwrap(); + let mut alice = Peer::new( + "alice", + [7, 7, 7, 7], + Storage::open(tmp.path().join("alice")).unwrap(), + ); + let eve = Peer::new( + "eve", + [8, 8, 8, 8], + Storage::open(tmp.path().join("eve")).unwrap(), + ); + + let bob = { + let mut rng = fastrand::Rng::new(); + let signer = MockSigner::new(&mut rng); + let storage = fixtures::storage(tmp.path().join("bob"), &signer).unwrap(); + + Peer::config( + "bob", + Config { + git_url: git::Url { + scheme: git::url::Scheme::File, + path: storage.path().to_string_lossy().to_string().into(), + + ..git::Url::default() + }, + ..Config::default() + }, + [9, 9, 9, 9], + storage, + signer, + rng, + ) + }; + let bob_inv = bob.inventory().unwrap(); + + alice.track(bob_inv[0]); + alice.track(bob_inv[1]); + alice.track(bob_inv[2]); + alice.connect_to(&bob); + alice.connect_to(&eve); + alice.receive(&eve.addr(), Message::Subscribe(Subscribe::all())); + + alice.receive(&bob.addr(), bob.refs_announcement(bob_inv[0])); + assert_matches!( + alice.messages(&eve.addr()).next(), + Some(Message::Announcement(_)), + "A refs announcement from Bob is relayed to Eve" + ); + + alice.receive(&bob.addr(), bob.refs_announcement(bob_inv[0])); + assert!( + alice.messages(&eve.addr()).next().is_none(), + "The same ref announement is not relayed" + ); + + alice.receive(&bob.addr(), bob.refs_announcement(bob_inv[1])); + assert_matches!( + alice.messages(&eve.addr()).next(), + Some(Message::Announcement(_)), + "But a different one is" + ); + + alice.receive(&bob.addr(), bob.refs_announcement(bob_inv[2])); + assert_matches!( + alice.messages(&eve.addr()).next(), + Some(Message::Announcement(_)), + "And a third one is as well" + ); +} + +#[test] +fn test_refs_gossip_no_subscribe() { + let mut alice = Peer::new("alice", [7, 7, 7, 7], MockStorage::empty()); + let bob = Peer::new("bob", [8, 8, 8, 8], MockStorage::empty()); + let eve = Peer::new("eve", [9, 9, 9, 9], MockStorage::empty()); + let id = arbitrary::gen(1); + + alice.track(id); + alice.connect_to(&bob); + alice.connect_to(&eve); + alice.receive(&bob.addr(), bob.refs_announcement(id)); + + assert!(alice.messages(&eve.addr()).next().is_none()); +} + #[test] fn test_inventory_relay() { // Topology is eve <-> alice <-> bob @@ -442,6 +539,7 @@ fn test_persistent_peer_reconnect() { }, [7, 7, 7, 7], MockStorage::empty(), + MockSigner::default(), fastrand::Rng::new(), );