Skip to content

feat: remove sequencer batch generation from consensus layer#35

Merged
FletcherMan merged 3 commits intomainfrom
feat/remove-sequencer-batch-gen
May 6, 2026
Merged

feat: remove sequencer batch generation from consensus layer#35
FletcherMan merged 3 commits intomainfrom
feat/remove-sequencer-batch-gen

Conversation

@FletcherMan
Copy link
Copy Markdown
Collaborator

Summary

  • Remove Batcher interface (6 methods), VerifySignature, BlsData, GetBLSDatas, ConsensusData.BatchHash from l2node; L2Node now only exposes block execution methods
  • Delete consensus/batch.go (entire BatchCache implementation), blssignatures/ package, and all BLS private key loading/passing chain
  • Remove decideBatchPoint* logic, batch-phase commit calls (CommitBatch/PackCurrentBlock), BLS accumulation/validation/self-sign paths from consensus/state.go and state/execution.go
  • Remove redundant batch-point validation block from blocksync/reactor.go
  • Remove Go-side BatchParams struct from types/params.go (proto field retained for wire compatibility)
  • Clean up BLS key config from config/config.go and cmd/tendermint/commands/init.go

Wire Compatibility

The following fields are retained per historical block sync requirements:

  • types/block.go: Header.BatchHash, BlockID.BatchHash, Data.L2BatchHeader, Header.IsBatchPoint()
  • types/vote.go: Vote.BLSSignature, ValidateBasic length constraints
  • types/canonical.go: BlockID.BatchHash in canonical bytes
  • All proto field definitions in proto/tendermint/types/types.proto + generated .pb.go

Test Notes

Three pre-existing test fixture failures are not introduced by this PR:

  • consensus/TestConsMsgsVectors
  • blocksync/TestBlockchainMessageVectors
  • evidence/TestEvidenceVectors

These fail due to a golden-byte mismatch caused by BlockID.BatchHash shifting PartSetHeader field number from 2 to 3 in proto encoding — a pre-existing issue confirmed by running the same tests against the base commit.

Upgrade Notes

Mixed-version deployment (some nodes upgraded, some not) carries a liveness risk if a batch-point block is proposed during the window. Recommended upgrade order: upgrade the sequencer node first to stop producing blocks with BatchHash, wait for the last batch to finalize on L1, then upgrade remaining nodes.

Scope Note

Also fixes a pre-existing compile gap in rpc/test/helpers.go and test/e2e/node/main.go where node.NewNode(...) callers were missing the sequencerVerifier/sequencerSigner params added in a prior PR (passing nil, nil).

Related

  • Temporary integration tag: v0.3.5-alpha.1
  • Stable tag to be cut after devnet validation: v0.3.5

Made with Cursor

Remove the Batcher interface and all related BLS/batch logic from the
Tendermint consensus layer per SPEC-004.

Deleted:
- consensus/batch.go: entire BatchCache implementation
- blssignatures/: entire package (BLS key gen, signing, file helpers)

Key changes:
- l2node: remove Batcher interface (6 methods), VerifySignature, BlsData,
  GetBLSDatas, ConsensusData.BatchHash; L2Node now only exposes
  RequestHeight/EncodeTxs/RequestBlockData/CheckBlockData/DeliverBlock
- l2node/mock.go: sync mock to new interface
- consensus/state.go: remove decideBatchPoint*, batchCache field,
  BLS accumulation/validation/self-sign paths, SetBLSPrivKey, blsPrivKey
- consensus/replay.go: sync ExecBlockOnL2Node call (drop dead commit param)
- consensus/wal_generator.go: remove BLS key loading
- state/state.go: remove decideBatchPointFunc type and MakeBlock param
- state/execution.go: remove decideBatchPoint param, commit-phase batch
  calls (CommitBatch/PackCurrentBlock), drop dead commit param from
  ExecBlockOnL2Node
- blocksync/reactor.go: remove redundant batch-point validation block
- types/params.go: remove Go-side BatchParams struct (proto field retained
  for wire compatibility)
