Skip to content

Commit 1e08cac

Browse files
authored
Simplify chain manager logic. (#3178)
## Motivation We are using the chain manager's `locked` field for two purposes: * In validators, this is meant to track _this validator's_ locked block, i.e. the latest proposal it signed to confirm. * In the client, it tracks the highest validated block (or fast proposal) we have seen, i.e. the highest block that _might_ be locked for any honest validator, so that the client knows which block it should re-propose in the next round. This is confusing, and makes it more difficult to tackle issues like #2971. ## Proposal Use the `locked` field for the first purpose, both in clients and validators: to track the highest block that _might_ be locked for any of the validators. In a validator's chain manager, the block that is locked for _that validator_ is the one it has last voted to confirm, i.e. the `confirmed_vote` field. ## Test Plan The tests already cover lots of scenarios involving locked blocks, and should catch any regressions. They have been updated where appropriate. ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - Closes #3170. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
1 parent 57a3eaa commit 1e08cac

File tree

16 files changed

+273
-278
lines changed

16 files changed

+273
-278
lines changed

linera-chain/src/block.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use crate::{
2323
BlockExecutionOutcome, EventRecord, ExecutedBlock, IncomingBundle, Medium, MessageBundle,
2424
OutgoingMessage, ProposedBlock,
2525
},
26+
types::CertificateValue,
2627
ChainError,
2728
};
2829

@@ -154,6 +155,28 @@ impl ConfirmedBlock {
154155
})
155156
}
156157
}
158+
159+
/// Returns whether this block matches the proposal.
160+
pub fn matches_proposed_block(&self, block: &ProposedBlock) -> bool {
161+
let ProposedBlock {
162+
chain_id,
163+
epoch,
164+
incoming_bundles,
165+
operations,
166+
height,
167+
timestamp,
168+
authenticated_signer,
169+
previous_block_hash,
170+
} = block;
171+
*chain_id == self.chain_id()
172+
&& *epoch == self.epoch()
173+
&& *incoming_bundles == self.block().body.incoming_bundles
174+
&& *operations == self.block().body.operations
175+
&& *height == self.block().header.height
176+
&& *timestamp == self.block().header.timestamp
177+
&& *authenticated_signer == self.block().header.authenticated_signer
178+
&& *previous_block_hash == self.block().header.previous_block_hash
179+
}
157180
}
158181

159182
#[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize, Serialize)]

linera-chain/src/lib.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,10 @@ pub enum ChainError {
127127
InsufficientRoundStrict(Round),
128128
#[error("Round number should be {0:?}")]
129129
WrongRound(Round),
130-
#[error("A different block for height {0:?} was already locked at round number {1:?}")]
131-
HasLockedBlock(BlockHeight, Round),
130+
#[error("Already voted to confirm a different block for height {0:?} at round number {1:?}")]
131+
HasIncompatibleConfirmedVote(BlockHeight, Round),
132+
#[error("Proposal for height {0:?} is not newer than locking block in round {1:?}")]
133+
MustBeNewerThanLockingBlock(BlockHeight, Round),
132134
#[error("Cannot confirm a block before its predecessors: {current_block_height:?}")]
133135
MissingEarlierBlocks { current_block_height: BlockHeight },
134136
#[error("Signatures in a certificate must be from different validators")]

linera-chain/src/manager.rs

Lines changed: 132 additions & 155 deletions
Large diffs are not rendered by default.

linera-core/src/chain_worker/actor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ where
141141
callback: oneshot::Sender<Result<(ChainInfoResponse, NetworkActions), WorkerError>>,
142142
},
143143

144-
/// Get a blob if it belongs to the current locked block or pending proposal.
144+
/// Get a blob if it belongs to the current locking block or pending proposal.
145145
DownloadPendingBlob {
146146
blob_id: BlobId,
147147
#[debug(skip)]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ where
295295
Ok((response, actions))
296296
}
297297

