Fix authenticated signer when re-proposing a fast block. (#3920)

## Motivation

When re-proposing a validated block from an earlier round there is a
validated block certificate, signed by a quorum of validators, attesting
the correctness of the outcome and the validity of the oracle
responses—_and_ that the `authenticated_signer` is actually the one who
originally submitted the block proposal to the validators!

This last part I missed in the special case where we re-propose an
earlier proposal by a super owner from the fast round! In that case,
there is no validated block certificate, which is why we disallow
oracles. But we erroneously compare the `authenticated_signer` to the
owner _re-proposing_ the block, rather than the original super owner.

Of course comparing it to the super owner is not enough: The regular
owner must not be able to do something in the super owner's name without
permission. So we need to also verify the super owner's signature (and
super ownership!) again.

Super owners by design take greater responsibility for a chain's
liveness than regular owners, and in the case of re-proposing a _fast_
block, the super owner's signature needs to play a similar role to the
validators' signatures when re-proposing a _regular_ block.

## Proposal

Properly distinguish the _three_ cases of a block proposal:
* The current proposer is the one who originally created this block.
They are the authenticated signer.
* This is a retry of an earlier proposal by a super owner in the _fast_
round: The super owner is the authenticated signer, but their signature
must be included in the proposal and verified again.
* This is a retry of an earlier proposal in regular round: The original
proposer is the authenticated signer, but we don't need to verify their
signature again; the validators' signatures of the earlier round's
certificate already proves that the proposal is valid (in addition to
proving that the included oracle responses are, too).

## Test Plan

`test_re_propose_fast_block` was added.

## 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)

Author: afck

linera-chain/src/data_types.rs

@@ -204,6 +204,21 @@ pub struct MessageBundle {
     pub messages: Vec<PostedMessage>,
 }
 
+#[derive(Clone, Debug, Serialize, Deserialize)]
+#[cfg_attr(with_testing, derive(Eq, PartialEq))]
+/// An earlier proposal that is being retried.
+pub enum OriginalProposal {
+    /// A proposal in the fast round.
+    Fast {
+        public_key: AccountPublicKey,
+        signature: AccountSignature,
+    },
+    /// A validated block certificate from an earlier round.
+    Regular {
+        certificate: LiteCertificate<'static>,
+    },
+}
+
 /// An authenticated proposal for a new block.
 // TODO(#456): the signature of the block owner is currently lost but it would be useful
 // to have it for auditing purposes.
@@ -214,7 +229,7 @@ pub struct BlockProposal {
     pub public_key: AccountPublicKey,
     pub signature: AccountSignature,
     #[debug(skip_if = Option::is_none)]
-    pub validated_block_certificate: Option<LiteCertificate<'static>>,
+    pub original_proposal: Option<OriginalProposal>,
 }
 
 /// A message together with kind, authentication and grant information.
@@ -502,17 +517,42 @@ impl BlockProposal {
             content,
             public_key,
             signature,
-            validated_block_certificate: None,
+            original_proposal: None,
+        })
+    }
+
+    pub async fn new_retry_fast(
+        owner: AccountOwner,
+        round: Round,
+        old_proposal: BlockProposal,
+        signer: &(impl Signer + ?Sized),
+    ) -> Result<Self, Box<dyn Error>> {
+        let content = ProposalContent {
+            round,
+            block: old_proposal.content.block,
+            outcome: None,
+        };
+        let signature = signer.sign(&owner, &CryptoHash::new(&content)).await?;
+        let public_key = signer.get_public_key(&owner).await?;
+
+        Ok(Self {
+            content,
+            public_key,
+            signature,
+            original_proposal: Some(OriginalProposal::Fast {
+                public_key: old_proposal.public_key,
+                signature: old_proposal.signature,
+            }),
         })
     }
 
