Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Shift networking configuration #4426

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
b97fe50
Adding networking configurations into ChainSpec
armaganyildirak Jun 21, 2023
18c02d2
Finishing gossip_max_size
armaganyildirak Jun 21, 2023
3c770a7
Finishing max_request_blocks?
armaganyildirak Jun 21, 2023
e01ab7b
Finishing max_chunk_size
armaganyildirak Jun 21, 2023
e8f86cd
Finishing ttfb_timeout
armaganyildirak Jun 21, 2023
2cccf6a
Finishing resp_timeout
armaganyildirak Jun 21, 2023
72f92af
Finishing message_domain_valid_snappy
armaganyildirak Jun 21, 2023
489558c
Finishing maximum_gossip_clock_disparity?
armaganyildirak Jun 21, 2023
5e28c2e
Finishing maximum_gossip_clock_disparity
armaganyildirak Jun 21, 2023
563ce17
Finishing for the PR
armaganyildirak Jun 22, 2023
c0ac9dc
Fixing conflicts
armaganyildirak Jun 22, 2023
47d1ae7
use chain instead of calling default_spec() function
armaganyildirak Jun 22, 2023
0724ae0
use chain instead of default_spec()
armaganyildirak Jun 22, 2023
a0f3b39
Adding gossip_max_size function ChainSpec object
armaganyildirak Jun 23, 2023
816ebec
Adding ChainSpec object to other parts
armaganyildirak Jun 24, 2023
b7e009c
Adding ChainSpec object to other parts
armaganyildirak Jun 27, 2023
d3d9b8f
Add the networking constants to Config object
armaganyildirak Jun 27, 2023
7090979
Add ChainSpec object to max_rpc_size function
armaganyildirak Jun 28, 2023
eec5813
remove the needless borrows
armaganyildirak Jun 28, 2023
6ca24e9
Fixing some test errors
armaganyildirak Jun 28, 2023
8f3b9a4
Fix the other tests
armaganyildirak Jun 28, 2023
e003f20
Resolve the some reviews
armaganyildirak Jun 29, 2023
49074a3
fmt
armaganyildirak Jun 29, 2023
ba59277
change self instead of &self for removing clone and unnecessary consu…
armaganyildirak Jun 29, 2023
7ce9005
Changes after review
armaganyildirak Jul 10, 2023
323ddda
Merge branch 'sigp:stable' into shift-networking-configuration
armaganyildirak Jul 10, 2023
214a5ff
Merge branch 'unstable' into shift-networking-configuration
divagant-martian Jul 10, 2023
d478314
Merge pull request #1 from divagant-martian/shift-networking-configur…
armaganyildirak Jul 10, 2023
2c403d9
patch merge
divagant-martian Jul 10, 2023
819ff45
fmt
divagant-martian Jul 10, 2023
3a0c57b
Merge pull request #2 from divagant-martian/shift-networking-configur…
armaganyildirak Jul 10, 2023
965ca75
fixing test errors
armaganyildirak Jul 10, 2023
d4ea7ad
Add sanity check to spawn_reprocess_scheduler function
armaganyildirak Jul 11, 2023
24ef86d
fmt
armaganyildirak Jul 11, 2023
d17b3c2
Merge branch 'unstable' into shift-networking-configuration
armaganyildirak Jul 13, 2023
75fcfbf
Remove unnecessary comment
armaganyildirak Jul 17, 2023
42cf723
fix merge conflicts
armaganyildirak Jul 17, 2023
0e67472
fixes after review
armaganyildirak Jul 18, 2023
821dab2
Set ATTESTATION_SUBNET_EXTRA_BITS value to 0
armaganyildirak Jul 18, 2023
9502596
clippy
armaganyildirak Jul 18, 2023
3608b1a
Add serde default impls to `Config`
paulhauner Jul 27, 2023
82fc995
Changes after review
armaganyildirak Jul 27, 2023
7c61007
Diva's patch. Seems to add subnet prefix back also
AgeManning Aug 1, 2023
165a1c2
change after review
armaganyildirak Aug 1, 2023
a5368b0
change after review
armaganyildirak Aug 1, 2023
e9e1ff9
fix the confilicts
armaganyildirak Aug 2, 2023
7fa9a17
fmt
armaganyildirak Aug 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .vscode/settings.json

