Skip to content

Commit 48843ba

Browse files
committed
Check lateness of block before requeuing it (#4208)
## Issue Addressed NA ## Proposed Changes Avoids reprocessing loops introduced in #4179. (Also somewhat related to #4192). Breaks the re-queue loop by only re-queuing when an RPC block is received before the attestation creation deadline. I've put `proposal_is_known` behind a closure to avoid interacting with the `observed_proposers` lock unnecessarily. ## Additional Info NA
1 parent 4343867 commit 48843ba

File tree

1 file changed

+36
-16
lines changed

1 file changed

+36
-16
lines changed

beacon_node/network/src/beacon_processor/worker/sync_methods.rs

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@ use crate::sync::manager::{BlockProcessType, SyncMessage};
99
use crate::sync::{BatchProcessResult, ChainId};
1010
use beacon_chain::CountUnrealized;
1111
use beacon_chain::{
12-
observed_block_producers::Error as ObserveError, BeaconChainError, BeaconChainTypes,
13-
BlockError, ChainSegmentResult, HistoricalBlockError, NotifyExecutionLayer,
12+
observed_block_producers::Error as ObserveError, validator_monitor::get_block_delay_ms,
13+
BeaconChainError, BeaconChainTypes, BlockError, ChainSegmentResult, HistoricalBlockError,
14+
NotifyExecutionLayer,
1415
};
1516
use lighthouse_network::PeerAction;
1617
use slog::{debug, error, info, warn};
18+
use slot_clock::SlotClock;
1719
use std::sync::Arc;
20+
use std::time::{SystemTime, UNIX_EPOCH};
1821
use tokio::sync::mpsc;
1922
use types::{Epoch, Hash256, SignedBeaconBlock};
2023

@@ -83,21 +86,38 @@ impl<T: BeaconChainTypes> Worker<T> {
8386
return;
8487
}
8588
};
86-
// Check if a block from this proposer is already known. If so, defer processing until later
87-
// to avoid wasting time processing duplicates.
88-
let proposal_already_known = match self
89-
.chain
90-
.observed_block_producers
91-
.read()
92-
.proposer_has_been_observed(block.message())
93-
{
94-
Ok(is_observed) => is_observed,
95-
// Both of these blocks will be rejected, so reject them now rather
96-
// than re-queuing them.
97-
Err(ObserveError::FinalizedBlock { .. })
98-
| Err(ObserveError::ValidatorIndexTooHigh { .. }) => false,
89+
90+
// Returns `true` if the time now is after the 4s attestation deadline.
91+
let block_is_late = SystemTime::now()
92+
.duration_since(UNIX_EPOCH)
93+
// If we can't read the system time clock then indicate that the
94+
// block is late (and therefore should *not* be requeued). This
95+
// avoids infinite loops.
96+
.map_or(true, |now| {
97+
get_block_delay_ms(now, block.message(), &self.chain.slot_clock)
98+
> self.chain.slot_clock.unagg_attestation_production_delay()
99+
});
100+
101+
// Checks if a block from this proposer is already known.
102+
let proposal_already_known = || {
103+
match self
104+
.chain
105+
.observed_block_producers
106+
.read()
107+
.proposer_has_been_observed(block.message())
108+
{
109+
Ok(is_observed) => is_observed,
110+
// Both of these blocks will be rejected, so reject them now rather
111+
// than re-queuing them.
112+
Err(ObserveError::FinalizedBlock { .. })
113+
| Err(ObserveError::ValidatorIndexTooHigh { .. }) => false,
114+
}
99115
};
100-
if proposal_already_known {
116+
117+
// If we've already seen a block from this proposer *and* the block
118+
// arrived before the attestation deadline, requeue it to ensure it is
119+
// imported late enough that it won't receive a proposer boost.
120+
if !block_is_late && proposal_already_known() {
101121
debug!(
102122
self.log,
103123
"Delaying processing of duplicate RPC block";

0 commit comments

Comments
 (0)