Skip to content

Commit 47f336f

Browse files
authored
Leave gaps in sender chains on the validator side (#4181)
## Motivation If a client creates a block receiving messages from a chain with gaps, and a validator doesn't have that chain yet, the client currently has no way of convincing the validator that the proposal is correct, because the validator expects the whole sender chain as proof of the sent messages. ## Proposal Make validators accept confirmed certificates for blocks on chains with gaps. If there is a gap, blocks should only be preprocessed, instead of fully processed - like on the clients. ## Test Plan CI with an added test exercising the problematic scenario ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist) - closes #4160
1 parent ad2a437 commit 47f336f

File tree

9 files changed

+338
-64
lines changed

9 files changed

+338
-64
lines changed

linera-chain/src/block.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ pub struct BlockBody {
345345
/// The list of outgoing messages for each transaction.
346346
pub messages: Vec<Vec<OutgoingMessage>>,
347347
/// The hashes of previous blocks that sent messages to the same recipients.
348-
pub previous_message_blocks: BTreeMap<ChainId, CryptoHash>,
348+
pub previous_message_blocks: BTreeMap<ChainId, (CryptoHash, BlockHeight)>,
349349
/// The record of oracle responses for each transaction.
350350
pub oracle_responses: Vec<Vec<OracleResponse>>,
351351
/// The list of events produced by each transaction.
@@ -586,7 +586,7 @@ impl BcsHashable<'_> for Block {}
586586

587587
#[derive(Serialize, Deserialize)]
588588
pub struct PreviousMessageBlocksMap<'a> {
589-
inner: Cow<'a, BTreeMap<ChainId, CryptoHash>>,
589+
inner: Cow<'a, BTreeMap<ChainId, (CryptoHash, BlockHeight)>>,
590590
}
591591

592592
impl<'de> BcsHashable<'de> for PreviousMessageBlocksMap<'de> {}

linera-chain/src/chain.rs

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ where
788788
.ok_or_else(|| {
789789
ChainError::InternalError("missing entry in confirmed_log".into())
790790
})?;
791-
previous_message_blocks.insert(recipient, hash);
791+
previous_message_blocks.insert(recipient, (hash, height));
792792
}
793793
}
794794

@@ -969,8 +969,7 @@ where
969969
Ok(())
970970
}
971971

