Remove generic Id param from RequestId (#6032)

* rename RequestId's for better context,

and move them to lighthouse_network crate.

* remove unrequired generic AppReqId from RequestID
This commit is contained in:
João Oliveira
2024-07-09 00:56:14 +01:00
committed by GitHub
parent 48c55ae295
commit a59a61fef9
18 changed files with 175 additions and 170 deletions

View File

@@ -212,7 +212,7 @@ mod tests {
use crate::rpc::rate_limiter::Quota;
use crate::rpc::self_limiter::SelfRateLimiter;
use crate::rpc::{OutboundRequest, Ping, Protocol};
use crate::service::api_types::RequestId;
use crate::service::api_types::{AppRequestId, RequestId, SyncRequestId};
use libp2p::PeerId;
use std::time::Duration;
use types::MainnetEthSpec;
@@ -225,15 +225,17 @@ mod tests {
ping_quota: Quota::n_every(1, 2),
..Default::default()
});
let mut limiter: SelfRateLimiter<RequestId<u64>, MainnetEthSpec> =
let mut limiter: SelfRateLimiter<RequestId, MainnetEthSpec> =
SelfRateLimiter::new(config, log).unwrap();
let peer_id = PeerId::random();
for i in 1..=5 {
for i in 1..=5u32 {
let _ = limiter.allows(
peer_id,
RequestId::Application(i),
OutboundRequest::Ping(Ping { data: i }),
RequestId::Application(AppRequestId::Sync(SyncRequestId::RangeBlockAndBlobs {
id: i,
})),
OutboundRequest::Ping(Ping { data: i as u64 }),
);
}
@@ -246,8 +248,13 @@ mod tests {
// Check that requests in the queue are ordered in the sequence 2, 3, 4, 5.
let mut iter = queue.iter();
for i in 2..=5 {
assert_eq!(iter.next().unwrap().request_id, RequestId::Application(i));
for i in 2..=5u32 {
assert!(matches!(
iter.next().unwrap().request_id,
RequestId::Application(AppRequestId::Sync(SyncRequestId::RangeBlockAndBlobs {
id,
})) if id == i
));
}
assert_eq!(limiter.ready_requests.len(), 0);
@@ -267,7 +274,12 @@ mod tests {
// Check that requests in the queue are ordered in the sequence 3, 4, 5.
let mut iter = queue.iter();
for i in 3..=5 {
assert_eq!(iter.next().unwrap().request_id, RequestId::Application(i));
assert!(matches!(
iter.next().unwrap().request_id,
RequestId::Application(AppRequestId::Sync(SyncRequestId::RangeBlockAndBlobs {
id
})) if id == i
));
}
assert_eq!(limiter.ready_requests.len(), 1);

View File

@@ -19,10 +19,36 @@ use crate::rpc::{
/// Identifier of requests sent by a peer.
pub type PeerRequestId = (ConnectionId, SubstreamId);
/// Identifier of a request.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RequestId<AppReqId> {
Application(AppReqId),
pub type Id = u32;
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
pub struct SingleLookupReqId {
pub lookup_id: Id,
pub req_id: Id,
}
/// Id of rpc requests sent by sync to the network.
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
pub enum SyncRequestId {
/// Request searching for a block given a hash.
SingleBlock { id: SingleLookupReqId },
/// Request searching for a set of blobs given a hash.
SingleBlob { id: SingleLookupReqId },
/// Range request that is composed by both a block range request and a blob range request.
RangeBlockAndBlobs { id: Id },
}
/// Application level requests sent to the network.
#[derive(Debug, Clone, Copy)]
pub enum AppRequestId {
Sync(SyncRequestId),
Router,
}
/// Global identifier of a request.
#[derive(Debug, Clone, Copy)]
pub enum RequestId {
Application(AppRequestId),
Internal,
}
@@ -142,7 +168,7 @@ impl<E: EthSpec> std::convert::From<Response<E>> for RPCCodedResponse<E> {
}
}
impl<AppReqId: std::fmt::Debug> slog::Value for RequestId<AppReqId> {
impl slog::Value for RequestId {
fn serialize(
&self,
record: &slog::Record,

View File

@@ -1,6 +1,6 @@
use crate::discovery::Discovery;
use crate::peer_manager::PeerManager;
use crate::rpc::{ReqId, RPC};
use crate::rpc::RPC;
use crate::types::SnappyTransform;
use libp2p::identify;
@@ -16,9 +16,8 @@ pub type SubscriptionFilter =
pub type Gossipsub = gossipsub::Behaviour<SnappyTransform, SubscriptionFilter>;
#[derive(NetworkBehaviour)]
pub(crate) struct Behaviour<AppReqId, E>
pub(crate) struct Behaviour<E>
where
AppReqId: ReqId,
E: EthSpec,
{
/// Keep track of active and pending connections to enforce hard limits.
@@ -26,7 +25,7 @@ where
/// The peer manager that keeps track of peer's reputation and status.
pub peer_manager: PeerManager<E>,
/// The Eth2 RPC specified in the wire-0 protocol.
pub eth2_rpc: RPC<RequestId<AppReqId>, E>,
pub eth2_rpc: RPC<RequestId, E>,
/// Discv5 Discovery protocol.
pub discovery: Discovery<E>,
/// Keep regular connection to peers and disconnect if absent.

View File

@@ -21,7 +21,7 @@ use crate::types::{
use crate::EnrExt;
use crate::Eth2Enr;
use crate::{error, metrics, Enr, NetworkGlobals, PubsubMessage, TopicHash};
use api_types::{PeerRequestId, Request, RequestId, Response};
use api_types::{AppRequestId, PeerRequestId, Request, RequestId, Response};
use futures::stream::StreamExt;
use gossipsub::{
IdentTopic as Topic, MessageAcceptance, MessageAuthenticity, MessageId, PublishError,
@@ -57,7 +57,7 @@ const MAX_IDENTIFY_ADDRESSES: usize = 10;
/// The types of events than can be obtained from polling the behaviour.
#[derive(Debug)]
pub enum NetworkEvent<AppReqId: ReqId, E: EthSpec> {
pub enum NetworkEvent<E: EthSpec> {
/// We have successfully dialed and connected to a peer.
PeerConnectedOutgoing(PeerId),
/// A peer has successfully dialed and connected to us.
@@ -67,7 +67,7 @@ pub enum NetworkEvent<AppReqId: ReqId, E: EthSpec> {
/// An RPC Request that was sent failed.
RPCFailed {
/// The id of the failed request.
id: AppReqId,
id: AppRequestId,
/// The peer to which this request was sent.
peer_id: PeerId,
/// The error of the failed request.
@@ -85,7 +85,7 @@ pub enum NetworkEvent<AppReqId: ReqId, E: EthSpec> {
/// Peer that sent the response.
peer_id: PeerId,
/// Id of the request to which the peer is responding.
id: AppReqId,
id: AppRequestId,
/// Response the peer sent.
response: Response<E>,
},
@@ -108,8 +108,8 @@ pub enum NetworkEvent<AppReqId: ReqId, E: EthSpec> {
/// Builds the network behaviour that manages the core protocols of eth2.
/// This core behaviour is managed by `Behaviour` which adds peer management to all core
/// behaviours.
pub struct Network<AppReqId: ReqId, E: EthSpec> {
swarm: libp2p::swarm::Swarm<Behaviour<AppReqId, E>>,
pub struct Network<E: EthSpec> {
swarm: libp2p::swarm::Swarm<Behaviour<E>>,
/* Auxiliary Fields */
/// A collections of variables accessible outside the network service.
network_globals: Arc<NetworkGlobals<E>>,
@@ -132,7 +132,7 @@ pub struct Network<AppReqId: ReqId, E: EthSpec> {
}
/// Implements the combined behaviour for the libp2p service.
impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
impl<E: EthSpec> Network<E> {
pub async fn new(
executor: task_executor::TaskExecutor,
mut ctx: ServiceContext<'_>,
@@ -592,7 +592,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
&mut self.swarm.behaviour_mut().gossipsub
}
/// The Eth2 RPC specified in the wire-0 protocol.
pub fn eth2_rpc_mut(&mut self) -> &mut RPC<RequestId<AppReqId>, E> {
pub fn eth2_rpc_mut(&mut self) -> &mut RPC<RequestId, E> {
&mut self.swarm.behaviour_mut().eth2_rpc
}
/// Discv5 Discovery protocol.
@@ -613,7 +613,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
&self.swarm.behaviour().gossipsub
}
/// The Eth2 RPC specified in the wire-0 protocol.
pub fn eth2_rpc(&self) -> &RPC<RequestId<AppReqId>, E> {
pub fn eth2_rpc(&self) -> &RPC<RequestId, E> {
&self.swarm.behaviour().eth2_rpc
}
/// Discv5 Discovery protocol.
@@ -920,9 +920,9 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
pub fn send_request(
&mut self,
peer_id: PeerId,
request_id: AppReqId,
request_id: AppRequestId,
request: Request,
) -> Result<(), (AppReqId, RPCError)> {
) -> Result<(), (AppRequestId, RPCError)> {
// Check if the peer is connected before sending an RPC request
if !self.swarm.is_connected(&peer_id) {
return Err((request_id, RPCError::Disconnected));
@@ -1157,10 +1157,10 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
#[must_use = "return the response"]
fn build_response(
&mut self,
id: RequestId<AppReqId>,
id: RequestId,
peer_id: PeerId,
response: Response<E>,
) -> Option<NetworkEvent<AppReqId, E>> {
) -> Option<NetworkEvent<E>> {
match id {
RequestId::Application(id) => Some(NetworkEvent::ResponseReceived {
peer_id,
@@ -1178,7 +1178,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
id: PeerRequestId,
peer_id: PeerId,
request: Request,
) -> NetworkEvent<AppReqId, E> {
) -> NetworkEvent<E> {
// Increment metrics
match &request {
Request::Status(_) => {
@@ -1244,7 +1244,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
/* Sub-behaviour event handling functions */
/// Handle a gossipsub event.
fn inject_gs_event(&mut self, event: gossipsub::Event) -> Option<NetworkEvent<AppReqId, E>> {
fn inject_gs_event(&mut self, event: gossipsub::Event) -> Option<NetworkEvent<E>> {
match event {
gossipsub::Event::Message {
propagation_source,
@@ -1383,10 +1383,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
}
/// Handle an RPC event.
fn inject_rpc_event(
&mut self,
event: RPCMessage<RequestId<AppReqId>, E>,
) -> Option<NetworkEvent<AppReqId, E>> {
fn inject_rpc_event(&mut self, event: RPCMessage<RequestId, E>) -> Option<NetworkEvent<E>> {
let peer_id = event.peer_id;
// Do not permit Inbound events from peers that are being disconnected, or RPC requests.
@@ -1619,10 +1616,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
}
/// Handle an identify event.
fn inject_identify_event(
&mut self,
event: identify::Event,
) -> Option<NetworkEvent<AppReqId, E>> {
fn inject_identify_event(&mut self, event: identify::Event) -> Option<NetworkEvent<E>> {
match event {
identify::Event::Received { peer_id, mut info } => {
if info.listen_addrs.len() > MAX_IDENTIFY_ADDRESSES {
@@ -1643,7 +1637,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
}
/// Handle a peer manager event.
fn inject_pm_event(&mut self, event: PeerManagerEvent) -> Option<NetworkEvent<AppReqId, E>> {
fn inject_pm_event(&mut self, event: PeerManagerEvent) -> Option<NetworkEvent<E>> {
match event {
PeerManagerEvent::PeerConnectedIncoming(peer_id) => {
Some(NetworkEvent::PeerConnectedIncoming(peer_id))
@@ -1747,7 +1741,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
/// Poll the p2p networking stack.
///
/// This will poll the swarm and do maintenance routines.
pub fn poll_network(&mut self, cx: &mut Context) -> Poll<NetworkEvent<AppReqId, E>> {
pub fn poll_network(&mut self, cx: &mut Context) -> Poll<NetworkEvent<E>> {
while let Poll::Ready(Some(swarm_event)) = self.swarm.poll_next_unpin(cx) {
let maybe_event = match swarm_event {
SwarmEvent::Behaviour(behaviour_event) => match behaviour_event {
@@ -1889,7 +1883,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
Poll::Pending
}
pub async fn next_event(&mut self) -> NetworkEvent<AppReqId, E> {
pub async fn next_event(&mut self) -> NetworkEvent<E> {
futures::future::poll_fn(|cx| self.poll_network(cx)).await
}
}

View File

@@ -13,7 +13,6 @@ use types::{
};
type E = MinimalEthSpec;
type ReqId = usize;
use tempfile::Builder as TempBuilder;
@@ -44,14 +43,14 @@ pub fn fork_context(fork_name: ForkName) -> ForkContext {
}
pub struct Libp2pInstance(
LibP2PService<ReqId, E>,
LibP2PService<E>,
#[allow(dead_code)]
// This field is managed for lifetime purposes may not be used directly, hence the `#[allow(dead_code)]` attribute.
async_channel::Sender<()>,
);
impl std::ops::Deref for Libp2pInstance {
type Target = LibP2PService<ReqId, E>;
type Target = LibP2PService<E>;
fn deref(&self) -> &Self::Target {
&self.0
}
@@ -125,7 +124,7 @@ pub async fn build_libp2p_instance(
}
#[allow(dead_code)]
pub fn get_enr(node: &LibP2PService<ReqId, E>) -> Enr {
pub fn get_enr(node: &LibP2PService<E>) -> Enr {
node.local_enr()
}

View File

@@ -4,6 +4,7 @@ mod common;
use common::Protocol;
use lighthouse_network::rpc::methods::*;
use lighthouse_network::service::api_types::AppRequestId;
use lighthouse_network::{rpc::max_rpc_size, NetworkEvent, ReportSource, Request, Response};
use slog::{debug, warn, Level};
use ssz::Encode;
@@ -99,12 +100,12 @@ fn test_tcp_status_rpc() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, 10, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
peer_id: _,
id: 10,
id: AppRequestId::Router,
response,
} => {
// Should receive the RPC response
@@ -196,7 +197,6 @@ fn test_tcp_blocks_by_range_chunked_rpc() {
// keep count of the number of messages received
let mut messages_received = 0;
let request_id = messages_to_send as usize;
// build the sender future
let sender_future = async {
loop {
@@ -205,7 +205,7 @@ fn test_tcp_blocks_by_range_chunked_rpc() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, request_id, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
@@ -323,7 +323,6 @@ fn test_blobs_by_range_chunked_rpc() {
// keep count of the number of messages received
let mut messages_received = 0;
let request_id = messages_to_send as usize;
// build the sender future
let sender_future = async {
loop {
@@ -332,7 +331,7 @@ fn test_blobs_by_range_chunked_rpc() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, request_id, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
@@ -433,7 +432,6 @@ fn test_tcp_blocks_by_range_over_limit() {
let rpc_response_bellatrix_large =
Response::BlocksByRange(Some(Arc::new(signed_full_block)));
let request_id = messages_to_send as usize;
// build the sender future
let sender_future = async {
loop {
@@ -442,12 +440,12 @@ fn test_tcp_blocks_by_range_over_limit() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, request_id, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
// The request will fail because the sender will refuse to send anything > MAX_RPC_SIZE
NetworkEvent::RPCFailed { id, .. } => {
assert_eq!(id, request_id);
assert!(matches!(id, AppRequestId::Router));
return;
}
_ => {} // Ignore other behaviour events
@@ -528,7 +526,6 @@ fn test_tcp_blocks_by_range_chunked_rpc_terminates_correctly() {
// keep count of the number of messages received
let mut messages_received: u64 = 0;
let request_id = messages_to_send as usize;
// build the sender future
let sender_future = async {
loop {
@@ -537,7 +534,7 @@ fn test_tcp_blocks_by_range_chunked_rpc_terminates_correctly() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, request_id, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
@@ -668,12 +665,12 @@ fn test_tcp_blocks_by_range_single_empty_rpc() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, 10, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
peer_id: _,
id: 10,
id: AppRequestId::Router,
response,
} => match response {
Response::BlocksByRange(Some(_)) => {
@@ -793,12 +790,12 @@ fn test_tcp_blocks_by_root_chunked_rpc() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, 6, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
peer_id: _,
id: 6,
id: AppRequestId::Router,
response,
} => match response {
Response::BlocksByRoot(Some(_)) => {
@@ -926,12 +923,12 @@ fn test_tcp_blocks_by_root_chunked_rpc_terminates_correctly() {
// Send a STATUS message
debug!(log, "Sending RPC");
sender
.send_request(peer_id, 10, rpc_request.clone())
.send_request(peer_id, AppRequestId::Router, rpc_request.clone())
.unwrap();
}
NetworkEvent::ResponseReceived {
peer_id: _,
id: 10,
id: AppRequestId::Router,
response,
} => {
debug!(log, "Sender received a response");