Skip to content

Commit

Permalink
Shift networking configuration (sigp#4426)
Browse files Browse the repository at this point in the history
Addresses [sigp#4401](sigp#4401)

Shift some constants into ```ChainSpec``` and remove the constant values from code space.

I mostly used ```MainnetEthSpec::default_spec()``` for getting ```ChainSpec```. I wonder Did I make a mistake about that.

Co-authored-by: armaganyildirak <armaganyildirak@gmail.com>
Co-authored-by: Paul Hauner <paul@paulhauner.com>
Co-authored-by: Age Manning <Age@AgeManning.com>
Co-authored-by: Diva M <divma@protonmail.com>
  • Loading branch information
5 people authored and Woodpile37 committed Jan 6, 2024
1 parent 9399ffa commit 888006e
Show file tree
Hide file tree
Showing 27 changed files with 336 additions and 138 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

20 changes: 9 additions & 11 deletions beacon_node/beacon_chain/src/attestation_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@
mod batch;

use crate::{
beacon_chain::{MAXIMUM_GOSSIP_CLOCK_DISPARITY, VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT},
metrics,
observed_aggregates::ObserveOutcome,
observed_attesters::Error as ObservedAttestersError,
beacon_chain::VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, metrics,
observed_aggregates::ObserveOutcome, observed_attesters::Error as ObservedAttestersError,
BeaconChain, BeaconChainError, BeaconChainTypes,
};
use bls::verify_signature_sets;
Expand All @@ -57,8 +55,8 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, BeaconCommittee, CommitteeIndex, Epoch, EthSpec, Hash256, IndexedAttestation,
SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
Attestation, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec, Hash256,
IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
};

pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
Expand Down Expand Up @@ -454,7 +452,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
// We do not queue future attestations for later processing.
verify_propagation_slot_range(&chain.slot_clock, attestation)?;
verify_propagation_slot_range(&chain.slot_clock, attestation, &chain.spec)?;

// Check the attestation's epoch matches its target.
if attestation.data.slot.epoch(T::EthSpec::slots_per_epoch())
Expand Down Expand Up @@ -722,7 +720,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
// MAXIMUM_GOSSIP_CLOCK_DISPARITY allowance).
//
// We do not queue future attestations for later processing.
verify_propagation_slot_range(&chain.slot_clock, attestation)?;
verify_propagation_slot_range(&chain.slot_clock, attestation, &chain.spec)?;

// Check to ensure that the attestation is "unaggregated". I.e., it has exactly one
// aggregation bit set.
Expand Down Expand Up @@ -1037,11 +1035,11 @@ fn verify_head_block_is_known<T: BeaconChainTypes>(
pub fn verify_propagation_slot_range<S: SlotClock, E: EthSpec>(
slot_clock: &S,
attestation: &Attestation<E>,
spec: &ChainSpec,
) -> Result<(), Error> {
let attestation_slot = attestation.data.slot;

let latest_permissible_slot = slot_clock
.now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY)
.now_with_future_tolerance(spec.maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?;
if attestation_slot > latest_permissible_slot {
return Err(Error::FutureSlot {
Expand All @@ -1052,7 +1050,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.maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?
- E::slots_per_epoch();
if attestation_slot < earliest_permissible_slot {
Expand Down
5 changes: 0 additions & 5 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,6 @@ pub enum OverrideForkchoiceUpdate {
AlreadyApplied,
}

/// The accepted clock drift for nodes gossiping blocks and attestations. See:
///
/// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/p2p-interface.md#configuration
pub const MAXIMUM_GOSSIP_CLOCK_DISPARITY: Duration = Duration::from_millis(500);

#[derive(Debug, PartialEq)]
pub enum AttestationProcessingOutcome {
Processed,
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub use self::beacon_chain::{
AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BeaconStore, ChainSegmentResult,
ForkChoiceError, OverrideForkchoiceUpdate, ProduceBlockVerification, StateSkipConfig,
WhenSlotSkipped, INVALID_FINALIZED_MERGE_TRANSITION_BLOCK_SHUTDOWN_REASON,
INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON,
};
pub use self::beacon_snapshot::BeaconSnapshot;
pub use self::chain_config::ChainConfig;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::{
beacon_chain::MAXIMUM_GOSSIP_CLOCK_DISPARITY, BeaconChain, BeaconChainError, BeaconChainTypes,
};
use crate::{BeaconChain, BeaconChainError, BeaconChainTypes};
use derivative::Derivative;
use slot_clock::SlotClock;
use std::time::Duration;
Expand Down Expand Up @@ -103,7 +101,8 @@ impl<T: BeaconChainTypes> VerifiedLightClientFinalityUpdate<T> {
// verify that enough time has passed for the block to have been propagated
match start_time {
Some(time) => {
if seen_timestamp + MAXIMUM_GOSSIP_CLOCK_DISPARITY < time + one_third_slot_duration
if seen_timestamp + chain.spec.maximum_gossip_clock_disparity()
< time + one_third_slot_duration
{
return Err(Error::TooEarly);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::{
beacon_chain::MAXIMUM_GOSSIP_CLOCK_DISPARITY, BeaconChain, BeaconChainError, BeaconChainTypes,
};
use crate::{BeaconChain, BeaconChainError, BeaconChainTypes};
use derivative::Derivative;
use eth2::types::Hash256;
use slot_clock::SlotClock;
Expand Down Expand Up @@ -103,7 +101,8 @@ impl<T: BeaconChainTypes> VerifiedLightClientOptimisticUpdate<T> {
// verify that enough time has passed for the block to have been propagated
match start_time {
Some(time) => {
if seen_timestamp + MAXIMUM_GOSSIP_CLOCK_DISPARITY < time + one_third_slot_duration
if seen_timestamp + chain.spec.maximum_gossip_clock_disparity()
< time + one_third_slot_duration
{
return Err(Error::TooEarly);
}
Expand Down
17 changes: 8 additions & 9 deletions beacon_node/beacon_chain/src/sync_committee_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@

use crate::observed_attesters::SlotSubcommitteeIndex;
use crate::{
beacon_chain::{MAXIMUM_GOSSIP_CLOCK_DISPARITY, VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT},
metrics,
observed_aggregates::ObserveOutcome,
BeaconChain, BeaconChainError, BeaconChainTypes,
beacon_chain::VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, metrics,
observed_aggregates::ObserveOutcome, BeaconChain, BeaconChainError, BeaconChainTypes,
};
use bls::{verify_signature_sets, PublicKeyBytes};
use derivative::Derivative;
Expand All @@ -52,6 +50,7 @@ use tree_hash_derive::TreeHash;
use types::consts::altair::SYNC_COMMITTEE_SUBNET_COUNT;
use types::slot_data::SlotData;
use types::sync_committee::Error as SyncCommitteeError;
use types::ChainSpec;
use types::{
sync_committee_contribution::Error as ContributionError, AggregateSignature, BeaconStateError,
EthSpec, Hash256, SignedContributionAndProof, Slot, SyncCommitteeContribution,
Expand Down Expand Up @@ -297,7 +296,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 @@ -460,7 +459,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 @@ -576,11 +575,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 latest_permissible_slot = slot_clock
.now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY)
.now_with_future_tolerance(spec.maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?;
if message_slot > latest_permissible_slot {
return Err(Error::FutureSlot {
Expand All @@ -590,7 +589,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.maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?;

if message_slot < earliest_permissible_slot {
Expand Down
3 changes: 2 additions & 1 deletion beacon_node/client/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,8 @@ where
self.work_reprocessing_rx,
None,
beacon_chain.slot_clock.clone(),
);
beacon_chain.spec.maximum_gossip_clock_disparity(),
)?;
}

let state_advance_context = runtime_context.service_context("state_advance".into());
Expand Down
6 changes: 2 additions & 4 deletions beacon_node/http_api/src/attester_duties.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
//! Contains the handler for the `GET validator/duties/attester/{epoch}` endpoint.

use crate::state_id::StateId;
use beacon_chain::{
BeaconChain, BeaconChainError, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
};
use beacon_chain::{BeaconChain, BeaconChainError, BeaconChainTypes};
use eth2::types::{self as api_types};
use slot_clock::SlotClock;
use state_processing::state_advance::partial_state_advance;
Expand Down Expand Up @@ -32,7 +30,7 @@ pub fn attester_duties<T: BeaconChainTypes>(
// will equal `current_epoch + 1`
let tolerant_current_epoch = chain
.slot_clock
.now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY)
.now_with_future_tolerance(chain.spec.maximum_gossip_clock_disparity())
.ok_or_else(|| warp_utils::reject::custom_server_error("unable to read slot clock".into()))?
.epoch(T::EthSpec::slots_per_epoch());

Expand Down
4 changes: 2 additions & 2 deletions beacon_node/http_api/src/proposer_duties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::state_id::StateId;
use beacon_chain::{
beacon_proposer_cache::{compute_proposer_duties_from_head, ensure_state_is_in_epoch},
BeaconChain, BeaconChainError, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
BeaconChain, BeaconChainError, BeaconChainTypes,
};
use eth2::types::{self as api_types};
use safe_arith::SafeArith;
Expand Down Expand Up @@ -33,7 +33,7 @@ pub fn proposer_duties<T: BeaconChainTypes>(
// will equal `current_epoch + 1`
let tolerant_current_epoch = chain
.slot_clock
.now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY)
.now_with_future_tolerance(chain.spec.maximum_gossip_clock_disparity())
.ok_or_else(|| warp_utils::reject::custom_server_error("unable to read slot clock".into()))?
.epoch(T::EthSpec::slots_per_epoch());

Expand Down
4 changes: 2 additions & 2 deletions beacon_node/http_api/src/sync_committees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use beacon_chain::sync_committee_verification::{
};
use beacon_chain::{
validator_monitor::timestamp_now, BeaconChain, BeaconChainError, BeaconChainTypes,
StateSkipConfig, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
StateSkipConfig,
};
use eth2::types::{self as api_types};
use lighthouse_network::PubsubMessage;
Expand Down Expand Up @@ -85,7 +85,7 @@ fn duties_from_state_load<T: BeaconChainTypes>(
let current_epoch = chain.epoch()?;
let tolerant_current_epoch = chain
.slot_clock
.now_with_future_tolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY)
.now_with_future_tolerance(chain.spec.maximum_gossip_clock_disparity())
.ok_or(BeaconChainError::UnableToReadSlot)?
.epoch(T::EthSpec::slots_per_epoch());

Expand Down
12 changes: 7 additions & 5 deletions beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use beacon_chain::test_utils::RelativeSyncCommittee;
use beacon_chain::{
test_utils::{AttestationStrategy, BeaconChainHarness, BlockStrategy, EphemeralHarnessType},
BeaconChain, StateSkipConfig, WhenSlotSkipped, MAXIMUM_GOSSIP_CLOCK_DISPARITY,
BeaconChain, StateSkipConfig, WhenSlotSkipped,
};
use environment::null_logger;
use eth2::{
Expand Down Expand Up @@ -2313,7 +2313,9 @@ impl ApiTester {
.unwrap();

self.chain.slot_clock.set_current_time(
current_epoch_start - MAXIMUM_GOSSIP_CLOCK_DISPARITY - Duration::from_millis(1),
current_epoch_start
- self.chain.spec.maximum_gossip_clock_disparity()
- Duration::from_millis(1),
);

let dependent_root = self
Expand Down Expand Up @@ -2350,9 +2352,9 @@ impl ApiTester {
"should not get attester duties outside of tolerance"
);

self.chain
.slot_clock
.set_current_time(current_epoch_start - MAXIMUM_GOSSIP_CLOCK_DISPARITY);
self.chain.slot_clock.set_current_time(
current_epoch_start - self.chain.spec.maximum_gossip_clock_disparity(),
);

self.client
.get_validator_duties_proposer(current_epoch)
Expand Down
36 changes: 19 additions & 17 deletions beacon_node/lighthouse_network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ use std::sync::Arc;
use std::time::Duration;
use types::{ForkContext, ForkName};

/// The maximum transmit size of gossip messages in bytes pre-merge.
const GOSSIP_MAX_SIZE: usize = 1_048_576; // 1M
/// The maximum transmit size of gossip messages in bytes post-merge.
const GOSSIP_MAX_SIZE_POST_MERGE: usize = 10 * 1_048_576; // 10M

/// The cache time is set to accommodate the circulation time of an attestation.
///
/// The p2p spec declares that we accept attestations within the following range:
Expand All @@ -35,20 +30,20 @@ const GOSSIP_MAX_SIZE_POST_MERGE: usize = 10 * 1_048_576; // 10M
/// another 500ms for "fudge factor".
pub const DUPLICATE_CACHE_TIME: Duration = Duration::from_secs(33 * 12 + 1);

// We treat uncompressed messages as invalid and never use the INVALID_SNAPPY_DOMAIN as in the
// specification. We leave it here for posterity.
// const MESSAGE_DOMAIN_INVALID_SNAPPY: [u8; 4] = [0, 0, 0, 0];
const MESSAGE_DOMAIN_VALID_SNAPPY: [u8; 4] = [1, 0, 0, 0];

/// The maximum size of gossip messages.
pub fn gossip_max_size(is_merge_enabled: bool) -> usize {
pub fn gossip_max_size(is_merge_enabled: bool, gossip_max_size: usize) -> usize {
if is_merge_enabled {
GOSSIP_MAX_SIZE_POST_MERGE
gossip_max_size
} else {
GOSSIP_MAX_SIZE
gossip_max_size / 10
}
}

pub struct GossipsubConfigParams {
pub message_domain_valid_snappy: [u8; 4],
pub gossip_max_size: usize,
}

#[derive(Clone, Debug, Serialize, Deserialize)]
#[serde(default)]
/// Network configuration for lighthouse.
Expand Down Expand Up @@ -413,7 +408,11 @@ impl From<u8> for NetworkLoad {
}

/// Return a Lighthouse specific `GossipsubConfig` where the `message_id_fn` depends on the current fork.
pub fn gossipsub_config(network_load: u8, fork_context: Arc<ForkContext>) -> gossipsub::Config {
pub fn gossipsub_config(
network_load: u8,
fork_context: Arc<ForkContext>,
gossipsub_config_params: GossipsubConfigParams,
) -> gossipsub::Config {
// The function used to generate a gossipsub message id
// We use the first 8 bytes of SHA256(topic, data) for content addressing
let fast_gossip_message_id = |message: &gossipsub::RawMessage| {
Expand Down Expand Up @@ -449,20 +448,23 @@ pub fn gossipsub_config(network_load: u8, fork_context: Arc<ForkContext>) -> gos
}
}
}

let message_domain_valid_snappy = gossipsub_config_params.message_domain_valid_snappy;
let is_merge_enabled = fork_context.fork_exists(ForkName::Merge);
let gossip_message_id = move |message: &gossipsub::Message| {
gossipsub::MessageId::from(
&Sha256::digest(
prefix(MESSAGE_DOMAIN_VALID_SNAPPY, message, fork_context.clone()).as_slice(),
prefix(message_domain_valid_snappy, message, fork_context.clone()).as_slice(),
)[..20],
)
};

let load = NetworkLoad::from(network_load);

gossipsub::ConfigBuilder::default()
.max_transmit_size(gossip_max_size(is_merge_enabled))
.max_transmit_size(gossip_max_size(
is_merge_enabled,
gossipsub_config_params.gossip_max_size,
))
.heartbeat_interval(load.heartbeat_interval)
.mesh_n(load.mesh_n)
.mesh_n_low(load.mesh_n_low)
Expand Down
Loading

0 comments on commit 888006e

Please sign in to comment.