972-
/// Returns the hashes of all blocks in the given range. Returns an error if we are missing
973-
/// any of those blocks.
972+
/// Returns the hashes of all blocks we have in the given range.
974973
pub async fn block_hashes(
975974
&self,
976975
range: impl RangeBounds<BlockHeight>,
@@ -990,14 +989,9 @@ where
990989
};
991990
// Everything after (including) next_height in in preprocessed_blocks if we have it.
992991
for height in start.max(next_height).0..=end.0 {
993-
hashes.push(
994-
self.preprocessed_blocks
995-
.get(&BlockHeight(height))
996-
.await?
997-
.ok_or_else(|| {
998-
ChainError::InternalError("missing entry in preprocessed_blocks".into())
999-
})?,
1000-
);
992+
if let Some(hash) = self.preprocessed_blocks.get(&BlockHeight(height)).await? {
993+
hashes.push(hash);
994+
}
1001995
}
1002996
Ok(hashes)
1003997
}
@@ -1040,7 +1034,8 @@ where
10401034
if block_height > next_height {
10411035
// There may be a gap in the chain before this block. We can only add it to this
10421036
// outbox if the previous message to the same recipient has already been added.
1043-
let prev_hash = match outbox.next_height_to_schedule.get().try_sub_one().ok() {
1037+
let maybe_prev_hash = match outbox.next_height_to_schedule.get().try_sub_one().ok()
1038+
{
10441039
// The block with the last added message has already been executed; look up its
10451040
// hash in the confirmed_log.
10461041
Some(height) if height < next_height => {
@@ -1058,8 +1053,38 @@ where
10581053
None => None, // No message to that sender was added yet.
10591054
};
10601055
// Only schedule if this block contains the next message for that recipient.
1061-
if prev_hash.as_ref() != block.body.previous_message_blocks.get(target) {
1062-
continue;
1056+
match (
1057+
maybe_prev_hash,
1058+
block.body.previous_message_blocks.get(target),
1059+
) {
1060+
(None, None) => {
1061+
// No previous message block expected and none indicated by the outbox -
1062+
// all good
1063+
}
1064+
(Some(_), None) => {
1065+
// Outbox indicates there was a previous message block, but
1066+
// previous_message_blocks has no idea about it - possible bug
1067+
return Err(ChainError::InternalError(
1068+
"block indicates no previous message block,\
1069+
but we have one in the outbox"
1070+
.into(),
1071+
));
1072+
}
1073+
(None, Some((_, prev_msg_block_height))) => {
1074+
// We have no previously processed block in the outbox, but we are
1075+
// expecting one - this could be due to an empty outbox having been pruned.
1076+
// Only process the outbox if the height of the previous message block is
1077+
// lower than the tip
1078+
if *prev_msg_block_height >= next_height {
1079+
continue;
1080+
}
1081+
}
1082+
(Some(ref prev_hash), Some((prev_msg_block_hash, _))) => {
1083+
// Only process the outbox if the hashes match.
1084+
if prev_hash != prev_msg_block_hash {
1085+
continue;
1086+
}
1087+
}
10631088
}
10641089
}
10651090
if outbox.schedule_message(block_height)? {

linera-chain/src/data_types.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ pub struct BlockExecutionOutcome {
296296
/// The list of outgoing messages for each transaction.
297297
pub messages: Vec<Vec<OutgoingMessage>>,
298298
/// The hashes of previous blocks that sent messages to the same recipients.
299-
pub previous_message_blocks: BTreeMap<ChainId, CryptoHash>,
299+
pub previous_message_blocks: BTreeMap<ChainId, (CryptoHash, BlockHeight)>,
300300
/// The hash of the chain's execution state after this block.
301301
pub state_hash: CryptoHash,
302302
/// The record of oracle responses for each transaction.

linera-core/src/chain_worker/state/attempted_changes.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,11 +290,6 @@ where
290290

291291
// Check that the chain is active and ready for this confirmation.
292292
let tip = self.state.chain.tip_state.get().clone();
293-
if tip.next_block_height < height {
294-
return Err(WorkerError::MissingEarlierBlocks {
295-
current_block_height: tip.next_block_height,
296-
});
297-
}
298293
if tip.next_block_height > height {
299294
// We already processed this block.
300295
let actions = self.state.create_network_actions().await?;
@@ -304,6 +299,7 @@ where
304299
return Ok((info, actions));
305300
}
306301

302+
// We haven't processed the block - verify the certificate first
307303
if height == BlockHeight::ZERO {
308304
// For the initial proposal on a chain, we read the committee directly from
309305
// storage. If the certificate is good for this committee, we will accept it -
@@ -325,6 +321,17 @@ where
325321
check_block_epoch(epoch, chain_id, block.header.epoch)?;
326322
certificate.check(committee)?;
327323
}
324+
325+
// If this block is higher than the next expected block in this chain, we're going
326+
// to have a gap: do not process this block fully, only preprocess it.
327+
if tip.next_block_height < height {
328+
let actions = self.preprocess_certificate(certificate).await?;
329+
self.register_delivery_notifier(height, &actions, notify_when_messages_are_delivered)
330+
.await;
331+
let info = ChainInfoResponse::new(&self.state.chain, self.state.config.key_pair());
332+
return Ok((info, actions));
333+
}
334+
328335
// This should always be true for valid certificates.
329336
ensure!(
330337
tip.block_hash == block.header.previous_block_hash,

linera-core/src/client/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ impl<Env: Environment> Client<Env> {
507507
}
508508

509509
#[instrument(level = "trace", skip_all)]
510-
pub async fn preprocess_certificate(
510+
async fn preprocess_certificate(
511511
&self,
512512
certificate: Box<ConfirmedBlockCertificate>,
513513
) -> Result<(), LocalNodeError> {

0 commit comments

Comments
 (0)