This file was deleted.

8 changes: 3 additions & 5 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use state_processing::{
signed_aggregate_selection_proof_signature_set, signed_aggregate_signature_set,
},
};
use std::{borrow::Cow, time::Duration};
use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Expand Down Expand Up @@ -1034,10 +1034,8 @@ pub fn verify_propagation_slot_range<S: SlotClock, E: EthSpec>(
spec: &ChainSpec,
) -> Result<(), Error> {
let attestation_slot = attestation.data.slot;
let maximum_gossip_clock_disparity =
Duration::from_millis(spec.maximum_gossip_clock_disparity_millis);
let latest_permissible_slot = slot_clock
.now_with_future_tolerance(maximum_gossip_clock_disparity)
.now_with_future_tolerance(spec.clone().maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?;
if attestation_slot > latest_permissible_slot {
return Err(Error::FutureSlot {
Expand All @@ -1048,7 +1046,7 @@ pub fn verify_propagation_slot_range<S: SlotClock, E: EthSpec>(

// Taking advantage of saturating subtraction on `Slot`.
let earliest_permissible_slot = slot_clock
.now_with_past_tolerance(maximum_gossip_clock_disparity)
.now_with_past_tolerance(spec.clone().maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?
- E::slots_per_epoch();
if attestation_slot < earliest_permissible_slot {
Expand Down
16 changes: 8 additions & 8 deletions beacon_node/beacon_chain/src/sync_committee_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,17 @@ use state_processing::signature_sets::{
sync_committee_contribution_signature_set_from_pubkeys,
sync_committee_message_set_from_pubkeys,
};
use types::ChainSpec;
use std::collections::HashMap;
use std::{borrow::Cow, time::Duration};
use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT;
use types::slot_data::SlotData;
use types::sync_committee::Error as SyncCommitteeError;
use types::{
sync_committee_contribution::Error as ContributionError, AggregateSignature, BeaconStateError,
EthSpec, Hash256, MainnetEthSpec, SignedContributionAndProof, Slot, SyncCommitteeContribution,
EthSpec, Hash256, SignedContributionAndProof, Slot, SyncCommitteeContribution,
SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId,
};

Expand Down Expand Up @@ -285,7 +286,7 @@ impl<T: BeaconChainTypes> VerifiedSyncContribution<T> {
let subcommittee_index = contribution.subcommittee_index as usize;

// Ensure sync committee contribution is within the MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance.
verify_propagation_slot_range(&chain.slot_clock, contribution)?;
verify_propagation_slot_range(&chain.slot_clock, contribution, &chain.spec)?;

// Validate subcommittee index.
if contribution.subcommittee_index >= SYNC_COMMITTEE_SUBNET_COUNT {
Expand Down Expand Up @@ -440,7 +441,7 @@ impl VerifiedSyncCommitteeMessage {
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
// We do not queue future sync committee messages for later processing.
verify_propagation_slot_range(&chain.slot_clock, &sync_message)?;
verify_propagation_slot_range(&chain.slot_clock, &sync_message, &chain.spec)?;

// Ensure the `subnet_id` is valid for the given validator.
let pubkey = chain
Expand Down Expand Up @@ -556,12 +557,11 @@ impl VerifiedSyncCommitteeMessage {
pub fn verify_propagation_slot_range<S: SlotClock, U: SlotData>(
slot_clock: &S,
sync_contribution: &U,
spec: &ChainSpec,
) -> Result<(), Error> {
let message_slot = sync_contribution.get_slot();
let maximum_gossip_clock_disparity =
Duration::from_millis(MainnetEthSpec::default_spec().maximum_gossip_clock_disparity_millis);
let latest_permissible_slot = slot_clock
.now_with_future_tolerance(maximum_gossip_clock_disparity)
.now_with_future_tolerance(spec.clone().maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?;
if message_slot > latest_permissible_slot {
return Err(Error::FutureSlot {
Expand All @@ -571,7 +571,7 @@ pub fn verify_propagation_slot_range<S: SlotClock, U: SlotData>(
}

let earliest_permissible_slot = slot_clock
.now_with_past_tolerance(maximum_gossip_clock_disparity)
.now_with_past_tolerance(spec.clone().maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?;

if message_slot < earliest_permissible_slot {
Expand Down
4 changes: 2 additions & 2 deletions beacon_node/lighthouse_network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,13 +444,13 @@ pub fn gossipsub_config<TSpec: EthSpec>(
}
}
}

let cloned_spec = spec.clone();
Copy link
Collaborator

Choose a reason for hiding this comment

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

you are being told by the compiler to do this because a couple lines below there is a move. However, cloning the entire spec is not necessary. Instead of creating a cloned_spec, create a variable with the value of cloned_spec.message_domain_valid_snappy here and use that variable below

let is_merge_enabled = fork_context.fork_exists(ForkName::Merge);
let gossip_message_id = move |message: &GossipsubMessage| {
MessageId::from(
&Sha256::digest(
prefix(
TSpec::default_spec().message_domain_valid_snappy,
cloned_spec.message_domain_valid_snappy,
message,
fork_context.clone(),
)
Expand Down
6 changes: 3 additions & 3 deletions beacon_node/lighthouse_network/src/rpc/codec/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ mod tests {

let mut snappy_outbound_codec = SSZSnappyOutboundCodec::<Spec>::new(
snappy_protocol_id,
max_rpc_size(&fork_context, &chain_spec),
max_rpc_size(&fork_context, chain_spec.max_chunk_size as usize),
fork_context,
);

Expand Down Expand Up @@ -259,7 +259,7 @@ mod tests {

let mut snappy_outbound_codec = SSZSnappyOutboundCodec::<Spec>::new(
snappy_protocol_id,
max_rpc_size(&fork_context, &chain_spec),
max_rpc_size(&fork_context, chain_spec.max_chunk_size as usize),
fork_context,
);

Expand Down Expand Up @@ -288,7 +288,7 @@ mod tests {

let chain_spec = Spec::default_spec();

let max_rpc_size = max_rpc_size(&fork_context, &chain_spec);
let max_rpc_size = max_rpc_size(&fork_context, chain_spec.max_chunk_size as usize);
let limit = protocol_id.rpc_response_limits::<Spec>(&fork_context);
let mut max = encode_len(limit.max + 1);
let mut codec = SSZSnappyOutboundCodec::<Spec>::new(
Expand Down
10 changes: 5 additions & 5 deletions beacon_node/lighthouse_network/src/rpc/codec/ssz_snappy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ mod tests {
block.body.execution_payload.execution_payload.transactions = txs;

let block = BeaconBlock::Merge(block);
assert!(block.ssz_bytes_len() <= max_rpc_size(fork_context, spec));
assert!(block.ssz_bytes_len() <= max_rpc_size(fork_context, spec.max_chunk_size as usize));
SignedBeaconBlock::from_block(block, Signature::empty())
}

Expand All @@ -683,7 +683,7 @@ mod tests {
block.body.execution_payload.execution_payload.transactions = txs;

let block = BeaconBlock::Merge(block);
assert!(block.ssz_bytes_len() > max_rpc_size(fork_context, spec));
assert!(block.ssz_bytes_len() > max_rpc_size(fork_context, spec.max_chunk_size as usize));
SignedBeaconBlock::from_block(block, Signature::empty())
}

Expand Down Expand Up @@ -741,7 +741,7 @@ mod tests {
) -> Result<BytesMut, RPCError> {
let snappy_protocol_id = ProtocolId::new(protocol, Encoding::SSZSnappy);
let fork_context = Arc::new(fork_context(fork_name));
let max_packet_size = max_rpc_size(&fork_context, spec);
let max_packet_size = max_rpc_size(&fork_context, spec.max_chunk_size as usize);

let mut buf = BytesMut::new();
let mut snappy_inbound_codec =
Expand Down Expand Up @@ -788,7 +788,7 @@ mod tests {
) -> Result<Option<RPCResponse<Spec>>, RPCError> {
let snappy_protocol_id = ProtocolId::new(protocol, Encoding::SSZSnappy);
let fork_context = Arc::new(fork_context(fork_name));
let max_packet_size = max_rpc_size(&fork_context, spec);
let max_packet_size = max_rpc_size(&fork_context, spec.max_chunk_size as usize);
let mut snappy_outbound_codec =
SSZSnappyOutboundCodec::<Spec>::new(snappy_protocol_id, max_packet_size, fork_context);
// decode message just as snappy message
Expand All @@ -813,7 +813,7 @@ mod tests {
spec: &ChainSpec,
) {
let fork_context = Arc::new(fork_context(fork_name));
let max_packet_size = max_rpc_size(&fork_context, spec);
let max_packet_size = max_rpc_size(&fork_context, spec.max_chunk_size as usize);
let protocol = ProtocolId::new(req.versioned_protocol(), Encoding::SSZSnappy);
// Encode a request we send
let mut buf = BytesMut::new();
Expand Down
16 changes: 8 additions & 8 deletions beacon_node/lighthouse_network/src/rpc/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ where
/// Logger for handling RPC streams
log: slog::Logger,

/// ChainSpec for networking constants
chain_spec: ChainSpec,
/// resp_timeout for networking constants
pawanjay176 marked this conversation as resolved.
Show resolved Hide resolved
resp_timeout: Duration,
}

enum HandlerState {
Expand Down Expand Up @@ -232,7 +232,7 @@ where
fork_context,
waker: None,
log: log.clone(),
chain_spec: spec.clone(),
resp_timeout: spec.clone().resp_timeout(),
}
}

Expand Down Expand Up @@ -353,7 +353,7 @@ where
// new outbound request. Store the stream and tag the output.
let delay_key = self.outbound_substreams_delay.insert(
self.current_outbound_substream_id,
Duration::from_secs(self.chain_spec.resp_timeout),
self.resp_timeout,
);
let awaiting_stream = OutboundSubstreamState::RequestPendingResponse {
substream: Box::new(out),
Expand Down Expand Up @@ -404,7 +404,7 @@ where
// Store the stream and tag the output.
let delay_key = self.inbound_substreams_delay.insert(
self.current_inbound_substream_id,
Duration::from_secs(self.chain_spec.resp_timeout),
self.resp_timeout,
);
let awaiting_stream = InboundState::Idle(substream);
self.inbound_substreams.insert(
Expand Down Expand Up @@ -717,7 +717,7 @@ where
if let Some(ref delay_key) = info.delay_key {
self.inbound_substreams_delay.reset(
delay_key,
Duration::from_secs(self.chain_spec.resp_timeout),
self.resp_timeout,
);
}

Expand Down Expand Up @@ -853,7 +853,7 @@ where
substream_entry.remaining_chunks = Some(remaining_chunks);
self.outbound_substreams_delay.reset(
delay_key,
Duration::from_secs(self.chain_spec.resp_timeout),
self.resp_timeout,
);
}
} else {
Expand Down Expand Up @@ -973,7 +973,7 @@ where
OutboundRequestContainer {
req: req.clone(),
fork_context: self.fork_context.clone(),
max_rpc_size: max_rpc_size(&self.fork_context, &self.chain_spec),
max_rpc_size: max_rpc_size(&self.fork_context, self.listen_protocol.upgrade().max_rpc_size),
},
(),
)
Expand Down
3 changes: 2 additions & 1 deletion beacon_node/lighthouse_network/src/rpc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,10 @@ where
SubstreamProtocol::new(
RPCProtocol {
fork_context: self.fork_context.clone(),
max_rpc_size: max_rpc_size(&self.fork_context, &self.chain_spec),
max_rpc_size: max_rpc_size(&self.fork_context, self.chain_spec.max_chunk_size as usize),
enable_light_client_server: self.enable_light_client_server,
phantom: PhantomData,
ttfb_timeout: self.chain_spec.clone().ttfb_timeout(),
},
(),
),
Expand Down
11 changes: 5 additions & 6 deletions beacon_node/lighthouse_network/src/rpc/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use tokio_util::{
};
use types::{
BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockCapella, BeaconBlockMerge,
ChainSpec, EmptyBlock, EthSpec, ForkContext, ForkName, Hash256, MainnetEthSpec, Signature,
EmptyBlock, EthSpec, ForkContext, ForkName, Hash256, MainnetEthSpec, Signature,
SignedBeaconBlock,
};

Expand Down Expand Up @@ -119,9 +119,9 @@ const PROTOCOL_PREFIX: &str = "/eth2/beacon_chain/req";
const REQUEST_TIMEOUT: u64 = 15;

/// Returns the maximum bytes that can be sent across the RPC.
pub fn max_rpc_size(fork_context: &ForkContext, spec: &ChainSpec) -> usize {
pub fn max_rpc_size(fork_context: &ForkContext, max_chunk_size: usize) -> usize {
match fork_context.current_fork() {
ForkName::Altair | ForkName::Base => spec.max_chunk_size as usize,
ForkName::Altair | ForkName::Base => max_chunk_size,
ForkName::Merge => MAX_RPC_SIZE_POST_MERGE,
ForkName::Capella => MAX_RPC_SIZE_POST_CAPELLA,
}
pawanjay176 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -258,6 +258,7 @@ pub struct RPCProtocol<TSpec: EthSpec> {
pub max_rpc_size: usize,
pub enable_light_client_server: bool,
pub phantom: PhantomData<TSpec>,
pub ttfb_timeout: Duration,
}

impl<TSpec: EthSpec> UpgradeInfo for RPCProtocol<TSpec> {
Expand Down Expand Up @@ -443,9 +444,7 @@ where
}
};
let mut timed_socket = TimeoutStream::new(socket);
timed_socket.set_read_timeout(Some(Duration::from_secs(
TSpec::default_spec().ttfb_timeout,
)));
timed_socket.set_read_timeout(Some(self.ttfb_timeout));

let socket = Framed::new(Box::pin(timed_socket), codec);

Expand Down
4 changes: 2 additions & 2 deletions beacon_node/lighthouse_network/tests/rpc_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn merge_block_small(fork_context: &ForkContext, spec: &ChainSpec) -> BeaconBloc
block.body.execution_payload.execution_payload.transactions = txs;

let block = BeaconBlock::Merge(block);
assert!(block.ssz_bytes_len() <= max_rpc_size(fork_context, spec));
assert!(block.ssz_bytes_len() <= max_rpc_size(fork_context, spec.max_chunk_size as usize));
block
}

Expand All @@ -42,7 +42,7 @@ fn merge_block_large(fork_context: &ForkContext, spec: &ChainSpec) -> BeaconBloc
block.body.execution_payload.execution_payload.transactions = txs;

let block = BeaconBlock::Merge(block);
assert!(block.ssz_bytes_len() > max_rpc_size(fork_context, spec));
assert!(block.ssz_bytes_len() > max_rpc_size(fork_context, spec.max_chunk_size as usize));
block
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2187,6 +2187,7 @@ impl<T: BeaconChainTypes> Worker<T> {
sync_committee_verification::verify_propagation_slot_range(
seen_clock,
&sync_committee_message_slot,
&self.chain.spec,
);
hindsight_verification.is_err()
};
Expand Down
13 changes: 13 additions & 0 deletions consensus/types/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use serde_derive::Deserialize;
use serde_utils::quoted_u64::MaybeQuoted;
use std::fs::File;
use std::path::Path;
use std::time::Duration;
use tree_hash::TreeHash;

/// Each of the BLS signature domains.
Expand Down Expand Up @@ -461,6 +462,18 @@ impl ChainSpec {
Hash256::from(domain)
}

pub fn maximum_gossip_clock_disparity(self) -> Duration {
Duration::from_millis(self.maximum_gossip_clock_disparity_millis)
}

pub fn ttfb_timeout(self) -> Duration {
Duration::from_secs(self.ttfb_timeout)
}

pub fn resp_timeout(self) -> Duration {
Duration::from_secs(self.resp_timeout)
}

pawanjay176 marked this conversation as resolved.
Show resolved Hide resolved
/// Returns a `ChainSpec` compatible with the Ethereum Foundation specification.
pub fn mainnet() -> Self {
Self {
Expand Down
Loading