Merge branch 'dev' into version-bump-v1.10.4

vms/proposervm/pre_fork_block.go

@@ -37,6 +37,28 @@ func (b *preForkBlock) acceptInnerBlk(ctx context.Context) error {
 	return b.Block.Accept(ctx)
 }
 
+func (b *preForkBlock) Status() choices.Status {
+	forkHeight, err := b.vm.getForkHeight()
+	if err == database.ErrNotFound {
+		return b.Block.Status()
+	}
+	if err != nil {
+		// TODO: Once `Status()` can return an error, we should return the error
+		// here.
+		b.vm.ctx.Log.Error("unexpected error looking up fork height",
+			zap.Error(err),
+		)
+		return b.Block.Status()
+	}
+
+	// The fork has occurred earlier than this block, so preForkBlocks are all
+	// invalid.
+	if b.Height() >= forkHeight {
+		return choices.Rejected
+	}
+	return b.Block.Status()
+}
+
 func (b *preForkBlock) Verify(ctx context.Context) error {
 	parent, err := b.vm.getPreForkBlock(ctx, b.Block.Parent())
 	if err != nil {
@@ -79,10 +101,6 @@ func (b *preForkBlock) verifyPreForkChild(ctx context.Context, child *preForkBlo
 			return err
 		}
 
-		if err := b.verifyIsPreForkBlock(); err != nil {
-			return err
-		}
-
 		b.vm.ctx.Log.Debug("allowing pre-fork block after the fork time",
 			zap.String("reason", "parent is an oracle block"),
 			zap.Stringer("blkID", b.ID()),
@@ -98,10 +116,6 @@ func (b *preForkBlock) verifyPostForkChild(ctx context.Context, child *postForkB
 		return err
 	}
 
-	if err := b.verifyIsPreForkBlock(); err != nil {
-		return err
-	}
-
 	childID := child.ID()
 	childPChainHeight := child.PChainHeight()
 	currentPChainHeight, err := b.vm.ctx.ValidatorState.GetCurrentHeight(ctx)
@@ -232,26 +246,3 @@ func (b *preForkBlock) buildChild(ctx context.Context) (Block, error) {
 func (*preForkBlock) pChainHeight(context.Context) (uint64, error) {
 	return 0, nil
 }
-
-func (b *preForkBlock) verifyIsPreForkBlock() error {
-	if status := b.Status(); status == choices.Accepted {
-		_, err := b.vm.GetLastAccepted()
-		if err == nil {
-			// If this block is accepted and it was a preForkBlock, then there
-			// shouldn't have been an accepted postForkBlock yet. If there was
-			// an accepted postForkBlock, then this block wasn't a preForkBlock.
-			return errUnexpectedBlockType
-		}
-		if err != database.ErrNotFound {
-			// If an unexpected error was returned - propagate that that
-			// error.
-			return err
-		}
-	} else if _, contains := b.vm.Tree.Get(b.Block); contains {
-		// If this block is a preForkBlock, then it's inner block shouldn't have
-		// been registered into the inner block tree. If this block was
-		// registered into the inner block tree, then it wasn't a preForkBlock.
-		return errUnexpectedBlockType
-	}
-	return nil
-}

vms/proposervm/vm.go

@@ -28,6 +28,7 @@ import (
 	"github.com/ava-labs/avalanchego/snow/engine/common"
 	"github.com/ava-labs/avalanchego/snow/engine/snowman/block"
 	"github.com/ava-labs/avalanchego/utils"
+	"github.com/ava-labs/avalanchego/utils/constants"
 	"github.com/ava-labs/avalanchego/utils/math"
 	"github.com/ava-labs/avalanchego/utils/timer/mockable"
 	"github.com/ava-labs/avalanchego/vms/proposervm/indexer"
@@ -54,9 +55,26 @@ var (
 	_ block.HeightIndexedChainVM = (*VM)(nil)
 	_ block.StateSyncableVM      = (*VM)(nil)
 
+	// TODO: remove after the X-chain supports height indexing.
+	mainnetXChainID ids.ID
+	fujiXChainID    ids.ID
+
 	dbPrefix = []byte("proposervm")
 )
 
+func init() {
+	var err error
+	mainnetXChainID, err = ids.FromString("2oYMBNV4eNHyqk2fjjV5nVQLDbtmNJzq5s3qs3Lo6ftnC6FByM")
+	if err != nil {
+		panic(err)
+	}
+
+	fujiXChainID, err = ids.FromString("2JVSBoinj9C2J33VntvzYtVJNZdN2NKiwwKjcumHUWEb5DbBrm")
+	if err != nil {
+		panic(err)
+	}
+}
+
 type VM struct {
 	block.ChainVM
 	blockBuilderVM block.BuildBlockWithContextChainVM
@@ -219,7 +237,26 @@ func (vm *VM) Initialize(
 		return err
 	}
 
-	return vm.setLastAcceptedMetadata(ctx)
+	if err := vm.setLastAcceptedMetadata(ctx); err != nil {
+		return err
+	}
+
+	forkHeight, err := vm.getForkHeight()
+	switch err {
+	case nil:
+		chainCtx.Log.Info("initialized proposervm",
+			zap.String("state", "after fork"),
+			zap.Uint64("forkHeight", forkHeight),
+			zap.Uint64("lastAcceptedHeight", vm.lastAcceptedHeight),
+		)
+	case database.ErrNotFound:
+		chainCtx.Log.Info("initialized proposervm",
+			zap.String("state", "before fork"),
+		)
+	default:
+		return err
+	}
+	return nil
 }
 
 // shutdown ops then propagate shutdown to innerVM
@@ -656,6 +693,26 @@ func (vm *VM) getBlock(ctx context.Context, id ids.ID) (Block, error) {
 	return vm.getPreForkBlock(ctx, id)
 }
 
+// TODO: remove after the P-chain and X-chain support height indexing.
+func (vm *VM) getForkHeight() (uint64, error) {
+	// The fork block can be easily identified with the provided links because
+	// the `Parent Hash` is equal to the `Proposer Parent ID`.
+	switch vm.ctx.ChainID {
+	case constants.PlatformChainID:
+		switch vm.ctx.NetworkID {
+		case constants.MainnetID:
+			return 805732, nil // https://subnets.avax.network/p-chain/block/805732
+		case constants.FujiID:
+			return 47529, nil // https://subnets-test.avax.network/p-chain/block/47529
+		}
+	case mainnetXChainID:
+		return 1, nil // https://subnets.avax.network/x-chain/block/1
+	case fujiXChainID:
+		return 1, nil // https://subnets-test.avax.network/x-chain/block/1
+	}
+	return vm.GetForkHeight()
+}
+
 func (vm *VM) getPostForkBlock(ctx context.Context, blkID ids.ID) (PostForkBlock, error) {
 	block, exists := vm.verifiedBlocks[blkID]
 	if exists {

vms/proposervm/vm_byzantine_test.go

@@ -180,18 +180,17 @@ func TestInvalidByzantineProposerOracleParent(t *testing.T) {
 	require.NoError(opts[0].Verify(context.Background()))
 	require.NoError(opts[1].Verify(context.Background()))
 
-	yBlock, err := proVM.ParseBlock(context.Background(), xBlock.opts[0].Bytes())
-	if err != nil {
-		// It's okay for this block not to be parsed
-		return
-	}
-	err = yBlock.Verify(context.Background())
+	wrappedXBlock, err := proVM.ParseBlock(context.Background(), xBlock.Bytes())
+	require.NoError(err)
+
+	err = wrappedXBlock.Verify(context.Background())
 	require.ErrorIs(err, errUnexpectedBlockType)
 
 	require.NoError(aBlock.Accept(context.Background()))
 
-	err = yBlock.Verify(context.Background())
-	require.ErrorIs(err, errUnexpectedBlockType)
+	// Because the wrappedXBlock never passed verification and is now rejected,
+	// the consensus engine will never verify any of its children.
+	require.Equal(choices.Rejected, wrappedXBlock.Status())
 }
 
 // Ensure that a byzantine node issuing an invalid PostForkBlock (B) when the
@@ -223,11 +222,6 @@ func TestInvalidByzantineProposerPreForkParent(t *testing.T) {
 		return xBlock, nil
 	}
 
-	aBlock, err := proVM.BuildBlock(context.Background())
-	require.NoError(err)
-
-	coreVM.BuildBlockF = nil
-
 	yBlockBytes := []byte{2}
 	yBlock := &snowman.TestBlock{
 		TestDecidable: choices.TestDecidable{
@@ -265,31 +259,24 @@ func TestInvalidByzantineProposerPreForkParent(t *testing.T) {
 		}
 	}
 
-	bStatelessBlock, err := block.BuildUnsigned(
-		xBlock.ID(),
-		yBlock.Timestamp(),
-		0,
-		yBlockBytes,
-	)
+	aBlock, err := proVM.BuildBlock(context.Background())
 	require.NoError(err)
-
-	bBlock, err := proVM.ParseBlock(context.Background(), bStatelessBlock.Bytes())
-	if err != nil {
-		// If there was an error parsing, then this is fine.
-		return
-	}
+	coreVM.BuildBlockF = nil
 
 	require.NoError(aBlock.Verify(context.Background()))
 
+	wrappedXBlock, err := proVM.ParseBlock(context.Background(), xBlock.Bytes())
+	require.NoError(err)
+
 	// If there wasn't an error parsing - verify must return an error
-	err = bBlock.Verify(context.Background())
+	err = wrappedXBlock.Verify(context.Background())
 	require.ErrorIs(err, errUnexpectedBlockType)
 
 	require.NoError(aBlock.Accept(context.Background()))
 
-	// If there wasn't an error parsing - verify must return an error
-	err = bBlock.Verify(context.Background())
-	require.ErrorIs(err, errUnexpectedBlockType)
+	// Because the wrappedXBlock never passed verification and is now rejected,
+	// the consensus engine will never verify any of its children.
+	require.Equal(choices.Rejected, wrappedXBlock.Status())
 }
 
 // Ensure that a byzantine node issuing an invalid OptionBlock (B) which

vms/proposervm/vm_test.go

@@ -174,6 +174,7 @@ func initTestProposerVM(
 	}
 
 	ctx := snow.DefaultContextTest()
+	ctx.ChainID = ids.ID{1}
 	ctx.NodeID = ids.NodeIDFromCert(pTestCert.Leaf)
 	ctx.ValidatorState = valState
 
@@ -2256,6 +2257,43 @@ func TestVMInnerBlkCacheDeduplicationRegression(t *testing.T) {
 	)
 }
 
+func TestVMInnerBlkMarkedAcceptedRegression(t *testing.T) {
+	require := require.New(t)
+	forkTime := time.Unix(0, 0)
+	coreVM, _, proVM, gBlock, _ := initTestProposerVM(t, forkTime, 0)
+
+	// create an inner block and wrap it in an postForkBlock.
+	innerBlock := &snowman.TestBlock{
+		TestDecidable: choices.TestDecidable{
+			IDV:     ids.GenerateTestID(),
+			StatusV: choices.Processing,
+		},
+		BytesV:     []byte{1},
+		ParentV:    gBlock.ID(),
+		HeightV:    gBlock.Height() + 1,
+		TimestampV: gBlock.Timestamp(),
+	}
+
+	coreVM.BuildBlockF = func(context.Context) (snowman.Block, error) {
+		return innerBlock, nil
+	}
+	outerBlock, err := proVM.BuildBlock(context.Background())
+	require.NoError(err)
+	coreVM.BuildBlockF = nil
+
+	require.NoError(outerBlock.Verify(context.Background()))
+	require.NoError(outerBlock.Accept(context.Background()))
+
+	coreVM.GetBlockF = func(_ context.Context, id ids.ID) (snowman.Block, error) {
+		require.Equal(innerBlock.ID(), id)
+		return innerBlock, nil
+	}
+
+	wrappedInnerBlock, err := proVM.GetBlock(context.Background(), innerBlock.ID())
+	require.NoError(err)
+	require.Equal(choices.Rejected, wrappedInnerBlock.Status())
+}
+
 type blockWithVerifyContext struct {
 	*snowman.MockBlock
 	*mocks.MockWithVerifyContext