From 526d19ad714e67c26a488334e67a836a332cc943 Mon Sep 17 00:00:00 2001 From: Yacov Manevich Date: Mon, 3 Aug 2026 13:54:26 +0200 Subject: [PATCH] Use the prev inner block hash of the last non simplex block Previously to this commit, when building the first ever simplex block, simplex was using the previous block hash of the protocol metadata, which is computed differently and incorrectly because the block previous to the first ever simplex block is not a simplex block. --- adapters.go | 6 ++- external.go | 12 +++++ instance.go | 12 ++--- instance_test.go | 105 ++++++++++++++++++++++++++++++++++++++++++- msm/msm.go | 13 +++--- msm/msm_test.go | 104 ++++++++++++++++++++++++++++++++---------- parsed_block_test.go | 38 ++++++++++++++++ 7 files changed, 251 insertions(+), 39 deletions(-) create mode 100644 parsed_block_test.go diff --git a/adapters.go b/adapters.go index 3072ab62..9c2fde48 100644 --- a/adapters.go +++ b/adapters.go @@ -35,8 +35,9 @@ func (c *Communication) Validators() common.Nodes { // Upon an epoch change, it will ignore blocks from previous epochs // and will call the onEpochChange callback when a new epoch is detected. type EpochAwareStorage struct { - msm *metadata.StateMachine - onEpochChange func(seq uint64, validators common.Nodes) error + lastNonSimplexHeight uint64 + msm *metadata.StateMachine + onEpochChange func(seq uint64, validators common.Nodes) error Storage epoch uint64 } @@ -49,6 +50,7 @@ func (e *EpochAwareStorage) Retrieve(seq uint64) (common.VerifiedBlock, common.F parsedBlock := &ParsedBlock{ msm: e.msm, StateMachineBlock: block, + legacyBlock: seq <= e.lastNonSimplexHeight, } return parsedBlock, *finalization, nil } diff --git a/external.go b/external.go index 41ed633e..1b5abef4 100644 --- a/external.go +++ b/external.go @@ -15,6 +15,8 @@ type ParsedBlock struct { metadata.StateMachineBlock msm *metadata.StateMachine + legacyBlock bool // true if this is not a simplex block, but a block pre-dating simplex. + // lock guards size, so Size() can be invoked concurrently lock sync.Mutex // size caches the length of the Bytes encoding, computed on first use @@ -27,6 +29,11 @@ func (p *ParsedBlock) Bytes() []byte { rawInnerBlock := p.InnerBlock.Bytes() innerBlockBytes = rawInnerBlock } + + if p.legacyBlock { + return innerBlockBytes + } + rawBlock := &metadata.RawBlock{ Metadata: p.Metadata.Clone(), InnerBlockBytes: innerBlockBytes, @@ -37,6 +44,11 @@ func (p *ParsedBlock) Bytes() []byte { func (p *ParsedBlock) BlockHeader() common.BlockHeader { md := p.Metadata.SimplexProtocolMetadata.Clone() digest := p.Digest() + + if p.legacyBlock { + digest = p.InnerBlock.Digest() + } + return common.BlockHeader{ ProtocolMetadata: md, Digest: digest, diff --git a/instance.go b/instance.go index 5d4fa5a3..a715ab94 100644 --- a/instance.go +++ b/instance.go @@ -156,8 +156,9 @@ func (i *Instance) createNonValidatorConfig(epochNum uint64, validators common.N comm.SetValidators(validators) epochAwareStorage := &EpochAwareStorage{ - epoch: epochNum, - Storage: i.Config.Storage, + lastNonSimplexHeight: i.Config.LastNonSimplexInnerBlock.Height(), + epoch: epochNum, + Storage: i.Config.Storage, onEpochChange: func(epoch uint64, validators common.Nodes) error { height := i.Config.PlatformChain.GetCurrentHeight() vdrs, err := i.Config.PlatformChain.GetValidatorSet(height) @@ -482,9 +483,10 @@ func (i *Instance) createEpochConfig() (simplex.EpochConfig, error) { comm.SetValidators(nodes) epochAwareStorage := &EpochAwareStorage{ - msm: msm, - epoch: epochNum, - Storage: i.cs, + lastNonSimplexHeight: i.Config.LastNonSimplexInnerBlock.Height(), + msm: msm, + epoch: epochNum, + Storage: i.cs, onEpochChange: func(epoch uint64, validators common.Nodes) error { blockBuilder.stop() comm.SetValidators(validators) diff --git a/instance_test.go b/instance_test.go index a4a4ce6c..8a490704 100644 --- a/instance_test.go +++ b/instance_test.go @@ -500,6 +500,83 @@ func TestInstanceDoubleStartFails(t *testing.T) { require.ErrorContains(t, inst.Start(t.Context()), "instance already started") } +// TestInstanceZeroBlockAfterPreSimplexBlocks brings up a network whose ledger already holds +// pre-Simplex blocks (not just the genesis block), and asserts that the zero block the network +// commits chains to the last non-Simplex block through the inner block's digest. +func TestInstanceZeroBlockAfterPreSimplexBlocks(t *testing.T) { + const ( + // The ledger holds pre-Simplex blocks from height 0 (genesis) to lastNonSimplexHeight. + lastNonSimplexHeight = uint64(3) + zeroBlockSeq = lastNonSimplexHeight + 1 + basePChainHeight = 100 + ) + + var firstID, secondID [20]byte + rand.Read(firstID[:]) + rand.Read(secondID[:]) + firstNodeID := common.NodeID(firstID[:]) + secondNodeID := common.NodeID(secondID[:]) + + // Both nodes are validators from the start, so neither can commit a block alone: the zero + // block proposed by the leader only gets committed if the other node verifies and votes for it. + validatorSetsAtHeight := map[uint64]metadata.NodeBLSMappings{ + basePChainHeight: { + {NodeID: firstID, BLSKey: []byte{0xaa}, Weight: 1}, + {NodeID: secondID, BLSKey: []byte{0xbb}, Weight: 1}, + }, + } + + pChain := newTestPlatformChain(basePChainHeight, validatorSetsAtHeight) + cops := &testCryptoOps{} + + // The pre-Simplex chain. Timestamps are in the past because the zero block carries over the + // last non-Simplex block's timestamp, which may not lie in the future. + preSimplexBlocks := make([]*testInnerBlock, 0, lastNonSimplexHeight+1) + for h := uint64(0); h <= lastNonSimplexHeight; h++ { + preSimplexBlocks = append(preSimplexBlocks, &testInnerBlock{ + Height_: h, + TS: time.Now().Add(-time.Duration(lastNonSimplexHeight-h+1) * time.Second), + Payload: []byte(fmt.Sprintf("pre-simplex block %d", h)), + }) + } + lastNonSimplexBlock := preSimplexBlocks[len(preSimplexBlocks)-1] + + net := newInMemNetwork(t) + t.Cleanup(net.stop) + + storage := newStorageWithBlocks(t, preSimplexBlocks...) + storage2 := newStorageWithBlocks(t, preSimplexBlocks...) + + // The VMs continue the pre-Simplex chain, so the first inner block they build sits right on + // top of the last non-Simplex block. + firstInstance := newInstanceWithVM(t, firstNodeID, storage, net, pChain, cops, lastNonSimplexBlock, newTestVMAtHeight(lastNonSimplexHeight+1)) + secondInstance := newInstanceWithVM(t, secondNodeID, storage2, net, pChain, cops, lastNonSimplexBlock, newTestVMAtHeight(lastNonSimplexHeight+1)) + net.register(firstNodeID, firstInstance) + net.register(secondNodeID, secondInstance) + + require.NoError(t, firstInstance.Start(t.Context())) + require.NoError(t, secondInstance.Start(t.Context())) + t.Cleanup(firstInstance.Stop) + t.Cleanup(secondInstance.Stop) + + // Both nodes commit the zero block and a few ordinary Simplex blocks on top of it. + const simplexBlocks = uint64(3) // zero block + 2 ordinary blocks + waitForNumBlocks(t, storage, zeroBlockSeq+simplexBlocks) + waitForNumBlocks(t, storage2, zeroBlockSeq+simplexBlocks) + + // Both nodes agree on a zero block that points to the last non-Simplex inner block. + for _, s := range []*MockStorage{storage, storage2} { + zeroBlock, ok := s.blockAt(zeroBlockSeq) + require.True(t, ok) + require.Equal(t, metadata.BlockTypeZero, zeroBlock.Type()) + require.Nil(t, zeroBlock.InnerBlock) + + md := zeroBlock.Metadata.SimplexProtocolMetadata + require.Equal(t, common.Digest(lastNonSimplexBlock.Digest()), md.Prev) + require.Equal(t, zeroBlockSeq, md.Seq) + } +} + // requireTipIsSealing asserts whether the last block in storage is a sealing block. func requireTipIsSealing(t *testing.T, storage *MockStorage, want bool) { t.Helper() @@ -539,10 +616,26 @@ func waitForSealingBlockCount(t *testing.T, storage *MockStorage, target int) { // newStorageWithGenesis returns storage holding only the genesis block, the ledger every node // here starts from. func newStorageWithGenesis(t *testing.T, genesisBlock *testInnerBlock) *MockStorage { + t.Helper() + return newStorageWithBlocks(t, genesisBlock) +} + +// newStorageWithBlocks returns storage holding the given non-Simplex blocks (a genesis block, and +// possibly further pre-Simplex blocks on top of it), indexed in order. +// A non-Simplex block carries no Simplex metadata, save for the sequence number the test storage +// indexes it by, which for these blocks equals its height. +func newStorageWithBlocks(t *testing.T, blocks ...*testInnerBlock) *MockStorage { t.Helper() storage := NewMockStorage(t) - genesis := &ParsedBlock{StateMachineBlock: metadata.StateMachineBlock{InnerBlock: genesisBlock}} - require.NoError(t, storage.Index(context.Background(), genesis, common.Finalization{})) + for _, inner := range blocks { + block := &ParsedBlock{StateMachineBlock: metadata.StateMachineBlock{ + InnerBlock: inner, + Metadata: metadata.StateMachineMetadata{ + SimplexProtocolMetadata: common.ProtocolMetadata{Seq: inner.Height()}, + }, + }} + require.NoError(t, storage.Index(context.Background(), block, common.Finalization{})) + } return storage } @@ -680,6 +773,14 @@ func newTestVM() *testVM { return vm } +// newTestVMAtHeight returns a VM whose first built block has the given height, for a ledger that +// already holds blocks above the genesis block. +func newTestVMAtHeight(height uint64) *testVM { + vm := &testVM{} + vm.nextHeight.Store(height) + return vm +} + func (vm *testVM) pause() { vm.paused.Store(true) } func (vm *testVM) resume() { vm.paused.Store(false) } diff --git a/msm/msm.go b/msm/msm.go index dc398411..a6aac036 100644 --- a/msm/msm.go +++ b/msm/msm.go @@ -73,7 +73,7 @@ var ( errNilBlock = errors.New("block is nil") errInvalidPChainHeight = errors.New("invalid P-chain height") errZeroBlockHasInnerBlock = errors.New("zero block must not have an inner block") - errZeroBlockInnerDigestMismatch = errors.New("zero block inner block digest does not match last non-Simplex inner block digest") + errZeroBlockPrevDigestMismatch = errors.New("zero block previous digest does not match last non-Simplex inner block digest") errZeroBlockTimestampMismatch = errors.New("zero block timestamp does not match last non-Simplex inner block timestamp") errPrevSealingBlockNotFinalized = errors.New("previous sealing block is not finalized") errBlockDigestMismatch = errors.New("does not match proposed block digest") @@ -772,9 +772,8 @@ func (sm *StateMachine) buildBlockZero(parentBlock StateMachineBlock, simplexMet timestamp := sm.LastNonSimplexInnerBlock.Timestamp().UnixMilli() simplexEpochInfo := constructSimplexZeroBlockSimplexEpochInfo(pChainHeight, validatorSet, prevVMBlockSeq) - md := simplexMetadata - md.Prev = sm.LastNonSimplexInnerBlock.Digest() - md.Seq = sm.LastNonSimplexInnerBlock.Height() + simplexMetadata.Prev = sm.LastNonSimplexInnerBlock.Digest() + simplexMetadata.Seq = sm.LastNonSimplexInnerBlock.Height() + 1 // The zero block carries over the parent's ICM epoch unchanged, just as it carries over the // timestamp. If the parent is a genesis block that predates ICM, the carried-over epoch is empty, @@ -849,8 +848,10 @@ func (sm *StateMachine) verifyBlockZero(block *StateMachineBlock, prevBlock Stat if block.InnerBlock != nil { return errZeroBlockHasInnerBlock } - if prevBlock.InnerBlock.Digest() != sm.LastNonSimplexInnerBlock.Digest() { - return errZeroBlockInnerDigestMismatch + + // The zero block must build upon the last non-Simplex block + if block.Metadata.SimplexProtocolMetadata.Prev != sm.LastNonSimplexInnerBlock.Digest() { + return errZeroBlockPrevDigestMismatch } // The timestamp must equal the last non-Simplex inner block's timestamp. diff --git a/msm/msm_test.go b/msm/msm_test.go index ddbd0df9..ba1366b5 100644 --- a/msm/msm_test.go +++ b/msm/msm_test.go @@ -169,7 +169,7 @@ func TestMSMFirstSimplexBlockAfterPreSimplexBlocks(t *testing.T) { Round: 0, Seq: 43, Epoch: 43, - Prev: preSimplexParent.Digest(), + Prev: preSimplexParent.InnerBlock.Digest(), } sm1, testConfig1 := newStateMachine(t) @@ -220,6 +220,65 @@ func TestMSMFirstSimplexBlockAfterPreSimplexBlocks(t *testing.T) { require.NoError(t, sm2.VerifyBlock(context.Background(), block)) } +// TestMSMZeroBlockPrevIsLastNonSimplexInnerBlockDigest ensures the zero block anchors itself to the +// last non-Simplex block via that block's inner digest, and not via the digest consensus hands us +// in the protocol metadata. +func TestMSMZeroBlockPrevIsLastNonSimplexInnerBlockDigest(t *testing.T) { + preSimplexParent := StateMachineBlock{ + InnerBlock: &testutil.InnerBlock{ + TS: time.Now(), + BlockHeight: 42, + Content: []byte{4, 5, 6}, + }, + } + + innerDigest := common.Digest(preSimplexParent.InnerBlock.Digest()) + outerDigest := common.Digest(preSimplexParent.Digest()) + require.NotEqual(t, innerDigest, outerDigest) + + newZeroBlockStateMachine := func(t *testing.T) *StateMachine { + sm, tc := newStateMachine(t) + tc.blockStore[42] = &outerBlock{block: preSimplexParent} + sm.LastNonSimplexInnerBlock = preSimplexParent.InnerBlock + return sm + } + + t.Run("built zero block points to the inner digest", func(t *testing.T) { + sm := newZeroBlockStateMachine(t) + + block, err := sm.BuildBlock(context.Background(), common.ProtocolMetadata{ + Round: 0, + Seq: 43, + Epoch: 43, + Prev: outerDigest, + }, emptyBlacklist) + require.NoError(t, err) + + require.Equal(t, innerDigest, block.Metadata.SimplexProtocolMetadata.Prev) + require.Equal(t, preSimplexParent.InnerBlock.Height()+1, block.Metadata.SimplexProtocolMetadata.Seq) + + // A different node, which only knows the last non-Simplex block, accepts it. + require.NoError(t, newZeroBlockStateMachine(t).VerifyBlock(context.Background(), block)) + }) + + t.Run("zero block not pointing to the inner digest is rejected", func(t *testing.T) { + sm := newZeroBlockStateMachine(t) + + block, err := sm.BuildBlock(context.Background(), common.ProtocolMetadata{ + Round: 0, + Seq: 43, + Epoch: 43, + Prev: innerDigest, + }, emptyBlacklist) + require.NoError(t, err) + + block.Metadata.SimplexProtocolMetadata.Prev = outerDigest + + err = newZeroBlockStateMachine(t).VerifyBlock(context.Background(), block) + require.ErrorIs(t, err, errZeroBlockPrevDigestMismatch) + }) +} + func TestMSMBuildBlockRejectsZeroSeq(t *testing.T) { // Seq 0 is reserved for the genesis block, which should never be built. sm, _ := newStateMachine(t) @@ -454,24 +513,21 @@ func TestMSMFullEpochLifecycle(t *testing.T) { } // ----- Step 0: Building on top of genesis or upgrading to Simplex----- - genesis := StateMachineBlock{ - InnerBlock: &testutil.InnerBlock{ - BlockHeight: 0, // Genesis block has height 0 - TS: startTime, - Content: []byte{0}, - }, + genesis := &testutil.InnerBlock{ + BlockHeight: 0, // Genesis block has height 0 + TS: startTime, + Content: []byte{0}, } - notGenesis := StateMachineBlock{ - InnerBlock: &testutil.InnerBlock{ - BlockHeight: 42, - TS: startTime, - Content: []byte{0}, - }, + notGenesis := &testutil.InnerBlock{ + BlockHeight: 42, + TS: startTime, + Content: []byte{0}, } + for _, testCase := range []struct { name string - firstBlockBeforeSimplex StateMachineBlock + firstBlockBeforeSimplex *testutil.InnerBlock epochNum uint64 // firstBlockICMEpochInfo is the ICM epoch of the pre-Simplex parent, which the zero block // carries over. A genesis parent predates ICM, so its ICM epoch is empty and the first epoch @@ -487,7 +543,7 @@ func TestMSMFullEpochLifecycle(t *testing.T) { { name: "upgrading to Simplex from pre-Simplex blocks", firstBlockBeforeSimplex: notGenesis, - epochNum: notGenesis.InnerBlock.Height() + 1, + epochNum: notGenesis.Height() + 1, firstBlockICMEpochInfo: ICMEpochInfo{ PChainEpochHeight: pChainHeight1, EpochNumber: 1, @@ -549,9 +605,6 @@ func TestMSMFullEpochLifecycle(t *testing.T) { EpochStartTime: uint64(startTime.Unix()) + 1, } - // The zero block carries over the parent's ICM epoch. - testCase.firstBlockBeforeSimplex.Metadata.ICMEpochInfo = testCase.firstBlockICMEpochInfo - sm, tc := newStateMachine(t) sm.GetValidatorSet = getValidatorSet @@ -563,10 +616,10 @@ func TestMSMFullEpochLifecycle(t *testing.T) { // behavior is covered by TestVerifyCollectingApprovalsNotReady and // TestCollectAuxiliaryInfo. sm.AuxiliaryInfoApp = &noopTestAuxInfoApp{} - tc.blockStore[0] = &outerBlock{block: genesis} - tc.blockStore[42] = &outerBlock{block: notGenesis} + tc.blockStore[0] = &outerBlock{block: StateMachineBlock{InnerBlock: genesis}} + tc.blockStore[42] = &outerBlock{block: StateMachineBlock{InnerBlock: notGenesis}} - sm.LastNonSimplexInnerBlock = testCase.firstBlockBeforeSimplex.InnerBlock + sm.LastNonSimplexInnerBlock = testCase.firstBlockBeforeSimplex sm.GenesisValidatorSet = validatorSet1 sm.LastNonSimplexBlockPChainHeight = pChainHeight1 @@ -591,7 +644,7 @@ func TestMSMFullEpochLifecycle(t *testing.T) { smVerify.GetTime = fixedTime - smVerify.LastNonSimplexInnerBlock = testCase.firstBlockBeforeSimplex.InnerBlock + smVerify.LastNonSimplexInnerBlock = testCase.firstBlockBeforeSimplex smVerify.GenesisValidatorSet = validatorSet1 smVerify.LastNonSimplexBlockPChainHeight = pChainHeight1 @@ -601,8 +654,11 @@ func TestMSMFullEpochLifecycle(t *testing.T) { tcVerify.blockStore[seq] = &outerBlock{block: block, finalization: fin} } - baseSeq := testCase.firstBlockBeforeSimplex.InnerBlock.Height() - addBlock(baseSeq, testCase.firstBlockBeforeSimplex, nil) + baseSeq := testCase.firstBlockBeforeSimplex.Height() + addBlock(baseSeq, StateMachineBlock{InnerBlock: testCase.firstBlockBeforeSimplex, Metadata: StateMachineMetadata{ + // The zero block carries over the parent's ICM epoch. + ICMEpochInfo: testCase.firstBlockICMEpochInfo, + }}, nil) aggr := &signatureAggregator{} diff --git a/parsed_block_test.go b/parsed_block_test.go new file mode 100644 index 00000000..26edea65 --- /dev/null +++ b/parsed_block_test.go @@ -0,0 +1,38 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package simplex + +import ( + "testing" + "time" + + "github.com/ava-labs/simplex/common" + metadata "github.com/ava-labs/simplex/msm" + + "github.com/stretchr/testify/require" +) + +func TestParsedBlockLegacyBlock(t *testing.T) { + inner := &testInnerBlock{Height_: 3, TS: time.Now(), Payload: []byte("pre-simplex")} + smb := metadata.StateMachineBlock{ + InnerBlock: inner, + Metadata: metadata.StateMachineMetadata{ + SimplexProtocolMetadata: common.ProtocolMetadata{Seq: inner.Height()}, + }, + } + + legacyBlock := &ParsedBlock{StateMachineBlock: smb, legacyBlock: true} + simplexBlock := &ParsedBlock{StateMachineBlock: smb} + + // A block predating Simplex is its inner block, both on the wire and in its digest. + require.Equal(t, inner.Bytes(), legacyBlock.Bytes()) + require.Equal(t, len(inner.Bytes()), legacyBlock.Size()) + require.Equal(t, common.Digest(inner.Digest()), legacyBlock.BlockHeader().Digest) + + // A Simplex block is the inner block wrapped together with its metadata, so both its + // encoding and its digest differ from the block predating Simplex. + require.Equal(t, common.Digest(smb.Digest()), simplexBlock.BlockHeader().Digest) + require.NotEqual(t, legacyBlock.Bytes(), simplexBlock.Bytes()) + require.NotEqual(t, legacyBlock.BlockHeader().Digest, simplexBlock.BlockHeader().Digest) +}