298-
/// Returns the requested blob, if it belongs to the current locked block or pending proposal.
298+
/// Returns the requested blob, if it belongs to the current locking block or pending proposal.
299299
pub(super) async fn download_pending_blob(&self, blob_id: BlobId) -> Result<Blob, WorkerError> {
300300
let maybe_blob = self.chain.manager.pending_blob(&blob_id).await?;
301301
maybe_blob.ok_or_else(|| WorkerError::BlobsNotFound(vec![blob_id]))

linera-core/src/client/mod.rs

Lines changed: 53 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use linera_chain::{
4444
BlockProposal, ChainAndHeight, ExecutedBlock, IncomingBundle, LiteVote, MessageAction,
4545
ProposedBlock,
4646
},
47-
manager::LockedBlock,
47+
manager::LockingBlock,
4848
types::{
4949
CertificateKind, CertificateValue, ConfirmedBlock, ConfirmedBlockCertificate,
5050
GenericCertificate, LiteCertificate, Timeout, TimeoutCertificate, ValidatedBlock,
@@ -1747,65 +1747,58 @@ where
17471747
{
17481748
return Ok(());
17491749
};
1750+
let mut proposals = Vec::new();
17501751
if let Some(proposal) = info.manager.requested_proposed {
1752+
proposals.push(*proposal);
1753+
}
1754+
if let Some(locking) = info.manager.requested_locking {
1755+
match *locking {
1756+
LockingBlock::Fast(proposal) => {
1757+
proposals.push(proposal);
1758+
}
1759+
LockingBlock::Regular(cert) => {
1760+
let hash = cert.hash();
1761+
if let Err(err) = self.try_process_locking_block_from(remote_node, cert).await {
1762+
warn!(
1763+
"Skipping certificate {hash} from validator {}: {err}",
1764+
remote_node.name
1765+
);
1766+
}
1767+
}
1768+
}
1769+
}
1770+
for proposal in proposals {
17511771
let owner = proposal.owner;
1752-
while let Err(original_err) = self
1772+
if let Err(mut err) = self
17531773
.client
17541774
.local_node
1755-
.handle_block_proposal(*proposal.clone())
1775+
.handle_block_proposal(proposal.clone())
17561776
.await
17571777
{
1758-
if let LocalNodeError::BlobsNotFound(blob_ids) = &original_err {
1778+
if let LocalNodeError::BlobsNotFound(blob_ids) = &err {
17591779
self.update_local_node_with_blobs_from(blob_ids.clone(), remote_node)
17601780
.await?;
1761-
continue; // We found the missing blobs: retry.
1762-
}
1763-
1764-
warn!(
1765-
"Skipping proposal from {} and validator {}: {}",
1766-
owner, remote_node.name, original_err
1767-
);
1768-
break;
1769-
}
1770-
}
1771-
if let Some(locked) = info.manager.requested_locked {
1772-
match *locked {
1773-
LockedBlock::Fast(proposal) => {
1774-
let owner = proposal.owner;
1775-
while let Err(original_err) = self
1781+
// We found the missing blobs: retry.
1782+
if let Err(new_err) = self
17761783
.client
17771784
.local_node
17781785
.handle_block_proposal(proposal.clone())
17791786
.await
17801787
{
1781-
if let LocalNodeError::BlobsNotFound(blob_ids) = &original_err {
1782-
self.update_local_node_with_blobs_from(blob_ids.clone(), remote_node)
1783-
.await?;
1784-
continue; // We found the missing blobs: retry.
1785-
}
1786-
1787-
warn!(
1788-
"Skipping proposal from {} and validator {}: {}",
1789-
owner, remote_node.name, original_err
1790-
);
1791-
break;
1792-
}
1793-
}
1794-
LockedBlock::Regular(cert) => {
1795-
let hash = cert.hash();
1796-
if let Err(err) = self.try_process_locked_block_from(remote_node, cert).await {
1797-
warn!(
1798-
"Skipping certificate {hash} from validator {}: {err}",
1799-
remote_node.name
1800-
);
1788+
err = new_err;
1789+
} else {
1790+
continue;
18011791
}
18021792
}
1793+
1794+
let name = &remote_node.name;
1795+
warn!("Skipping proposal from {owner} and validator {name}: {err}",);
18031796
}
18041797
}
18051798
Ok(())
18061799
}
18071800

1808-
async fn try_process_locked_block_from(
1801+
async fn try_process_locking_block_from(
18091802
&self,
18101803
remote_node: &RemoteNode<P::Node>,
18111804
certificate: GenericCertificate<ValidatedBlock>,
@@ -1984,7 +1977,7 @@ where
19841977

19851978
let maybe_blob = {
19861979
let chain_state_view = self.chain_state_view().await?;
1987-
chain_state_view.manager.locked_blobs.get(&blob_id).await?
1980+
chain_state_view.manager.locking_blobs.get(&blob_id).await?
19881981
};
19891982

19901983
if let Some(blob) = maybe_blob {
@@ -2426,17 +2419,18 @@ where
24262419
let info = self.request_leader_timeout_if_needed().await?;
24272420

24282421
// If there is a validated block in the current round, finalize it.
2429-
if info.manager.has_locked_block_in_current_round() && !info.manager.current_round.is_fast()
2422+
if info.manager.has_locking_block_in_current_round()
2423+
&& !info.manager.current_round.is_fast()
24302424
{
2431-
return self.finalize_locked_block(info).await;
2425+
return self.finalize_locking_block(info).await;
24322426
}
24332427

24342428
// Otherwise we have to re-propose the highest validated block, if there is one.
24352429
let pending: Option<ProposedBlock> = self.state().pending_proposal().clone();
2436-
let executed_block = if let Some(locked) = &info.manager.requested_locked {
2437-
match &**locked {
2438-
LockedBlock::Regular(certificate) => certificate.block().clone().into(),
2439-
LockedBlock::Fast(proposal) => {
2430+
let executed_block = if let Some(locking) = &info.manager.requested_locking {
2431+
match &**locking {
2432+
LockingBlock::Regular(certificate) => certificate.block().clone().into(),
2433+
LockingBlock::Fast(proposal) => {
24402434
let block = proposal.content.block.clone();
24412435
self.stage_block_execution(block).await?.0
24422436
}
@@ -2463,12 +2457,12 @@ where
24632457
.already_handled_proposal(round, &executed_block.block);
24642458
let key_pair = self.key_pair().await?;
24652459
// Create the final block proposal.
2466-
let proposal = if let Some(locked) = info.manager.requested_locked {
2467-
Box::new(match *locked {
2468-
LockedBlock::Regular(cert) => {
2460+
let proposal = if let Some(locking) = info.manager.requested_locking {
2461+
Box::new(match *locking {
2462+
LockingBlock::Regular(cert) => {
24692463
BlockProposal::new_retry(round, cert, &key_pair, blobs)
24702464
}
2471-
LockedBlock::Fast(proposal) => {
2465+
LockingBlock::Fast(proposal) => {
24722466
BlockProposal::new_initial(round, proposal.content.block, &key_pair, blobs)
24732467
}
24742468
})
@@ -2517,19 +2511,19 @@ where
25172511
Ok(info)
25182512
}
25192513

2520-
/// Finalizes the locked block.
2514+
/// Finalizes the locking block.
25212515
///
2522-
/// Panics if there is no locked block; fails if the locked block is not in the current round.
2523-
async fn finalize_locked_block(
2516+
/// Panics if there is no locking block; fails if the locking block is not in the current round.
2517+
async fn finalize_locking_block(
25242518
&self,
25252519
info: Box<ChainInfo>,
25262520
) -> Result<ClientOutcome<Option<ConfirmedBlockCertificate>>, ChainClientError> {
2527-
let locked = info
2521+
let locking = info
25282522
.manager
2529-
.requested_locked
2530-
.expect("Should have a locked block");
2531-
let LockedBlock::Regular(certificate) = *locked else {
2532-
panic!("Should have a locked validated block");
2523+
.requested_locking
2524+
.expect("Should have a locking block");
2525+
let LockingBlock::Regular(certificate) = *locking else {
2526+
panic!("Should have a locking validated block");
25332527
};
25342528
let committee = self.local_committee().await?;
25352529
match self.finalize_block(&committee, certificate.clone()).await {

linera-core/src/local_node.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,17 +187,17 @@ where
187187
Ok(storage.read_blobs(blob_ids).await?.into_iter().collect())
188188
}
189189

190-
/// Looks for the specified blobs in the local chain manager's locked blobs.
190+
/// Looks for the specified blobs in the local chain manager's locking blobs.
191191
/// Returns `Ok(None)` if any of the blobs is not found.
192-
pub async fn get_locked_blobs(
192+
pub async fn get_locking_blobs(
193193
&self,
194194
blob_ids: &[BlobId],
195195
chain_id: ChainId,
196196
) -> Result<Option<Vec<Blob>>, LocalNodeError> {
197197
let chain = self.chain_state_view(chain_id).await?;
198198
let mut blobs = Vec::new();
199199
for blob_id in blob_ids {
200-
match chain.manager.locked_blobs.get(blob_id).await? {
200+
match chain.manager.locking_blobs.get(blob_id).await? {
201201
None => return Ok(None),
202202
Some(blob) => blobs.push(blob),
203203
}

linera-core/src/node.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ pub trait ValidatorNode {
108108
/// Downloads a blob. Returns an error if the validator does not have the blob.
109109
async fn download_blob(&self, blob_id: BlobId) -> Result<BlobContent, NodeError>;
110110

111-
/// Downloads a blob that belongs to a pending proposal or the locked block on a chain.
111+
/// Downloads a blob that belongs to a pending proposal or the locking block on a chain.
112112
async fn download_pending_blob(
113113
&self,
114114
chain_id: ChainId,

linera-core/src/remote_node.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,10 @@ impl<N: ValidatorNode> RemoteNode<N> {
153153
) -> Result<Box<ChainInfo>, NodeError> {
154154
let manager = &response.info.manager;
155155
let proposed = manager.requested_proposed.as_ref();
156-
let locked = manager.requested_locked.as_ref();
156+
let locking = manager.requested_locking.as_ref();
157157
ensure!(
158158
proposed.map_or(true, |proposal| proposal.content.block.chain_id == chain_id)
159-
&& locked.map_or(true, |cert| cert.chain_id() == chain_id)
159+
&& locking.map_or(true, |cert| cert.chain_id() == chain_id)
160160
&& response.check(&self.name).is_ok(),
161161
NodeError::InvalidChainInfoResponse
162162
);

0 commit comments

Comments
 (0)