feat(node): verifying cometbft precommit signatures #1402

cryptoAtwill · 2025-07-22T15:08:03Z

This PR adds light client commitment to fendermint app state.

At the end of each block , the old checkpoint process is replaced with generating light client commitments instead of old bottom up checkpoints. When commit from abci is called, it will collect the light client commitments (if any) to derive a app hash that includes the old state param with new light client commitments.

This new app hash will be forwards to solidity light client contract to validate the cometbft precommit signatures quorum cert.

Then replayer will then collect all the commitments to submit the cometbft header including the app hash.

Key changes:
Contracts:

Added SubnetActorCheckpointFacet contract to validate cometbft signed header
Added app hash validation logic to SubnetActorCheckpointFacet and side effects execution.
Removed BottomUpCheckpoint and updated tests
Fendermint:
Removed signature submission for bottom up checkpoint
Updated end block logic in interpreter to generate light client commitments.
Relayer:
Replaced old bottom up checkpoint process to query cometbft signed header and submit side effects for execution.

Current submission flow:

In abci end block, the commitment(if any) will be generate at each bottom up checkpoint period:

ipc/fendermint/vm/interpreter/src/fvm/interpreter.rs

Line 402 in 0e2a137

let maybe_result = self
relayer picks up if there are any commitment to be submitted:

ipc/ipc/provider/src/checkpoint.rs

Line 131 in 0e2a137

