Use the prev inner block hash of the last non simplex block

adapters.go

@@ -67,6 +67,7 @@ func (s *CallbackStorage) Retrieve(seq uint64) (common.VerifiedBlock, common.Fin
 		return nil, common.Finalization{}, err
 	}
 	parsedBlock := &ParsedBlock{
+		legacyBlock:       seq <= s.lastNonSimplexHeight,
 		msm:               s.msm,
 		StateMachineBlock: block,
 	}
@@ -108,16 +109,20 @@ func (cb *cachedBlock) Verify(ctx context.Context, verifyOpts ...common.VerifyOp
 }
 
 type CachedStorage struct {
-	msm  *metadata.StateMachine
-	lock sync.RWMutex
+	// lastNonSimplexHeight is the height of the last block that is not a Simplex block.
+	// Blocks with height <= this are called legacy blocks.
+	lastNonSimplexHeight uint64
+	msm                  *metadata.StateMachine
+	lock                 sync.RWMutex
 	Storage
 	cache map[common.Digest]cachedBlock
 }
 
-func NewCachedStorage(storage Storage) *CachedStorage {
+func NewCachedStorage(storage Storage, lastNonSimplexHeight uint64) *CachedStorage {
 	return &CachedStorage{
-		Storage: storage,
-		cache:   make(map[common.Digest]cachedBlock),
+		lastNonSimplexHeight: lastNonSimplexHeight,
+		Storage:              storage,
+		cache:                make(map[common.Digest]cachedBlock),
 	}
 }
 
@@ -151,11 +156,25 @@ func (cs *CachedStorage) Retrieve(seq uint64, digest common.Digest) (common.Veri
 
 	// We don't populate the cache here because we populate it externally.
 	block, finalization, err := cs.GetBlock(seq)
-	if digest != (common.Digest{}) && block.Digest() != digest {
-		return nil, nil, common.ErrBlockNotFound
+	if err != nil {
+		return nil, nil, err
+	}
+
+	legacy := seq <= cs.lastNonSimplexHeight
+
+	if digest != (common.Digest{}) {
+		// A block predating Simplex is identified by its inner block's digest, not by a Simplex block digest.
+		blockDigest := block.Digest()
+		if legacy && block.InnerBlock != nil {
+			blockDigest = common.Digest(block.InnerBlock.Digest())
+		}
+		if blockDigest != digest {
+			return nil, nil, common.ErrBlockNotFound
+		}
 	}
 
 	return &ParsedBlock{
+		legacyBlock:       legacy,
 		StateMachineBlock: block,
 		msm:               cs.msm,
 	}, finalization, err
@@ -259,6 +278,7 @@ func (bw *blockBuilderWaiter) BuildBlock(ctx context.Context, metadata common.Pr
 	}
 
 	pb := &ParsedBlock{
+		legacyBlock:       metadata.Seq <= bw.cs.lastNonSimplexHeight,
 		StateMachineBlock: *block,
 		msm:               bw.msm,
 	}
@@ -289,8 +309,12 @@ func (bd *blockDeserializer) DeserializeBlock(ctx context.Context, bytes []byte)
 		innerBlock = block
 	}
 
+	seq := rawBlock.Metadata.SimplexProtocolMetadata.Seq
+	legacy := seq <= bd.cs.lastNonSimplexHeight
+
 	return &cachedBlock{
 		ParsedBlock: &ParsedBlock{
+			legacyBlock: legacy,
 			StateMachineBlock: metadata.StateMachineBlock{
 				InnerBlock: innerBlock,
 				Metadata:   rawBlock.Metadata,

adapters_test.go

@@ -38,7 +38,7 @@ func newTestParsedBlock(num uint64, payload string) *ParsedBlock {
 // and a verified but not yet indexed block at seq 5. A zero digest matches on
 // seq alone, a non-zero digest must match the block's digest exactly.
 func TestCachedStorageRetrieve(t *testing.T) {
-	cs := NewCachedStorage(newTestStorage())
+	cs := NewCachedStorage(newTestStorage(), 0)
 	indexedBlock := newTestParsedBlock(0, "indexed")
 	require.NoError(t, cs.Index(t.Context(), indexedBlock, common.Finalization{}))
 
@@ -56,7 +56,9 @@ func TestCachedStorageRetrieve(t *testing.T) {
 		seq       uint64
 		digest    common.Digest
 		wantBlock *ParsedBlock
-		wantErr   error
+		// legacy marks a block predating Simplex, which is identified by its inner block's digest.
+		legacy  bool
+		wantErr error
 	}{
 		{
 			name:      "cached block by seq with zero digest",
@@ -76,9 +78,11 @@ func TestCachedStorageRetrieve(t *testing.T) {
 			wantErr: common.ErrBlockNotFound,
 		},
 		{
+			// Seq 0 is at or below the last non-Simplex height, so it is served as a legacy block.
 			name:      "uncached seq falls through to storage",
 			seq:       0,
 			wantBlock: indexedBlock,
+			legacy:    true,
 		},
 		{
 			name:    "indexed block with mismatched digest",
@@ -101,7 +105,11 @@ func TestCachedStorageRetrieve(t *testing.T) {
 				return
 			}
 			require.NoError(t, err)
-			require.Equal(t, common.Digest(tt.wantBlock.Digest()), got.BlockHeader().Digest)
+			wantDigest := common.Digest(tt.wantBlock.Digest())
+			if tt.legacy {
+				wantDigest = common.Digest(tt.wantBlock.InnerBlock.Digest())
+			}
+			require.Equal(t, wantDigest, got.BlockHeader().Digest)
 			if tt.wantBlock == verifiedBlock {
 				require.Nil(t, fin)
 			}
@@ -113,7 +121,7 @@ func TestCachedStorageRetrieve(t *testing.T) {
 // a zero-digest Retrieve of that seq returns the finalized block with its
 // finalization, even when a verified fork at the same seq was cached.
 func TestCachedStorageIndexEvictsSameSeqFork(t *testing.T) {
-	cs := NewCachedStorage(newTestStorage())
+	cs := NewCachedStorage(newTestStorage(), 0)
 	require.NoError(t, cs.Index(t.Context(), newTestParsedBlock(0, "genesis"), common.Finalization{}))
 
 	equivocatedBlock := &cachedBlock{
@@ -181,7 +189,7 @@ func TestCachedStoragePopulatedByWal(t *testing.T) {
 // it is finalized and indexed.
 func TestCachedStoragePopulatedBySelfBuiltBlock(t *testing.T) {
 	storage := newTestStorageWithGenesis(t)
-	cs := NewCachedStorage(storage)
+	cs := NewCachedStorage(storage, 0)
 
 	msm, err := metadata.NewStateMachine(&metadata.Config{
 		Logger:                   testutil.MakeLogger(t, 1),
@@ -196,9 +204,8 @@ func TestCachedStoragePopulatedBySelfBuiltBlock(t *testing.T) {
 	vm := newBlockBuilderVM(storage, newPendingBlockSignal())
 	bw := newBlockBuilderWaiter(msm, cs, vm)
 
-	// Build a block on top of genesis
-	genesis := &ParsedBlock{StateMachineBlock: metadata.StateMachineBlock{InnerBlock: genesisBlock}}
-	md := common.ProtocolMetadata{Seq: 1, Prev: genesis.BlockHeader().Digest}
+	// Build a block on top of genesis, which as a block predating Simplex is identified by its inner digest.
+	md := common.ProtocolMetadata{Seq: 1, Prev: common.Digest(genesisBlock.Digest())}
 	vb, built := bw.BuildBlock(t.Context(), md, common.Blacklist{})
 	require.True(t, built)
 	require.Equal(t, md.Seq, vb.BlockHeader().Seq)

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,

instance.go

@@ -77,7 +77,7 @@ type Instance struct {
 }
 
 func NewInstance(config Config) *Instance {
-	cs := NewCachedStorage(config.Storage)
+	cs := NewCachedStorage(config.Storage, config.LastNonSimplexInnerBlock.Height())
 	// Non-validators have no block builder, so they pass a nil approval handler:
 	// they broadcast approvals but do not need to record their own locally.
 	transitionListener := newEpochTransitionListener(

instance_helpers_test.go

@@ -240,9 +240,20 @@ func newTestStorage() *testStorage {
 }
 
 func newTestStorageWithGenesis(t *testing.T) *testStorage {
+	return newTestStorageWithPreSimplexBlocks(t, genesisBlock)
+}
+
+func newTestStorageWithPreSimplexBlocks(t *testing.T, blocks ...*testInnerBlock) *testStorage {
 	s := newTestStorage()
-	genesis := &ParsedBlock{StateMachineBlock: metadata.StateMachineBlock{InnerBlock: genesisBlock}}
-	require.NoError(t, s.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, s.Index(context.Background(), block, common.Finalization{}))
+	}
 	return s
 }
 

instance_test.go

@@ -142,10 +142,11 @@ func TestInstanceDropsMessagesBeforeStart(t *testing.T) {
 	validator := newNodeMapping(1)
 	pChain := newTestPChain([]metadata.NodeBLSMapping{validator})
 	instance := NewInstance(Config{
-		PlatformChain: pChain,
-		Storage:       newTestStorageWithGenesis(t),
-		Logger:        testutil.MakeLogger(t, 1),
-		ID:            validator.NodeID[:],
+		LastNonSimplexInnerBlock: genesisBlock,
+		PlatformChain:            pChain,
+		Storage:                  newTestStorageWithGenesis(t),
+		Logger:                   testutil.MakeLogger(t, 1),
+		ID:                       validator.NodeID[:],
 	})
 
 	msg := &common.Message{Finalization: &common.Finalization{}}
@@ -556,3 +557,57 @@ func TestValidatorSetsMetadataFromSnowman(t *testing.T) {
 	require.Equal(t, uint64(1), block.BlockHeader().Round)
 	require.Equal(t, numNonSimplexBlocks, block.BlockHeader().Seq)
 }
+
+// TestInstanceZeroBlockAfterPreSimplexBlocks brings up a network whose ledger already holds pre-Simplex
+// blocks beyond the genesis block, and asserts that the zero block the network commits chains to the
+// last non-Simplex block through that block's inner digest, at the sequence right above it.
+func TestInstanceZeroBlockAfterPreSimplexBlocks(t *testing.T) {
+	// The ledger holds pre-Simplex blocks from height 0 (genesis) to lastNonSimplexHeight.
+	const lastNonSimplexHeight = uint64(3)
+	zeroBlockSeq := lastNonSimplexHeight + 1
+
+	// Timestamps lie in the past because the zero block carries over the last non-Simplex block's timestamp.
+	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: append([]byte("pre-simplex block "), byte('0'+h)),
+		})
+	}
+	lastNonSimplexBlock := preSimplexBlocks[lastNonSimplexHeight]
+
+	// Two validators, so neither can commit the zero block alone: the leader's proposal is only
+	// committed once the other node has verified it against its own copy of the pre-Simplex chain.
+	nodeA := newNodeMapping(1)
+	nodeB := newNodeMapping(2)
+	pChain := newTestPChain(metadata.NodeBLSMappings{nodeA, nodeB})
+
+	network := newNetwork(t, pChain)
+	// The zero block is built automatically at zeroBlockSeq; the first block the network asks for follows it.
+	network.seq = zeroBlockSeq + 1
+
+	for _, id := range []common.NodeID{nodeA.NodeID[:], nodeB.NodeID[:]} {
+		network.addNodeWithConfig(id, nodeConfig{
+			storage:             newTestStorageWithPreSimplexBlocks(t, preSimplexBlocks...),
+			lastNonSimplexBlock: lastNonSimplexBlock,
+		})
+	}
+
+	// Both nodes commit the zero block, then a couple of ordinary Simplex blocks on top of it.
+	network.sync()
+	network.acceptNewBlock()
+	network.acceptNewBlock()
+
+	for _, n := range network.nodesSnapshot() {
+		zeroBlock, _, err := n.storage.GetBlock(zeroBlockSeq)
+		require.NoError(t, err)
+		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,
+			"zero block must point to the last non-Simplex block by its inner digest")
+		require.Equal(t, zeroBlockSeq, md.Seq)
+	}
+}

msm/msm.go

@@ -71,7 +71,8 @@ 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 block digest")
+	errZeroBlockSeqMismatch           = errors.New("zero block sequence does not succeed the last non-Simplex block sequence")
 	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")
@@ -905,6 +906,11 @@ func (sm *StateMachine) buildBlockZero(parentBlock StateMachineBlock, simplexMet
 	timestamp := sm.LastNonSimplexInnerBlock.Timestamp().UnixMilli()
 	simplexEpochInfo := constructSimplexZeroBlockSimplexEpochInfo(pChainHeight, validatorSet, prevVMBlockSeq)
 
+	// The zero block builds on top of the last non-Simplex block, which is identified by its inner block's
+	// digest rather than by a Simplex block digest, and sits right above it in the sequence.
+	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,
 	// and the first ICM epoch begins on the block built on top of the zero block.
@@ -984,8 +990,13 @@ 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
+	}
+	if block.Metadata.SimplexProtocolMetadata.Seq != sm.LastNonSimplexInnerBlock.Height()+1 {
+		return errZeroBlockSeqMismatch
 	}
 
 	// The timestamp must equal the last non-Simplex inner block's timestamp.