Skip to content

Commit 905bfaa

Browse files
committed
Retry proposals without CheckpointAck bundles a validator cannot hold
When a validator rejects a proposal with MissingCrossChainUpdate, the updater first tries to heal it by pushing the sender chain. That cannot work for a CheckpointAck bundle on a validator that bootstrapped the sender chain from a checkpoint later than the ack-sending block: the block sits below its tip, is vouched for by nothing, and is never resent. With enough such validators (plus ordinary unavailability), a proposal consuming the ack can never be certified, and the client would keep re-proposing it forever. The client now settles exactly the acknowledgement bundles named by such rejections in its local inbox - removing them from the pending bundles and advancing the lane's sender_pruned_cursor past them, the state a node that never received them is in - and retries the proposal without them. Skipping them is always legal: they are Simple, zero-grant bundles, and validators that do hold one discard it when the lane's next bundle is consumed; if another owner's quorum does consume it, the certified block reconciles as a settled no-op. The settlement is part of the chain state, so it survives client restarts and one-shot CLI commands instead of costing a failed proposal round-trip each time. A leftover pending proposal from an interrupted call gets the same treatment rather than wedging every future proposal. If nothing else is pending, no block is proposed at all.
1 parent d98485b commit 905bfaa

6 files changed

Lines changed: 288 additions & 13 deletions

File tree

linera-chain/src/inbox.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,26 @@ where
235235
Ok(())
236236
}
237237

238+
/// Removes the added bundle at `cursor` and records the lane as settled up to and
239+
/// including it, as if the sender had declared it so. Used for a bundle that can
240+
/// never be consumed by this chain's blocks — enough validators cannot hold it that
241+
/// no proposal consuming it gets certified. A certified consumption by another
242+
/// owner's block still reconciles, as a no-op below `sender_pruned_cursor`.
243+
pub async fn settle_unavailable_bundle(&mut self, cursor: Cursor) -> Result<(), ViewError> {
244+
let bundles = self.added_bundles.elements().await?;
245+
self.added_bundles.clear();
246+
for bundle in bundles {
247+
if bundle.cursor() != cursor {
248+
self.added_bundles.push_back(bundle);
249+
}
250+
}
251+
self.note_sender_pruned_below(Cursor {
252+
height: cursor.height,
253+
index: cursor.index.saturating_add(1),
254+
})
255+
.await
256+
}
257+
238258
/// Consumes a bundle from the inbox.
239259
///
240260
/// Returns `true` if the bundle was already known, i.e. it was present in `added_bundles`.

linera-core/src/chain_worker/state.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ use linera_base::prometheus_util::MeasureLatency as _;
1515
use linera_base::{
1616
crypto::{CryptoHash, ValidatorPublicKey},
1717
data_types::{
18-
ApplicationDescription, ArithmeticError, Blob, BlockHeight, Epoch, OracleResponse, Round,
19-
Timestamp,
18+
ApplicationDescription, ArithmeticError, Blob, BlockHeight, Cursor, Epoch, OracleResponse,
19+
Round, Timestamp,
2020
},
2121
ensure,
2222
hashed::Hashed,
@@ -1507,6 +1507,22 @@ where
15071507
Ok(CrossChainUpdateResult::Updated(last_updated_height))
15081508
}
15091509

1510+
/// Removes an incoming bundle that can never be consumed by this chain's blocks —
1511+
/// enough validators cannot hold it that no proposal consuming it gets certified —
1512+
/// and marks the lane as settled up to it, so that a certified consumption by
1513+
/// another owner's block still reconciles as a no-op.
1514+
pub(crate) async fn settle_unavailable_bundle(
1515+
&mut self,
1516+
origin: ChainId,
1517+
cursor: Cursor,
1518+
) -> Result<(), WorkerError> {
1519+
let mut inbox = self.chain.inboxes.try_load_entry_mut(&origin).await?;
1520+
inbox.settle_unavailable_bundle(cursor).await?;
1521+
drop(inbox);
1522+
self.save().await?;
1523+
Ok(())
1524+
}
1525+
15101526
/// Handles the cross-chain request confirming that the recipient was updated.
15111527
#[instrument(skip_all, fields(
15121528
chain_id = %self.chain_id(),