if let Err(e) = self.commit_next_validator_changes(submitter, height).await {
- cometbft signed headers are queried from:
  
  ipc/ipc/provider/src/manager/cometbft/mod.rs
  
  Line 21 in 0e2a137
  
  pub async fn fetch_signed_header(&self, height: ChainEpoch) -> anyhow::Result<SignedHeader> {
gateway contract validates cometbft signed headers:

ipc/contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

Line 32 in 0e2a137

function submitSignedHeader(bytes calldata rawData) external whenNotPaused {
Side effects execution through:
- ipc/contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol
  
  Line 49 in 0e2a137
  
  function recordActivityRollup(
- ipc/contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol
  
  Line 61 in 0e2a137
  
  function confirmValidatorChange(
- ipc/contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol
  
  Line 77 in 0e2a137
  
  function execBottomUpMsgBatch(

Note

Replaces bottom-up checkpoints with CometBFT light client commitments across contracts, node, and relayer, updating app hash derivation and submission/exec flows.

Contracts:
- Add SubnetActorCheckpointingFacet to validate CometBFT signed headers and execute side effects; emit StateCommitmentCreated and support batch exec via commitments.
- Remove BottomUpCheckpoint/quorum logic and related getters; update tests and selector gen.
- Wire new Tendermint light-client types (tendermint-sol), add chainID to Subnet params.
Node (Fendermint):
- Replace checkpointing with end-block generation of LightClientCommitments; include in snapshot and app hash (new derive path) and state payloads.
- Remove bottom-up signature flow/managers; add EndBlockManager and commitment plumbing.
Relayer/Provider:
- Query CometBFT SignedHeader (new RPC & types), submit headers to subnet, confirm validator changes, and execute bottom-up batches using commitments.
- Replace old checkpoint/quorum code; add active/waiting validator listing; adjust submission period/last height APIs.
API:
- Add eth_getCommitSignedHeader; various ABI/type conversions updated; remove unused checkpoint APIs.
CLI/Genesis:
- Add chain-id to subnet create; remove old checkpoint commands; make genesis chain_id mandatory and persisted.
Misc:
- Add tendermint-sol submodule and deps; update storage-layouts accordingly.

^{Written by Cursor Bugbot for commit e6900d0. This will update automatically on new commits. Configure here.}

v0.1.0

Co-authored-by: cryptoAtwill <[email protected]>

contracts/contracts/gateway/router/CheckpointingFacet.sol

fendermint/app/src/cmd/genesis.rs

LePremierHomme · 2025-09-05T08:40:54Z

contracts/contracts/gateway/GatewayGetterFacet.sol

        return s.networkName;
    }

-    /// @notice Returns a specific bottom-up checkpoint based on an epoch number.


Is there any reason to keep all code related to the checkpoints quorum?

I missed them in deleting code, will delete one more round.

contracts/contracts/gateway/router/CheckpointingFacet.sol

contracts/contracts/lib/LibPower.sol

LePremierHomme · 2025-09-05T09:59:33Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

-    /// @param checkpoint The executed bottom-up checkpoint.
-    /// @param signatories The addresses of validators signing the checkpoint.
-    /// @param signatures The signatures of validators on the checkpoint.
-    function submitCheckpoint(


Can you explain why this was removed?
Conceptually we should still have the notion of checkpoint submission to the parent, as it's a larger process than merely submitting the signing data.

Any reason why to decouple the activity rollup & bottom-up messages commitment from that?

For the second question, it's mainly for gas considerations. The verification of signed header and signatures consumes a lot of gas. Adding more processing to existing process will likely take longer to be mined.

In that question I was referring to the commitment (hash) of the activity rollup & the bottom-up messages.

I have split them into separate functions as it provides max flexibility. If there is no activity required for a subnet, then activity rollup should never be used. If someone wants to merge them in one transaction, they can simply use multi-call pattern.

I'm not sure i'm convinced.. if there was no activity, then it can be marked with a zero-value commitment, or something similar. Otherwise, the state of the checkpoint in that regard is unknown.

In addition, I don't think the current implementation supports multiple calls to execBottomUpMsgBatch for the same checkpoint.

I understand where you are coming from, but I still think there are more flexibility if we split the calls and putting them in one call does not add much more.

I dont consider the state of the checkpoint in that regard is unknown. to be true, because the checkpoint is the signed header submission, which is already handled. These are just executing side effects, merging them in one method does not change the face that the checkpoint is committed. And if no one cares about the activity, then no one needs to do anything about it. Putting them together forces these two to be handled simultaneously.

Eventually we will move to supporting single bottom up message batch execution, then we will have to break the methods into two.

LePremierHomme · 2025-09-05T10:03:42Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

-        // validate signatures and quorum threshold, revert if validation fails
-        validateActiveQuorumSignatures({signatories: signatories, hash: checkpointHash, signatures: signatures});
+    // TODO: The parameter should be SignedHeader.Data, but ethers-rust failed to generate the bindings
+    function submitSignedHeader(bytes calldata rawData) external whenNotPaused {


This is a cometbft-specific implementation. We need to introduce polymorphism, where the variant of the verifying client is declared for the given subnet.

I'm thinking the polymorphism should be from the Facet. I think that might be a better way to keep things simpler. Currently we dont have other checkpoint mechanism, I dont really see a point of polymorphism at the moment to create something that does not exist.

The IPC roadmap is to allow configurable consensus mechanisms, so we should cater for it, even if it doesn't exist yet.

Making the Facet as the configurable component is fine, but it currently carries other, non-related functionality.

I suggest to see the checkpoint mechanism as fixed, decoupled from a configurable verifying light client component.

Updated, the naming has been reverted.

I wasn't referring to the naming here.

LePremierHomme · 2025-09-05T10:54:12Z

contracts/contracts/lib/cometbft/CometbftLightClient.sol

+    }
+
+    function ensureValidatorSubmission(uint256 i, bytes memory incomingValidator) internal view returns (uint256 power, address validator) {
+        validator = LibPower.getActiveValidatorAddressByIndex(uint16(i));


Can you explain how the signature index correlates with the validator priority position?

I will add doc to this. TLDR, they must match.

You added a comment that it should match, which is fine for the contract side. But you didn't explain what exactly is making sure it is matching, given that the list is fetched from CometBFT, without being re-ordered prior to calling submitBottomUpCheckpoint.

Simply put, the validator orders are checked before SignedHeader submission, then the validator signatures are sorted against that order, see

ipc/ipc/provider/src/checkpoint.rs

Line 204 in c1b5842

header.order_commit_against(pubkeys)?;

Ok, I now see the code in the off-chain relayer. Please add some docs there explaining why this ordering is taking place.

contracts/contracts/lib/cometbft/CometbftLightClient.sol

LePremierHomme · 2025-09-05T11:02:42Z

contracts/contracts/lib/cometbft/CometbftLightClient.sol

+        string memory _chainID,
+        uint256 idx
+    ) internal pure returns (bytes memory) {
+        CanonicalVote.Data memory vote = CanonicalVote.Data({


What prevents the signed header to contain an arbitrary app_hash?
It doesn't seems to be used here.

it cannot be arbitrary app_hash because checkCommitHash checks it before reaching this step

It's not clear to me what exactly checkCommitHash is checking, and how that relates specifically to app_hash. Please provide more docs.

It still not clear what exactly checkCommitHash is checking.

What header.hash() is including?

You wrote "This also makes sure the AppHash is not fabricated" without explaining how it's done.

function hash(SignedHeader.Data memory h) internal pure returns (bytes32) { require(h.header.validators_hash.length > 0, "Tendermint: hash can't be empty"); bytes memory hbz = Consensus.encode(h.header.version); bytes memory pbt = Timestamp.encode(h.header.time); bytes memory bzbi = BlockID.encode(h.header.last_block_id); bytes[14] memory all = [ hbz, Encoder.cdcEncode(h.header.chain_id), Encoder.cdcEncode(h.header.height), pbt, bzbi, Encoder.cdcEncode(h.header.last_commit_hash), Encoder.cdcEncode(h.header.data_hash), Encoder.cdcEncode(h.header.validators_hash), Encoder.cdcEncode(h.header.next_validators_hash), Encoder.cdcEncode(h.header.consensus_hash), Encoder.cdcEncode(h.header.app_hash), Encoder.cdcEncode(h.header.last_results_hash), Encoder.cdcEncode(h.header.evidence_hash), Encoder.cdcEncode(h.header.proposer_address) ]; return MerkleTree.merkleRootHash(all, 0, all.length); }

This is what the hash function does. You can see the app_hash is part of the checking mechanism.

…nsus-shipyard/ipc into cometbft-precommit-signatures

ipc/provider/src/manager/cometbft/types.rs

LePremierHomme · 2025-09-30T11:48:23Z

contracts/contracts/lib/cometbft/CometbftLightClient.sol

+    }
+
+    function ensureValidatorSubmission(uint256 i, bytes memory incomingValidator) internal view returns (uint256 power, address validator) {
+        validator = LibPower.getActiveValidatorAddressByIndex(uint16(i));


Ok, I now see the code in the off-chain relayer. Please add some docs there explaining why this ordering is taking place.

LePremierHomme · 2025-09-30T11:56:39Z

contracts/contracts/lib/cometbft/CometbftLightClient.sol

+        validator = LibPower.getActiveValidatorAddressByIndex(uint16(i));
+        ValidatorInfo memory info = LibPower.getActiveValidatorInfo(validator);
+
+        bytes20 expectedCometbftAccountId = toCometBFTAddress(info.metadata);


It's not clear how info.metadata contains the public key. It is documented as data for off-chain retrieval.

contracts/contracts/lib/cometbft/CometbftLightClient.sol

LePremierHomme · 2025-09-30T12:33:59Z

contracts/contracts/lib/cometbft/CometbftLightClient.sol

+            bytes memory message = generateSignePayload(header.commit, LibSubnetActorStorage.appStorage().chainID, i);
+            bytes32 messageHash = sha256(message);
+
+            ensureValidSignature(messageHash, commitSig.signature, validator);


Signature is checked here with the onchain address validator.
If it's actually passing, then CometBFT is using that key for signing.
If so, why ensureValidatorSubmission needs to check CometBFT address? If it doesn't match, the signature check will fail.
In that case, i'm not sure if/why commitSig.validator_address needs to be submitted.

I think removing them should work as well because public key has to match the validator. I will remove this.

LePremierHomme · 2025-09-30T12:48:01Z

contracts/contracts/lib/cometbft/CometbftLightClient.sol

+            commitSig = header.commit.signatures[i];
+            // no need to verify absent or nil votes.
+            if (commitSig.block_id_flag != TENDERMINTLIGHT_PROTO_GLOBAL_ENUMS.BlockIDFlag.BLOCK_ID_FLAG_COMMIT) {
+                continue;


Any reason why not to filter these off-chain?

I think even if offchain is filtered, contract should check it just ensure block id flag is correct. Sanity check, no need to revert, just ignore.

Since it's unsigned data, I don't see why it's need to be submitted. (both as a field, and as items who contain the wrong field value).

And on the same note, I don't think we should follow CometBFT data types when not needed.

LePremierHomme · 2025-10-06T07:49:09Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

-        // confirming the changes in membership in the child
-        LibPower.confirmChange(checkpoint.nextConfigurationNumber);
+        checkpointStorage.stateCommitments[height] = StateCommitment({
+            blockHeight: height,


Why blockHeight is needed in this struct? it's already used as the key.

LePremierHomme · 2025-10-06T08:07:26Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

+        assembly {
+            mstore(add(appHash, 32), derived)
+        }
+        return appHash;


You're trying to produce bytes instead of bytes32 to comply with having the commitment being bytes (taken from header_app_hash), which I don't think is needed.

True, we can simplify the payload. Because behind the scene it's using Tendermint library to do the verification, I would prefer to use the library as it is for now. Unless there is strong gas concerns.

LePremierHomme · 2025-10-06T08:19:15Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

+        SubnetActorCheckpointingStorage storage checkpointStorage = LibCheckpointingStorage.getStorage();

-        uint256 threshold = (activeCollateral * s.majorityPercentage) / 100;
+        validateAppHash(checkpointHeight, breakdown);


What prevents an arbitrary activity data, unrelated to the commitment breakdown?

good catch, I missed this. The keccack should be equal. Updated.

LePremierHomme · 2025-10-06T08:25:31Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol


-        uint256 threshold = (activeCollateral * s.majorityPercentage) / 100;
+        validateAppHash(checkpointHeight, breakdown);
+        ensureValidHeight(checkpointHeight, checkpointStorage.commitmentHeights.activity);


It seems here that this will need to be called for every checkpoint, otherwise will fail, which doesn't follow what you wrote in #1402 (comment).

I mean if there is activity rollup, then we have to enforce it every checkpoint. If there is no activity rollup, then no one needs to call this method.

LePremierHomme · 2025-10-06T08:45:01Z

contracts/contracts/subnet/SubnetActorCheckpointingFacet.sol

+        validateAppHash(checkpointHeight, breakdown);
+
+        LibBottomUpBatch.recordBottomUpBatchCommitment(checkpointHeight, breakdown.msgBatchCommitment);
+        _execBottomUpMsgBatch(checkpointHeight, inclusions);


This won’t allow multi-phase execution, i.e., execution without the full set of inclusion proofs.

Is multi-phase execution possible before this change?

Ok, in the relayer, it's packing all the BottomUpBatch.Inclusion[] and submit. In the contract, it supports submission one by one. To replicate that, we can just add two more methods:

function recordBottomUpMsgBatch( uint64 checkpointHeight, StateCommitmentBreakDown calldata breakdown ) external whenNotPaused { validateAppHash(checkpointHeight, breakdown); LibBottomUpBatch.recordBottomUpBatchCommitment(checkpointHeight, breakdown.msgBatchCommitment); } function execBottomUpMsgBatch( uint64 checkpointHeight, BottomUpBatch.Inclusion[] calldata inclusions ) external whenNotPaused { _execBottomUpMsgBatch(checkpointHeight, inclusions); }

cryptoAtwill added 3 commits July 21, 2025 16:43

forge install: tendermint-sol

7cf3a32

v0.1.0

validate cometbft pre commit

30fd904

unit test

77c7ab4

cryptoAtwill requested a review from a team as a code owner July 22, 2025 15:08

cryptoAtwill and others added 3 commits July 28, 2025 16:46

feat(node): patch replay cometbft (#1405)

bcbac71

Co-authored-by: cryptoAtwill <[email protected]>

fix fmt

2e11100

feat(node): adding subnet light client commitment to app hash (#1414)

7cc7588

Co-authored-by: cryptoAtwill <[email protected]>

This comment was marked as outdated.

Sign in to view

cryptoAtwill and others added 14 commits August 18, 2025 20:03

clippy

022b212

patch bottom up batch

ae2edbb

inclusion

dbf67d3

remove unused code (#1420)

6a97714

Co-authored-by: cryptoAtwill <[email protected]>

clean up further

0c9eb7d

remove unused code

fbdea72

add cometbft url args

1f75339

fix tests

59b3144

resolve conflicts

71de73d

update app hash conversion

0e2a137

fix tests wip

2900ffc

fix contract tests

89af385

lint

15c6dcd

prettier

424403c

This comment was marked as outdated.

Sign in to view

cryptoAtwill added 4 commits August 27, 2025 10:42

remove ununsed code

4209d04

remove unused code

4a2f0b7

clean up methods

ad2d340

merge with main

118b3d7

This comment was marked as outdated.

Sign in to view

cryptoAtwill added 2 commits August 27, 2025 14:22

add relayer cometbft url in materializer

f07b122

update storage

9bf9903

cryptoAtwill added 2 commits August 27, 2025 16:13

update storage layout

c6ee952

fix zero block

230f2ee

LePremierHomme reviewed Aug 27, 2025

View reviewed changes

contracts/contracts/gateway/router/CheckpointingFacet.sol Outdated Show resolved Hide resolved

cryptoAtwill and others added 7 commits August 27, 2025 19:07

fix snapshot

eae3cd9

clippy fmt

a169a54

fix unit test

de3b586

eth node query cometbft

9a2e079

fix wrong rpc endpoint

901916d

fix height to string

527ec7e

Merge branch 'main' into cometbft-precommit-signatures

da6531f

LePremierHomme reviewed Sep 5, 2025

View reviewed changes

cryptoAtwill added 9 commits September 15, 2025 17:45

tidy up

0770bc9

Merge branch 'cometbft-precommit-signatures' of protocol-github:conse…

047118b

…nsus-shipyard/ipc into cometbft-precommit-signatures

update storage

ce124fc

fix chain id

b68f079

fix layer 2 tests

4e9abfa

update deploy stack

4a86350

update app storage access

9382bba

fix wrong chain id

e322238

review comments

c1b5842

LePremierHomme reviewed Sep 30, 2025

View reviewed changes

update review

9f0250e

LePremierHomme reviewed Oct 6, 2025

View reviewed changes

cryptoAtwill added 2 commits October 7, 2025 16:36

fmt

5e30365

fix clippy

e6900d0

feat(node): verifying cometbft precommit signatures #1402

Are you sure you want to change the base?

feat(node): verifying cometbft precommit signatures #1402

Conversation

cryptoAtwill commented Jul 22, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current submission flow:

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

cryptoAtwill commented Jul 22, 2025 •

edited by cursor bot

Loading

cryptoAtwill Oct 6, 2025 •

edited

Loading