-    pub async fn new_retry(
+    pub async fn new_retry_regular(
         owner: AccountOwner,
         round: Round,
         validated_block_certificate: ValidatedBlockCertificate,
         signer: &(impl Signer + ?Sized),
     ) -> Result<Self, Box<dyn Error>> {
-        let lite_cert = validated_block_certificate.lite_certificate().cloned();
+        let certificate = validated_block_certificate.lite_certificate().cloned();
         let block = validated_block_certificate.into_inner().into_inner();
         let (block, outcome) = block.into_proposal();
         let content = ProposalContent {
@@ -527,7 +567,7 @@ impl BlockProposal {
             content,
             public_key,
             signature,
-            validated_block_certificate: Some(lite_cert),
+            original_proposal: Some(OriginalProposal::Regular { certificate }),
         })
     }
 
@@ -558,17 +598,19 @@ impl BlockProposal {
     /// Checks that the public key matches the owner and that the optional certificate matches
     /// the outcome.
     pub fn check_invariants(&self) -> Result<(), &'static str> {
-        match (&self.validated_block_certificate, &self.content.outcome) {
-            (None, None) => {}
-            (None, Some(_)) | (Some(_), None) => {
+        match (&self.original_proposal, &self.content.outcome) {
+            (None, None) | (Some(OriginalProposal::Fast { .. }), None) => {}
+            (None, Some(_))
+            | (Some(OriginalProposal::Fast { .. }), Some(_))
+            | (Some(OriginalProposal::Regular { .. }), None) => {
                 return Err("Must contain a validation certificate if and only if \
                      it contains the execution outcome from a previous round");
             }
-            (Some(lite_certificate), Some(outcome)) => {
+            (Some(OriginalProposal::Regular { certificate }), Some(outcome)) => {
                 let block = outcome.clone().with(self.content.block.clone());
                 let value = ValidatedBlock::new(block);
                 ensure!(
-                    lite_certificate.check_value(&value),
+                    certificate.check_value(&value),
                     "Lite certificate must match the given block and execution outcome"
                 );
             }

linera-chain/src/manager.rs

@@ -92,7 +92,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::{
     block::{Block, ConfirmedBlock, Timeout, ValidatedBlock},
-    data_types::{BlockProposal, LiteVote, ProposedBlock, Vote},
+    data_types::{BlockProposal, LiteVote, OriginalProposal, ProposedBlock, Vote},
     types::{TimeoutCertificate, ValidatedBlockCertificate},
     ChainError,
 };
@@ -343,10 +343,13 @@ where
         // if there is a validated block certificate from a later round.
         if let Some(vote) = self.confirmed_vote() {
             ensure!(
-                if let Some(validated_cert) = proposal.validated_block_certificate.as_ref() {
-                    vote.round <= validated_cert.round
-                } else {
-                    vote.round.is_fast() && vote.value().matches_proposed_block(new_block)
+                match proposal.original_proposal.as_ref() {
+                    None => false,
+                    Some(OriginalProposal::Regular { certificate }) =>
+                        vote.round <= certificate.round,
+                    Some(OriginalProposal::Fast { .. }) => {
+                        vote.round.is_fast() && vote.value().matches_proposed_block(new_block)
+                    }
                 },
                 ChainError::HasIncompatibleConfirmedVote(new_block.height, vote.round)
             );
@@ -451,22 +454,44 @@ where
     ) -> Result<Option<ValidatedOrConfirmedVote>, ChainError> {
         let round = proposal.content.round;
 
-        // If the validated block certificate is more recent, update our locking block.
-        if let Some(lite_cert) = &proposal.validated_block_certificate {
-            if self
-                .locking_block
-                .get()
-                .as_ref()
-                .is_none_or(|locking| locking.round() < lite_cert.round)
-            {
-                let value = ValidatedBlock::new(block.clone());
-                if let Some(certificate) = lite_cert.clone().with_value(value) {
-                    self.update_locking(LockingBlock::Regular(certificate), blobs.clone())?;
+        match &proposal.original_proposal {
+            // If the validated block certificate is more recent, update our locking block.
+            Some(OriginalProposal::Regular { certificate }) => {
+                if self
+                    .locking_block
+                    .get()
+                    .as_ref()
+                    .is_none_or(|locking| locking.round() < certificate.round)
+                {
+                    let value = ValidatedBlock::new(block.clone());
+                    if let Some(certificate) = certificate.clone().with_value(value) {
+                        self.update_locking(LockingBlock::Regular(certificate), blobs.clone())?;
+                    }
+                }
+            }
+            // If this contains a proposal from the fast round, we consider that a locking block.
+            // It is useful for clients synchronizing with us, so they can re-propose it.
+            Some(OriginalProposal::Fast {
+                public_key,
+                signature,
+            }) => {
+                if self.locking_block.get().is_none() {
+                    let original_proposal = BlockProposal {
+                        public_key: *public_key,
+                        signature: *signature,
+                        ..proposal.clone()
+                    };
+                    self.update_locking(LockingBlock::Fast(original_proposal), blobs.clone())?;
+                }
+            }
+            // If this proposal itself is from the fast round, it is also a locking block: We
+            // will vote to confirm it, so it is locked.
+            None => {
+                if round.is_fast() && self.locking_block.get().is_none() {
+                    // The fast block also counts as locking.
+                    self.update_locking(LockingBlock::Fast(proposal.clone()), blobs.clone())?;
                 }
             }
-        } else if round.is_fast() && self.locking_block.get().is_none() {
-            // The fast block also counts as locking.
-            self.update_locking(LockingBlock::Fast(proposal.clone()), blobs.clone())?;
         }
 
         // We record the proposed block, in case it affects the current round number.

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

@@ -13,7 +13,9 @@ use linera_base::{
     identifiers::{AccountOwner, ChainId},
 };
 use linera_chain::{
-    data_types::{BlockExecutionOutcome, BlockProposal, MessageBundle, ProposalContent},
+    data_types::{
+        BlockExecutionOutcome, BlockProposal, MessageBundle, OriginalProposal, ProposalContent,
+    },
     manager,
     types::{ConfirmedBlockCertificate, TimeoutCertificate, ValidatedBlockCertificate},
     ChainExecutionContext, ChainStateView, ExecutionResultExt as _,
@@ -130,7 +132,7 @@ where
                     outcome: _,
                 },
             public_key,
-            validated_block_certificate,
+            original_proposal,
             signature: _,
         } = proposal;
 
@@ -146,11 +148,12 @@ where
                 // TODO(#3203): Allow multiple pending proposals on permissionless chains.
                 chain.pending_proposed_blobs.clear();
             }
+            let validated = matches!(original_proposal, Some(OriginalProposal::Regular { .. }));
             chain
                 .pending_proposed_blobs
                 .try_load_entry_mut(&owner)
                 .await?
-                .update(*round, validated_block_certificate.is_some(), maybe_blobs)
+                .update(*round, validated, maybe_blobs)
                 .await?;
             self.save().await?;
             return Err(WorkerError::BlobsNotFound(missing_blob_ids));

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

@@ -4,14 +4,14 @@
 //! Operations that don't persist any changes to the chain state.
 
 use linera_base::{
-    data_types::{ApplicationDescription, ArithmeticError, Blob, Timestamp},
+    data_types::{ApplicationDescription, ArithmeticError, Blob, Round, Timestamp},
     ensure,
     identifiers::{AccountOwner, ApplicationId},
 };
 use linera_chain::{
     data_types::{
-        BlockExecutionOutcome, BlockProposal, IncomingBundle, MessageAction, ProposalContent,
-        ProposedBlock,
+        BlockExecutionOutcome, BlockProposal, IncomingBundle, MessageAction, OriginalProposal,
+        ProposalContent, ProposedBlock,
     },
     manager,
     types::Block,
@@ -161,7 +161,7 @@ where
         let BlockProposal {
             content,
             public_key,
-            validated_block_certificate,
+            original_proposal,
             signature: _,
         } = proposal;
         let block = &content.block;
@@ -178,12 +178,47 @@ where
             chain.manager.verify_owner(proposal),
             WorkerError::InvalidOwner
         );
-        if let Some(lite_certificate) = validated_block_certificate {
-            // Verify that this block has been validated by a quorum before.
-            lite_certificate.check(committee)?;
-        } else if let Some(signer) = block.authenticated_signer {
-            // Check the authentication of the operations in the new block.
-            ensure!(signer == owner, WorkerError::InvalidSigner(signer));
+        match original_proposal {
+            None => {
+                if let Some(signer) = block.authenticated_signer {
+                    // Check the authentication of the operations in the new block.
+                    ensure!(signer == owner, WorkerError::InvalidSigner(owner));
+                }
+            }
+            Some(OriginalProposal::Regular { certificate }) => {
+                // Verify that this block has been validated by a quorum before.
+                certificate.check(committee)?;
+            }
+            Some(OriginalProposal::Fast {
+                public_key,
+                signature,
+            }) => {
+                let super_owner = AccountOwner::from(*public_key);
+                ensure!(
+                    chain
+                        .manager
+                        .ownership
+                        .get()
+                        .super_owners
+                        .contains(&super_owner),
+                    WorkerError::InvalidOwner
+                );
+                if let Some(signer) = block.authenticated_signer {
+                    // Check the authentication of the operations in the new block.
+                    ensure!(signer == super_owner, WorkerError::InvalidSigner(signer));
+                }
+                let old_proposal = BlockProposal {
+                    content: ProposalContent {
+                        block: proposal.content.block.clone(),
+                        round: Round::Fast,
+                        outcome: None,
+                    },
+                    public_key: *public_key,
+                    signature: *signature,
+                    original_proposal: None,
+                };
+                old_proposal.check_signature()?;
+            }
         }
         // Check if the chain is ready for this new block proposal.
         chain.tip_state.get().verify_block_chaining(block)?;

linera-core/src/client/mod.rs

@@ -2744,12 +2744,12 @@ impl<Env: Environment> ChainClient<Env> {
         let proposal = if let Some(locking) = info.manager.requested_locking {
             Box::new(match *locking {
                 LockingBlock::Regular(cert) => {
-                    BlockProposal::new_retry(owner, round, cert, self.signer())
+                    BlockProposal::new_retry_regular(owner, round, cert, self.signer())
                         .await
                         .map_err(ChainClientError::signer_failure)?
                 }
                 LockingBlock::Fast(proposal) => {
-                    BlockProposal::new_initial(owner, round, proposal.content.block, self.signer())
+                    BlockProposal::new_retry_fast(owner, round, proposal, self.signer())
                         .await
                         .map_err(ChainClientError::signer_failure)?
                 }