linera-core/src/client/chain_client/mod.rs

Lines changed: 123 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use linera_base::{
2121
crypto::{signer, CryptoHash, Signer, ValidatorPublicKey},
2222
data_types::{
2323
Amount, ApplicationDescription, ApplicationPermissions, ArithmeticError, Blob, BlobContent,
24-
BlockHeight, ChainDescription, Epoch, MessagePolicy, Round, TimeDelta, Timestamp,
24+
BlockHeight, ChainDescription, Cursor, Epoch, MessagePolicy, Round, TimeDelta, Timestamp,
2525
},
2626
ensure,
2727
identifiers::{
@@ -427,6 +427,22 @@ impl<Env: Environment> ChainClient<Env> {
427427
.proposal_mutex()
428428
}
429429

430+
/// Settles acknowledgement bundles that a validator rejected as unavailable in the
431+
/// local inbox: they are removed from the pending bundles, durably, so future
432+
/// proposals leave them out without a failed round-trip.
433+
async fn settle_unavailable_acks(
434+
&self,
435+
acks: BTreeMap<(ChainId, BlockHeight), Cursor>,
436+
) -> Result<(), Error> {
437+
for ((origin, _), cursor) in acks {
438+
self.client
439+
.local_node
440+
.settle_unavailable_bundle(self.chain_id, origin, cursor)
441+
.await?;
442+
}
443+
Ok(())
444+
}
445+
430446
/// Returns the pending proposal, if any.
431447
#[instrument(level = "trace", skip(self))]
432448
pub async fn pending_proposal(&self) -> Option<PendingProposal> {
@@ -1452,22 +1468,41 @@ impl<Env: Environment> ChainClient<Env> {
14521468
// Even if there is no pending proposal, this still calls
14531469
// `request_leader_timeout_if_needed` which ensures the local chain state
14541470
// is synchronized with the current consensus round.
1471+
let pending_acks = proposal_guard
1472+
.as_ref()
1473+
.map(|pending| ack_bundles_in(&pending.block.transactions))
1474+
.unwrap_or_default();
14551475
match self
14561476
.process_pending_block_without_prepare(&mut proposal_guard)
1457-
.await?
1477+
.await
14581478
{
1459-
ClientOutcome::Committed(Some(certificate)) => {
1479+
Ok(ClientOutcome::Committed(Some(certificate))) => {
14601480
return Ok(self.classify_committed(certificate, &operations));
14611481
}
1462-
ClientOutcome::WaitForTimeout(timeout) => {
1482+
Ok(ClientOutcome::WaitForTimeout(timeout)) => {
14631483
return Ok(ClientOutcome::WaitForTimeout(timeout))
14641484
}
1465-
ClientOutcome::Conflict(certificate) => {
1485+
Ok(ClientOutcome::Conflict(certificate)) => {
14661486
return Ok(ClientOutcome::Conflict(certificate))
14671487
}
1468-
ClientOutcome::Committed(None) => {}
1488+
Ok(ClientOutcome::Committed(None)) => {}
1489+
Err(error) => {
1490+
// The leftover proposal itself may contain an unavailable
1491+
// acknowledgement bundle; discard it rather than stay wedged. The loop
1492+
// below rebuilds the block without the bundle.
1493+
let unavailable = unavailable_ack_bundles(&error, &pending_acks);
1494+
if unavailable.is_empty() {
1495+
return Err(error);
1496+
}
1497+
warn!(
1498+
chain_id = %self.chain_id,
1499+
?unavailable,
1500+
"discarding a pending proposal with unavailable acknowledgement bundles",
1501+
);
1502+
self.settle_unavailable_acks(unavailable).await?;
1503+
*proposal_guard = None;
1504+
}
14691505
}
1470-
14711506
loop {
14721507
// Collect pending messages and epoch changes after acquiring the lock to avoid
14731508
// race conditions where messages valid for one block height are proposed at a
@@ -1483,13 +1518,38 @@ impl<Env: Environment> ChainClient<Env> {
14831518
)));
14841519
}
14851520

1521+
let proposed_acks = ack_bundles_in(&transactions);
1522+
14861523
self.new_pending_block(transactions, blobs.clone(), &mut proposal_guard)
14871524
.await?;
14881525

1489-
match self
1526+
let outcome = match self
14901527
.process_pending_block_without_prepare(&mut proposal_guard)
1491-
.await?
1528+
.await
14921529
{
1530+
Ok(outcome) => outcome,
1531+
Err(error) => {
1532+
let unavailable = unavailable_ack_bundles(&error, &proposed_acks);
1533+
if unavailable.is_empty() {
1534+
return Err(error);
1535+
}
1536+
// A validator rejected an acknowledgement bundle it does not hold —
1537+
// e.g. it bootstrapped the sender chain from a checkpoint past the
1538+
// acknowledgement, which nothing resends. Settle the bundle in the
1539+
// local inbox and retry without it: validators that do hold it
1540+
// discard it when the lane's next bundle is consumed.
1541+
warn!(
1542+
chain_id = %self.chain_id,
1543+
?unavailable,
1544+
"retrying the proposal without unavailable acknowledgement bundles",
1545+
);
1546+
self.settle_unavailable_acks(unavailable).await?;
1547+
*proposal_guard = None;
1548+
continue;
1549+
}
1550+
};
1551+
1552+
match outcome {
14931553
ClientOutcome::Committed(Some(certificate)) => {
14941554
return Ok(self.classify_committed(certificate, &operations));
14951555
}
@@ -3529,6 +3589,60 @@ impl<Env: Environment> ChainClient<Env> {
35293589
}
35303590
}
35313591

3592+
/// Returns the transactions' incoming bundles that consist solely of `CheckpointAck`
3593+
/// messages, keyed by `(origin, height)` — the fields named by
3594+
/// [`NodeError::MissingCrossChainUpdate`] — with their inbox cursors as values.
3595+
fn ack_bundles_in(transactions: &[Transaction]) -> BTreeMap<(ChainId, BlockHeight), Cursor> {
3596+
transactions
3597+
.iter()
3598+
.filter_map(|transaction| match transaction {
3599+
Transaction::ReceiveMessages(incoming)
3600+
if incoming
3601+
.bundle
3602+
.messages
3603+
.iter()
3604+
.all(|posted| posted.message.is_checkpoint_ack()) =>
3605+
{
3606+
Some((
3607+
(incoming.origin, incoming.bundle.height),
3608+
incoming.bundle.cursor(),
3609+
))
3610+
}
3611+
_ => None,
3612+
})
3613+
.collect()
3614+
}
3615+
3616+
/// Returns the subset of `ack_bundles` — the proposed bundles consisting solely of
3617+
/// `CheckpointAck` messages, keyed by `(origin, height)` — that a validator rejected
3618+
/// with [`NodeError::MissingCrossChainUpdate`]. An acknowledgement's bundle has no
3619+
/// availability guarantee, so a validator that does not hold it never will; a proposal
3620+
/// containing it can only be certified by retrying without it.
3621+
fn unavailable_ack_bundles(
3622+
error: &Error,
3623+
ack_bundles: &BTreeMap<(ChainId, BlockHeight), Cursor>,
3624+
) -> BTreeMap<(ChainId, BlockHeight), Cursor> {
3625+
let Error::CommunicationError(error) = error else {
3626+
return BTreeMap::new();
3627+
};
3628+
let node_errors = match error {
3629+
CommunicationError::Trusted(error) => vec![error],
3630+
CommunicationError::Sample(samples) => samples.iter().map(|(error, _)| error).collect(),
3631+
CommunicationError::NoConsensus(_, _) => Vec::new(),
3632+
};
3633+
node_errors
3634+
.into_iter()
3635+
.filter_map(|error| match error {
3636+
NodeError::MissingCrossChainUpdate { origin, height, .. } => {
3637+
let key = (*origin, *height);
3638+
let cursor = ack_bundles.get(&key)?;
3639+
Some((key, *cursor))
3640+
}
3641+
_ => None,
3642+
})
3643+
.collect()
3644+
}
3645+
35323646
#[cfg(with_testing)]
35333647
impl<Env: Environment> ChainClient<Env> {
35343648
/// Processes a notification received from the given validator.

linera-core/src/local_node.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,21 @@ where
491491
Ok(self.node.state.reset_and_reexecute_chain(chain_id).await?)
492492
}
493493

494+
/// Removes an incoming bundle of `chain_id` that can never be consumed by its
495+
/// blocks, and marks the lane from `origin` as settled up to it.
496+
pub async fn settle_unavailable_bundle(
497+
&self,
498+
chain_id: ChainId,
499+
origin: ChainId,
500+
cursor: linera_base::data_types::Cursor,
501+
) -> Result<(), LocalNodeError> {
502+
Ok(self
503+
.node
504+
.state
505+
.settle_unavailable_bundle(chain_id, origin, cursor)
506+
.await?)
507+
}
508+
494509
/// Gets received certificate trackers.
495510
pub async fn get_received_certificate_trackers(
496511
&self,

linera-core/src/unit_tests/client_tests.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4573,6 +4573,102 @@ where
45734573
Ok(())
45744574
}
45754575

4576+
/// A proposal consuming a `CheckpointAck` bundle that some validator can never hold
4577+
/// must be retried without it. Validator 0 misses the recipient's ack-sending
4578+
/// checkpoint and is later bootstrapped from the recipient's *next* checkpoint, so the
4579+
/// ack block stays below its tip: pushing certificates cannot heal it, and it rejects
4580+
/// every ack-consuming proposal with `MissingCrossChainUpdate`. With a second validator
4581+
/// offline, such a proposal cannot reach quorum — the client must drop the skippable
4582+
/// ack bundle and move on instead of wedging.
4583+
#[test_case(MemoryStorageBuilder::default(); "memory")]
4584+
#[cfg_attr(feature = "rocksdb", test_case(RocksDbStorageBuilder::new().await; "rocks_db"))]
4585+
#[test_log::test(tokio::test)]
4586+
async fn test_unavailable_checkpoint_ack_is_skipped<B>(storage_builder: B) -> anyhow::Result<()>
4587+
where
4588+
B: StorageBuilder,
4589+
{
4590+
let signer = InMemorySigner::new(None);
4591+
let mut builder = TestBuilder::new(storage_builder, 4, 0, signer).await?;
4592+
// Validator 0 sees nothing of what follows: not the transfer, and crucially not the
4593+
// recipient's checkpoints.
4594+
builder.set_fault_type([0], FaultType::NoChains);
4595+
let producer = builder.add_root_chain(1, Amount::from_tokens(7)).await?;
4596+
let recipient = builder.add_root_chain(2, Amount::ZERO).await?;
4597+
let recipient_id = recipient.chain_id();
4598+
4599+
// The recipient consumes a transfer and checkpoints, acknowledging it; then burns
4600+
// and checkpoints again, so its latest checkpoint sits above the ack-sending block.
4601+
producer
4602+
.transfer_to_account(
4603+
AccountOwner::CHAIN,
4604+
Amount::from_tokens(2),
4605+
Account::chain(recipient_id),
4606+
)
4607+
.await
4608+
.unwrap_ok_committed();
4609+
recipient.synchronize_from_validators().await?;
4610+
recipient.process_inbox().await?;
4611+
recipient.checkpoint().await.unwrap().unwrap();
4612+
recipient
4613+
.burn(AccountOwner::CHAIN, Amount::ONE)
4614+
.await
4615+
.unwrap_ok_committed();
4616+
recipient.checkpoint().await.unwrap().unwrap();
4617+
4618+
// Validator 0 comes back and validator 1 goes offline. The recipient's next block
4619+
// bootstraps validator 0 from the latest checkpoint — past the ack-sending block,
4620+
// which nothing can deliver to it anymore.
4621+
builder.set_fault_type([0], FaultType::Honest);
4622+
builder.set_fault_type([1], FaultType::Offline);
4623+
recipient
4624+
.burn(AccountOwner::CHAIN, Amount::ONE)
4625+
.await
4626+
.unwrap_ok_committed();
4627+
4628+
// The producer's inbox holds only the unavailable ack. Consuming it can never be
4629+
// certified (validator 0 rejects it, validator 1 is offline), so the client skips
4630+
// the bundle; with nothing else to include, no block is proposed at all.
4631+
producer.synchronize_from_validators().await?;
4632+
let (certificates, _) = producer.process_inbox().await?;
4633+
assert!(
4634+
certificates.is_empty(),
4635+
"the unconsumable ack must be skipped, not wedge the client",
4636+
);
4637+
4638+
// The skip is durable: the ack was settled in the local inbox, so a fresh
4639+
// `process_inbox` — e.g. after a client restart — has nothing to retry.
4640+
{
4641+
let chain = producer
4642+
.client
4643+
.local_node
4644+
.chain_state_view(producer.chain_id())
4645+
.await?;
4646+
let inbox = chain
4647+
.inboxes
4648+
.try_load_entry(&recipient_id)
4649+
.await?
4650+
.expect("the producer has an inbox for the recipient");
4651+
assert_eq!(
4652+
inbox.added_bundles.count(),
4653+
0,
4654+
"the settled ack must be gone from the local inbox",
4655+
);
4656+
}
4657+
4658+
// The chain stays fully usable: a new transfer commits — without the ack.
4659+
let certificate = producer
4660+
.transfer_to_account(
4661+
AccountOwner::CHAIN,
4662+
Amount::ONE,
4663+
Account::chain(recipient_id),
4664+
)
4665+
.await
4666+
.unwrap_ok_committed();
4667+
assert!(!certificate.block().consumes_checkpoint_ack());
4668+
4669+
Ok(())
4670+
}
4671+
45764672
/// Verifies the push side of the checkpoint flow: a validator that missed the whole
45774673
/// chain is brought up to speed by the proposing client pushing only the latest
45784674
/// checkpoint plus the pre-checkpoint sender blocks it certifies, not every

linera-core/src/worker.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use futures::{
1717
use linera_base::{
1818
crypto::{CryptoError, CryptoHash, ValidatorPublicKey},
1919
data_types::{
20-
ApplicationDescription, ArithmeticError, Blob, BlockHeight, Epoch, Round, TimeDelta,
21-
Timestamp,
20+
ApplicationDescription, ArithmeticError, Blob, BlockHeight, Cursor, Epoch, Round,
21+
TimeDelta, Timestamp,
2222
},
2323
doc_scalar,
2424
identifiers::{AccountOwner, ApplicationId, BlobId, ChainId, EventId, StreamId},
@@ -1577,6 +1577,20 @@ where
15771577
.await
15781578
}
15791579

1580+
/// Removes an incoming bundle of `chain_id` that can never be consumed by its
1581+
/// blocks, and marks the lane from `origin` as settled up to it. See
1582+
/// [`ChainWorkerState::settle_unavailable_bundle`].
1583+
pub async fn settle_unavailable_bundle(
1584+
&self,
1585+
chain_id: ChainId,
1586+
origin: ChainId,
1587+
cursor: Cursor,
1588+
) -> Result<(), WorkerError> {
1589+
let state = self.get_or_create_chain_worker(chain_id).await?;
1590+
let mut guard = handle::write_lock(&state).await?;
1591+
guard.settle_unavailable_bundle(origin, cursor).await
1592+
}
1593+
15801594
/// Test helper that runs `ChainWorkerState::reset_and_reexecute_chain` for the given
15811595
/// chain (the same routine the corruption-recovery path invokes).
15821596
#[cfg(with_testing)]

0 commit comments

Comments
 (0)