- node/node.go: remove blsPrivKey field, param, and DefaultNewNode loading
- cmd/tendermint/commands/init.go: remove BLS key gen/load on init
- config/config.go: remove BLSKey field, defaultBLSKeyName/Path constants,
  BLSKeyFile() method
- test/e2e/node/main.go, rpc/test/helpers.go, consensus/wal_generator.go:
  remove BLS key loading and NewNode blsPrivKey param

Wire-compatible fields retained (per spec §3.2.1):
- types/block.go: Header.BatchHash, BlockID.BatchHash, Data.L2BatchHeader,
  Header.IsBatchPoint()
- types/vote.go: Vote.BLSSignature, ValidateBasic length constraints
- types/canonical.go: BlockID.BatchHash in canonical bytes
- proto/tendermint/types/types.proto + generated .pb.go: all proto fields

Note: three test fixtures (consensus/TestConsMsgsVectors,
blocksync/TestBlockchainMessageVectors, evidence/TestEvidenceVectors) fail
due to a pre-existing golden-byte mismatch caused by BlockID.BatchHash
shifting PartSetHeader field number from 2 to 3; unrelated to this change.

Also fixes a pre-existing compile gap in rpc/test/helpers.go and
test/e2e/node/main.go where NewNode callers were missing the
sequencerVerifier/sequencerSigner params added in a prior PR.

Made-with: Cursor
@FletcherMan FletcherMan requested a review from a team as a code owner April 23, 2026 11:03
@FletcherMan FletcherMan requested review from secmgt and removed request for a team April 23, 2026 11:03
fletcher.fan added 2 commits April 24, 2026 16:36
L2Node.DeliverBlock returned *tmproto.BatchParams as its first value, but
that value is forwarded through ExecBlockOnL2Node -> ApplyBlock ->
GetConsensusParamsUpdate -> ConsensusParams.Update / FromProto, where the
Batch field is explicitly ignored (see types/params.go comments). The full
Go-level path is dead.

This commit removes the unused return at every Go-level layer:
  - l2node.L2Node.DeliverBlock interface signature (drop first return)
  - l2node.MockL2Node.DeliverBlock mock impl
  - state.ExecBlockOnL2Node return type
  - state.BlockExecutor.ApplyBlock call site
  - state.BlockExecutor.GetConsensusParamsUpdate parameter
  - consensus/replay.go ExecBlockOnL2Node call site

Wire format is unchanged: tmproto.ConsensusParams.Batch and
tmproto.BatchParams proto fields stay for historical state compatibility,
and the "intentionally ignored" comments in ConsensusParams.Update and
ConsensusParamsFromProto are preserved.

This is a coordinated cross-repo break with morph/node Executor.DeliverBlock
implementation, on top of the existing ConsensusData.BatchHash field
removal in this PR (no extra coordination cost).

Per spec-004 tech-design-tendermint.md task #5b.

Made-with: Cursor
@panos-xyz
Copy link
Copy Markdown

Code review

Found 1 issue:

  1. Vote.ValidateBasic still enforces BLS-signature requirement for batch-point blocks — if upgrade order is violated, consensus stalls

Lines 208–210 of types/vote.go are unchanged by this PR:

if vote.Type == tmproto.PrecommitType && len(vote.BlockID.BatchHash) > 0 && len(vote.BLSSignature) == 0 { return errors.New("blsSignature must exist when batchHash is not empty") }

If any validator upgrades (loses BLS keys) before the sequencer does, the sequencer will still propose blocks with BatchHash != nil. Upgraded validators receive such a block, construct a vote whose BlockID inherits BatchHash, but have no BLS key material to populate BLSSignature. That vote fails ValidateBasic and cannot be broadcast — a consensus liveness stall results.

The PR body's upgrade note ("sequencer must upgrade first, wait for last batch to finalize on L1, then upgrade remaining nodes") is a manual mitigation, but the code provides no enforcement. Consider removing or relaxing lines 208–210 so that an empty BLSSignature is accepted regardless of BatchHash, completing the BLS removal at the validation layer as well.

tendermint/types/vote.go

Lines 207 to 212 in f48e285

