Skip to content

Commit 60fff56

Browse files
StephenButtolphStephenButtolph
authored
Mark preForkBlocks after the fork as Rejected (#1683)
1 parent 62d99d7 commit 60fff56

File tree

4 files changed

+134
-61
lines changed

4 files changed

+134
-61
lines changed

‎vms/proposervm/pre_fork_block.go‎

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,28 @@ func (b *preForkBlock) acceptInnerBlk(ctx context.Context) error {
3737
return b.Block.Accept(ctx)
3838
}
3939

40+
func (b *preForkBlock) Status() choices.Status {
41+
forkHeight, err := b.vm.getForkHeight()
42+
if err == database.ErrNotFound {
43+
return b.Block.Status()
44+
}
45+
if err != nil {
46+
// TODO: Once `Status()` can return an error, we should return the error
47+
// here.
48+
b.vm.ctx.Log.Error("unexpected error looking up fork height",
49+
zap.Error(err),
50+
)
51+
return b.Block.Status()
52+
}
53+
54+
// The fork has occurred earlier than this block, so preForkBlocks are all
55+
// invalid.
56+
if b.Height() >= forkHeight {
57+
return choices.Rejected
58+
}
59+
return b.Block.Status()
60+
}
61+
4062
func (b *preForkBlock) Verify(ctx context.Context) error {
4163
parent, err := b.vm.getPreForkBlock(ctx, b.Block.Parent())
4264
if err != nil {
@@ -79,10 +101,6 @@ func (b *preForkBlock) verifyPreForkChild(ctx context.Context, child *preForkBlo
79101
return err
80102
}
81103

82-
if err := b.verifyIsPreForkBlock(); err != nil {
83-
return err
84-
}
85-
86104
b.vm.ctx.Log.Debug("allowing pre-fork block after the fork time",
87105
zap.String("reason", "parent is an oracle block"),
88106
zap.Stringer("blkID", b.ID()),
@@ -98,10 +116,6 @@ func (b *preForkBlock) verifyPostForkChild(ctx context.Context, child *postForkB
98116
return err
99117
}
100118

101-
if err := b.verifyIsPreForkBlock(); err != nil {
102-
return err
103-
}
104-
105119
childID := child.ID()
106120
childPChainHeight := child.PChainHeight()
107121
currentPChainHeight, err := b.vm.ctx.ValidatorState.GetCurrentHeight(ctx)
@@ -232,26 +246,3 @@ func (b *preForkBlock) buildChild(ctx context.Context) (Block, error) {
232246
func (*preForkBlock) pChainHeight(context.Context) (uint64, error) {
233247
return 0, nil
234248
}
235-
236-
func (b *preForkBlock) verifyIsPreForkBlock() error {
237-
if status := b.Status(); status == choices.Accepted {
238-
_, err := b.vm.GetLastAccepted()
239-
if err == nil {
240-
// If this block is accepted and it was a preForkBlock, then there
241-
// shouldn't have been an accepted postForkBlock yet. If there was
242-
// an accepted postForkBlock, then this block wasn't a preForkBlock.
243-
return errUnexpectedBlockType
244-
}
245-
if err != database.ErrNotFound {
246-
// If an unexpected error was returned - propagate that that
247-
// error.
248-
return err
249-
}
250-
} else if _, contains := b.vm.Tree.Get(b.Block); contains {
251-
// If this block is a preForkBlock, then it's inner block shouldn't have
252-
// been registered into the inner block tree. If this block was
253-
// registered into the inner block tree, then it wasn't a preForkBlock.
254-
return errUnexpectedBlockType
255-
}
256-
return nil
257-
}

‎vms/proposervm/vm.go‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/ava-labs/avalanchego/snow/engine/common"
2929
"github.com/ava-labs/avalanchego/snow/engine/snowman/block"
3030
"github.com/ava-labs/avalanchego/utils"
31+
"github.com/ava-labs/avalanchego/utils/constants"
3132
"github.com/ava-labs/avalanchego/utils/math"
3233
"github.com/ava-labs/avalanchego/utils/timer/mockable"
3334
"github.com/ava-labs/avalanchego/vms/proposervm/indexer"
@@ -54,9 +55,26 @@ var (
5455
_ block.HeightIndexedChainVM = (*VM)(nil)
5556
_ block.StateSyncableVM = (*VM)(nil)
5657

58+
// TODO: remove after the X-chain supports height indexing.
59+
mainnetXChainID ids.ID
60+
fujiXChainID ids.ID
61+
5762
dbPrefix = []byte("proposervm")
5863
)
5964

65+
func init() {
66+
var err error
67+
mainnetXChainID, err = ids.FromString("2oYMBNV4eNHyqk2fjjV5nVQLDbtmNJzq5s3qs3Lo6ftnC6FByM")
68+
if err != nil {
69+
panic(err)
70+
}
71+
72+
fujiXChainID, err = ids.FromString("2JVSBoinj9C2J33VntvzYtVJNZdN2NKiwwKjcumHUWEb5DbBrm")
73+
if err != nil {
74+
panic(err)
75+
}
76+
}
77+
6078
type VM struct {
6179
block.ChainVM
6280
blockBuilderVM block.BuildBlockWithContextChainVM
@@ -219,7 +237,26 @@ func (vm *VM) Initialize(
219237
return err
220238
}
221239

222-
return vm.setLastAcceptedMetadata(ctx)
240+
if err := vm.setLastAcceptedMetadata(ctx); err != nil {
241+
return err
242+
}
243+
244+
forkHeight, err := vm.getForkHeight()
245+
switch err {
246+
case nil:
247+
chainCtx.Log.Info("initialized proposervm",
248+
zap.String("state", "after fork"),
249+
zap.Uint64("forkHeight", forkHeight),
250+
zap.Uint64("lastAcceptedHeight", vm.lastAcceptedHeight),
251+
)
252+
case database.ErrNotFound:
253+
chainCtx.Log.Info("initialized proposervm",
254+
zap.String("state", "before fork"),
255+
)
256+
default:
257+
return err
258+
}
259+
return nil
223260
}
224261

225262
// shutdown ops then propagate shutdown to innerVM
@@ -656,6 +693,26 @@ func (vm *VM) getBlock(ctx context.Context, id ids.ID) (Block, error) {
656693
return vm.getPreForkBlock(ctx, id)
657694
}
658695

696+
// TODO: remove after the P-chain and X-chain support height indexing.
697+
func (vm *VM) getForkHeight() (uint64, error) {
698+
// The fork block can be easily identified with the provided links because
699+
// the `Parent Hash` is equal to the `Proposer Parent ID`.
700+
switch vm.ctx.ChainID {
701+
case constants.PlatformChainID:
702+
switch vm.ctx.NetworkID {
703+
case constants.MainnetID:
704+
return 805732, nil // https://subnets.avax.network/p-chain/block/805732
705+
case constants.FujiID:
706+
return 47529, nil // https://subnets-test.avax.network/p-chain/block/47529
707+
}
708+
case mainnetXChainID:
709+
return 1, nil // https://subnets.avax.network/x-chain/block/1
710+
case fujiXChainID:
711+
return 1, nil // https://subnets-test.avax.network/x-chain/block/1
712+
}
713+
return vm.GetForkHeight()
714+
}
715+
659716
func (vm *VM) getPostForkBlock(ctx context.Context, blkID ids.ID) (PostForkBlock, error) {
660717
block, exists := vm.verifiedBlocks[blkID]
661718
if exists {

‎vms/proposervm/vm_byzantine_test.go‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -180,18 +180,17 @@ func TestInvalidByzantineProposerOracleParent(t *testing.T) {
180180
require.NoError(opts[0].Verify(context.Background()))
181181
require.NoError(opts[1].Verify(context.Background()))
182182

183-
yBlock, err := proVM.ParseBlock(context.Background(), xBlock.opts[0].Bytes())
184-
if err != nil {
185-
// It's okay for this block not to be parsed
186-
return
187-
}
188-
err = yBlock.Verify(context.Background())
183+
wrappedXBlock, err := proVM.ParseBlock(context.Background(), xBlock.Bytes())
184+
require.NoError(err)
185+
186+
err = wrappedXBlock.Verify(context.Background())
189187
require.ErrorIs(err, errUnexpectedBlockType)
190188

191189
require.NoError(aBlock.Accept(context.Background()))
192190

193-
err = yBlock.Verify(context.Background())
194-
require.ErrorIs(err, errUnexpectedBlockType)
191+
// Because the wrappedXBlock never passed verification and is now rejected,
192+
// the consensus engine will never verify any of its children.
193+
require.Equal(choices.Rejected, wrappedXBlock.Status())
195194
}
196195

197196
// Ensure that a byzantine node issuing an invalid PostForkBlock (B) when the
@@ -223,11 +222,6 @@ func TestInvalidByzantineProposerPreForkParent(t *testing.T) {
223222
return xBlock, nil
224223
}
225224

226-
aBlock, err := proVM.BuildBlock(context.Background())
227-
require.NoError(err)
228-
229-
coreVM.BuildBlockF = nil
230-
231225
yBlockBytes := []byte{2}
232226
yBlock := &snowman.TestBlock{
233227
TestDecidable: choices.TestDecidable{
@@ -265,31 +259,24 @@ func TestInvalidByzantineProposerPreForkParent(t *testing.T) {
265259
}
266260
}
267261

268-
bStatelessBlock, err := block.BuildUnsigned(
269-
xBlock.ID(),
270-
yBlock.Timestamp(),
271-
0,
272-
yBlockBytes,
273-
)
262+
aBlock, err := proVM.BuildBlock(context.Background())
274263
require.NoError(err)
275-
276-
bBlock, err := proVM.ParseBlock(context.Background(), bStatelessBlock.Bytes())
277-
if err != nil {
278-
// If there was an error parsing, then this is fine.
279-
return
280-
}
264+
coreVM.BuildBlockF = nil
281265

282266
require.NoError(aBlock.Verify(context.Background()))
283267

268+
wrappedXBlock, err := proVM.ParseBlock(context.Background(), xBlock.Bytes())
269+
require.NoError(err)
270+
284271
// If there wasn't an error parsing - verify must return an error
285-
err = bBlock.Verify(context.Background())
272+
err = wrappedXBlock.Verify(context.Background())
286273
require.ErrorIs(err, errUnexpectedBlockType)
287274

288275
require.NoError(aBlock.Accept(context.Background()))
289276

290-
// If there wasn't an error parsing - verify must return an error
291-
err = bBlock.Verify(context.Background())
292-
require.ErrorIs(err, errUnexpectedBlockType)
277+
// Because the wrappedXBlock never passed verification and is now rejected,
278+
// the consensus engine will never verify any of its children.
279+
require.Equal(choices.Rejected, wrappedXBlock.Status())
293280
}
294281

295282
// Ensure that a byzantine node issuing an invalid OptionBlock (B) which

‎vms/proposervm/vm_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ func initTestProposerVM(
174174
}
175175

176176
ctx := snow.DefaultContextTest()
177+
ctx.ChainID = ids.ID{1}
177178
ctx.NodeID = ids.NodeIDFromCert(pTestCert.Leaf)
178179
ctx.ValidatorState = valState
179180

@@ -2256,6 +2257,43 @@ func TestVMInnerBlkCacheDeduplicationRegression(t *testing.T) {
22562257
)
22572258
}
22582259

2260+
func TestVMInnerBlkMarkedAcceptedRegression(t *testing.T) {
2261+
require := require.New(t)
2262+
forkTime := time.Unix(0, 0)
2263+
coreVM, _, proVM, gBlock, _ := initTestProposerVM(t, forkTime, 0)
2264+
2265+
// create an inner block and wrap it in an postForkBlock.
2266+
innerBlock := &snowman.TestBlock{
2267+
TestDecidable: choices.TestDecidable{
2268+
IDV: ids.GenerateTestID(),
2269+
StatusV: choices.Processing,
2270+
},
2271+
BytesV: []byte{1},
2272+
ParentV: gBlock.ID(),
2273+
HeightV: gBlock.Height() + 1,
2274+
TimestampV: gBlock.Timestamp(),
2275+
}
2276+
2277+
coreVM.BuildBlockF = func(context.Context) (snowman.Block, error) {
2278+
return innerBlock, nil
2279+
}
2280+
outerBlock, err := proVM.BuildBlock(context.Background())
2281+
require.NoError(err)
2282+
coreVM.BuildBlockF = nil
2283+
2284+
require.NoError(outerBlock.Verify(context.Background()))
2285+
require.NoError(outerBlock.Accept(context.Background()))
2286+
2287+
coreVM.GetBlockF = func(_ context.Context, id ids.ID) (snowman.Block, error) {
2288+
require.Equal(innerBlock.ID(), id)
2289+
return innerBlock, nil
2290+
}
2291+
2292+
wrappedInnerBlock, err := proVM.GetBlock(context.Background(), innerBlock.ID())
2293+
require.NoError(err)
2294+
require.Equal(choices.Rejected, wrappedInnerBlock.Status())
2295+
}
2296+
22592297
type blockWithVerifyContext struct {
22602298
*snowman.MockBlock
22612299
*mocks.MockWithVerifyContext

0 commit comments

Comments
 (0)