block component: verify and ingest finalization certs from footer #693

AshwinSekar · 2026-01-22T02:29:24Z

Problem

We need to verify the slow / fast finalization certificates from the block footer and ingest it into our consensus pool.
Ingestion here is key for unstaked nodes.

Summary of Changes

rename HighestFinalizedSlotCert -> BlockFinalization since this is used both in the consensus pool to track highest finalized block and for extraction of the footer
removed the Arc from BlockFinalization, copying should be cheap enough ~ once every 400ms
refactor some functions to allow for code reuse in verification w/ the genesis certificate
reorganize the limit map so it's accessible by block component processor
send the verified certs to consensus pool

AshwinSekar · 2026-01-22T02:31:38Z

runtime/src/block_component_processor.rs

        bank: Arc<Bank>,
        parent_bank: Arc<Bank>,
        marker: VersionedBlockMarker,
+        finalization_cert_sender: Option<&Sender<ConsensusMessage>>,


this is optional (similar to replay_vote_sender in TowerBFT) because we don't need it for startup blocks

AshwinSekar · 2026-01-22T02:38:16Z

votor-messages/src/consensus_message.rs

+    /// `VoteType`s that contribute to this certificate.
+    ///
+    /// Must be in sync with `Vote::to_cert_types`
+    pub const fn limits_and_vote_types(&self) -> (Fraction, &'static [VoteType]) {


just moved these over from votor::common to here

AshwinSekar · 2026-01-22T02:38:50Z

votor-messages/src/consensus_message.rs

-pub enum HighestFinalizedSlotCert {
-    /// Standard finalization: requires both a Finalize cert and a Notarize cert for the same slot
+#[allow(clippy::large_enum_variant)]
+pub enum BlockFinalization {


open to naming suggestions

I might be tempted to to add a Cert suffix otherwise this is definitely an improvement.

akhi3030

Generally looks good. Minor comments really. It would've been easier to review if the PR was done in parts e.g. removal of Arc and renaming, etc. being done separately.

akhi3030 · 2026-01-23T13:39:15Z

entry/src/block_component.rs

+
+    /// Converts the block marker representation of a finalization certificate into verification friendly
+    /// representation of Slow or Fast finalization certificates
+    pub fn to_certificates(self) -> Result<BlockFinalization, BlsError> {


if this fails, we will get a generic bls error that will not explain which verification failed. Might make things hard to debug from logs.

akhi3030 · 2026-01-23T13:44:51Z

runtime/src/block_component_processor.rs

+
+        let final_certs = final_cert
+            .to_certificates()
+            .map_err(|_| BlockComponentProcessorError::InvalidFinalizationCertificate)?;


if we return a more informed error here then this might have to be updated. I think just printing out the error in the warn would suffice.

akhi3030 · 2026-01-23T13:45:37Z

entry/src/block_component.rs

+
+    /// Converts the block marker representation of a finalization certificate into verification friendly
+    /// representation of Slow or Fast finalization certificates
+    pub fn to_certificates(self) -> Result<BlockFinalization, BlsError> {


nit: since we are consuming self, the function should be called into_....

akhi3030 · 2026-01-23T13:50:18Z

runtime/src/block_component_processor.rs

+            .to_certificates()
+            .map_err(|_| BlockComponentProcessorError::InvalidFinalizationCertificate)?;
+
+        match &final_certs {


nit: I wonder if this verification can be moved into to_certificates() so that we cannot even construct BlockFinalization unless all the checks pass and then we do not have to worry about whether or not such checks were performed when we are using a object of type BlockFinalization. Bonus points for using a new type e.g. ValidatedBlockFinalization 😄

akhi3030 · 2026-01-23T13:53:43Z

runtime/src/block_component_processor.rs

+                        finalize_cert,
+                        notarize_cert,
+                    } => {
+                        let _ = sender.send(ConsensusMessage::from(notarize_cert));


should we print a warn if this fails to help with debugging? If someone changes the unbounded or the receiver is disconnected, we would at least capture some logs.

akhi3030 · 2026-01-23T13:55:38Z

votor-messages/src/consensus_message.rs

-pub enum HighestFinalizedSlotCert {
-    /// Standard finalization: requires both a Finalize cert and a Notarize cert for the same slot
+#[allow(clippy::large_enum_variant)]
+pub enum BlockFinalization {


I might be tempted to to add a Cert suffix otherwise this is definitely an improvement.

akhi3030 · 2026-01-23T13:56:12Z

votor-messages/src/consensus_message.rs

    pub fn slot(&self) -> Slot {
        match self {
-            HighestFinalizedSlotCert::Finalize {
+            BlockFinalization::Finalize {


Suggested change

BlockFinalization::Finalize {

Self::Finalize {

So we do not have to update this in case of renaming.

AshwinSekar · 2026-01-23T19:00:34Z

I wonder if this verification can be moved into to_certificates() so that we cannot even construct BlockFinalization unless all the checks pass and then we do not have to worry about whether or not such checks were performed when we are using a object of type BlockFinalization. Bonus points for using a new type e.g. ValidatedBlockFinalization 😄

Excellent suggestion, this makes the code a lot easier to look at and lets me encapsulate all the deserialize and verification logic.

Should be good for another look, and apologies for how long this PR is, it got out of hand as I was writing it.

akhi3030

thanks, looks much better. Just a couple of small comments now.

akhi3030 · 2026-02-02T11:51:00Z

runtime/src/validated_block_finalization.rs

+/// This type can only be constructed through validation methods, providing
+/// compile-time guarantees that the contained certificates are valid.


Unfortunately, since this is an enum, this can be constructed directly. E.g. the compiler will let us just do let foo = ValidatedBlockFinalizationCert::FastFinalize(...);.

The way to get this guarantee is to turn this into a struct. Maybe we can define it as:

struct ValidatedBlockFinalizationCert { finalize_cert: Certificate, notar_cert: Option<Certificate>, }

this is a really good point, I do like keeping the enum instead of having the option. I hid it in an inner type here c903a11 seems like an idiomatic approach

akhi3030 · 2026-02-02T12:06:33Z

runtime/src/validated_block_finalization.rs

+        }
+    }
+
+    fn verify_certificate(


could you add some doc comments to explain what is being returned in the u64s please?

done 7e0b411

akhi3030 · 2026-02-02T12:07:30Z

runtime/src/validated_block_finalization.rs

+    /// - BLS signature conversion fails
+    /// - Certificate signature verification fails
+    /// - The certificates don't meet the required stake thresholds
+    pub fn try_from_footer(


It is fine to defer it for now but we should add some unit tests to ensure that this function does fail for invalid certs.

ah I did have some basic unit tests but forgot to move them over, done here 7acac02

akhi3030 · 2026-02-02T12:09:08Z

runtime/src/validated_block_finalization.rs

+                        "Received a slow finalization in the footer for slot {slot} block_id \
+                         {block_id:?} in bank slot {} with notarize {notarize_percent} stake \
+                         expecting at least {notarize_threshold}",
+                        bank.slot()


should we take the slot from the footer instead of the bank here? Given the call tree, they should match up but it isn't clear in this function that that will always be the case.

the footer itself doesn't have a slot.
Here we log both the finalization cert slot and the bank slot.

akhi3030 · 2026-02-02T12:13:57Z

core/src/block_creation_loop.rs


    BlockComponentProcessor::update_bank_with_footer(
-        working_bank.bank.clone_without_scheduler(),
+        &working_bank.bank.clone_without_scheduler(),


nit: we are taking a reference to a cloned object. Could this just be &working_bank.bank?

good catch 1c4c67d

akhi3030

LGTM

AshwinSekar force-pushed the finalization-footer branch 2 times, most recently from a1a080a to 9f437fb Compare January 22, 2026 02:36

AshwinSekar marked this pull request as draft January 22, 2026 02:44

AshwinSekar commented Jan 22, 2026

View reviewed changes

AshwinSekar requested review from akhi3030 and ksn6 January 22, 2026 13:41

AshwinSekar marked this pull request as ready for review January 22, 2026 13:41

akhi3030 reviewed Jan 23, 2026

View reviewed changes

AshwinSekar force-pushed the finalization-footer branch from 9f437fb to 07357a2 Compare January 23, 2026 18:04

AshwinSekar marked this pull request as draft January 23, 2026 18:04

AshwinSekar force-pushed the finalization-footer branch 3 times, most recently from 656763a to 2a2de5e Compare January 23, 2026 18:27

AshwinSekar marked this pull request as ready for review January 23, 2026 20:37

AshwinSekar force-pushed the finalization-footer branch from 2a2de5e to ed10697 Compare January 28, 2026 15:53

AshwinSekar added 2 commits February 2, 2026 01:18

block component: verify and ingest finalization certs from footer

74e0861

pr feedback: ValidatedBlockFinalizationCert

cac24be

AshwinSekar force-pushed the finalization-footer branch from ed10697 to cac24be Compare February 2, 2026 01:21

akhi3030 reviewed Feb 2, 2026

View reviewed changes

AshwinSekar added 5 commits February 2, 2026 17:30

pr feedback: doc comment

7e0b411

pr feedback: fix ref to clone

1c4c67d

pr feedback: move tests to validated_block_finalization.rs

7acac02

pr feedback: use inner type to hide direct construction

c903a11

reuse slot blockid local vars

4a7f3c9

akhi3030 approved these changes Feb 3, 2026

View reviewed changes

		/// This type can only be constructed through validation methods, providing
		/// compile-time guarantees that the contained certificates are valid.

block component: verify and ingest finalization certs from footer #693

Are you sure you want to change the base?

block component: verify and ingest finalization certs from footer #693

Conversation

AshwinSekar commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akhi3030 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AshwinSekar commented Jan 23, 2026

Uh oh!

akhi3030 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akhi3030 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AshwinSekar commented Jan 22, 2026 •

edited

Loading