// only check it if it is `precommit` vote; no need to sign a bls signature during `prevote`
if vote.Type == tmproto.PrecommitType && len(vote.BlockID.BatchHash) > 0 && len(vote.BLSSignature) == 0 {
return errors.New("blsSignature must exist when batchHash is not empty")
}
return nil

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@panos-xyz
Copy link
Copy Markdown

Code review (additional findings)

Found 2 more issues:

  1. [Critical] finalizeCommit stores block.BatchHash in state.LastBlockID, but votes carry nil BatchHash — state and seen commit go out of sync

ApplyBlock is called with BlockID.BatchHash = block.BatchHash, but every signAddVote call now passes nil as batchHash, so seenCommit.Votes[i].BlockID.BatchHash = nil. Since BlockID.Equals compares BatchHash:

  • If a malicious or non-upgraded proposer produces a block with Header.BatchHash != nil, upgraded validators will vote with BlockID.BatchHash = nil while state.LastBlockID.BatchHash = block.BatchHash. In a mixed-version cluster, upgraded and legacy validators would vote on different BlockIDs (nil vs non-nil batch hash) and cannot reach a 2/3 quorum on the same BlockID → liveness stall.
  • Even in an all-upgraded cluster, if any legacy block slips through, the persisted state.LastBlockID disagrees with seenCommit, and future replay / new-node sync fails BlockID.Equals on the commit.

Recommend rejecting non-empty Header.BatchHash / Data.L2BatchHeader in ValidateBlock or proposal validation post-upgrade, or unify persistence to use the majority vote's BlockID. Add a regression test for this boundary.

tendermint/consensus/state.go

Lines 1771 to 1777 in f48e285

stateCopy, retainHeight, err = cs.blockExec.ApplyBlock(
stateCopy,
types.BlockID{
Hash: block.Hash(),
BatchHash: block.BatchHash,
PartSetHeader: blockParts.Header(),
},

tendermint/types/block.go

Lines 1229 to 1234 in f48e285

// Equals returns true if the BlockID matches the given BlockID
func (blockID BlockID) Equals(other BlockID) bool {
return bytes.Equal(blockID.Hash, other.Hash) &&
bytes.Equal(blockID.BatchHash, other.BatchHash) &&
blockID.PartSetHeader.Equals(other.PartSetHeader)
}

InitialHeight: state.InitialHeight,
LastBlockHeight: header.Height,
LastBlockID: blockID,
LastBlockTime: header.Time,
NextValidators: nValSet,

  1. [Important] ConsensusParams.ToProto no longer emits Batch, contradicting the PR's "proto field retained for wire compat" claim

The PR body states that tmproto.BatchParams is retained for wire compatibility, but ToProto() omits it entirely. ConsensusHash is unaffected (Hash() only uses BlockMaxBytes/BlockMaxGas), so this doesn't break consensus. However:

  • External tools or legacy binaries that read a post-upgrade node's state and dereference pbParams.Batch.BlocksInterval (without nil-check) will panic.
  • The symmetric comments on Update / FromProto ("intentionally ignored; proto field exists only for wire compat") suggest the intent was to preserve the proto field on the wire, but ToProto silently drops it.

Recommend either emitting Batch: &tmproto.BatchParams{} (empty but non-nil) to honor the wire-compat claim, or updating the PR body to explicitly note that new nodes emit nil Batch and legacy tooling must be upgraded.

tendermint/types/params.go

Lines 246 to 264 in f48e285

func (params *ConsensusParams) ToProto() tmproto.ConsensusParams {
return tmproto.ConsensusParams{
Block: &tmproto.BlockParams{
MaxBytes: params.Block.MaxBytes,
MaxGas: params.Block.MaxGas,
},
Evidence: &tmproto.EvidenceParams{
MaxAgeNumBlocks: params.Evidence.MaxAgeNumBlocks,
MaxAgeDuration: params.Evidence.MaxAgeDuration,
MaxBytes: params.Evidence.MaxBytes,
},
Validator: &tmproto.ValidatorParams{
PubKeyTypes: params.Validator.PubKeyTypes,
},
Version: &tmproto.VersionParams{
App: params.Version.App,
},
}
}

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@FletcherMan FletcherMan merged commit 7c61e1e into main May 6, 2026
9 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants