Merge 318bab8 into 3988055

iotexproject · Apr 12, 2023 · dccdf90 · dccdf90
2 parents 3988055 + 318bab8
commit dccdf90
Show file tree

Hide file tree

Showing 5 changed files with 191 additions and 36 deletions.
diff --git a/action/protocol/context.go b/action/protocol/context.go
@@ -111,6 +111,7 @@ type (
 		FixRewardErroCheckPosition              bool
 		EnableWeb3Rewarding                     bool
 		SkipSystemActionNonce                   bool
+		ValidateSystemAction                    bool
 	}
 
 	// FeatureWithHeightCtx provides feature check functions.
@@ -248,6 +249,7 @@ func WithFeatureCtx(ctx context.Context) context.Context {
 			FixRewardErroCheckPosition:              g.IsOkhotsk(height),
 			EnableWeb3Rewarding:                     g.IsPalau(height),
 			SkipSystemActionNonce:                   g.IsPalau(height),
+			ValidateSystemAction:                    g.IsToBeEnabled(height),
 		},
 	)
 }

diff --git a/state/factory/factory.go b/state/factory/factory.go
@@ -359,20 +359,16 @@ func (sf *factory) NewBlockBuilder(
 		return nil, errors.Wrap(err, "Failed to obtain working set from state factory")
 	}
 	postSystemActions := make([]action.SealedEnvelope, 0)
-	for _, p := range sf.registry.All() {
-		if psac, ok := p.(protocol.PostSystemActionsCreator); ok {
-			elps, err := psac.CreatePostSystemActions(ctx, ws)
-			if err != nil {
-				return nil, err
-			}
-			for _, elp := range elps {
-				se, err := sign(elp)
-				if err != nil {
-					return nil, err
-				}
-				postSystemActions = append(postSystemActions, se)
-			}
+	unsignedSystemActions, err := ws.generateSystemActions(ctx)
+	if err != nil {
+		return nil, err
+	}
+	for _, elp := range unsignedSystemActions {
+		se, err := sign(elp)
+		if err != nil {
+			return nil, err
 		}
+		postSystemActions = append(postSystemActions, se)
 	}
 	blkBuilder, err := ws.CreateBuilder(ctx, ap, postSystemActions, sf.cfg.Chain.AllowedBlockGasResidue)
 	if err != nil {

diff --git a/state/factory/statedb.go b/state/factory/statedb.go
@@ -239,20 +239,16 @@ func (sdb *stateDB) NewBlockBuilder(
 		return nil, err
 	}
 	postSystemActions := make([]action.SealedEnvelope, 0)
-	for _, p := range sdb.registry.All() {
-		if psac, ok := p.(protocol.PostSystemActionsCreator); ok {
-			elps, err := psac.CreatePostSystemActions(ctx, ws)
-			if err != nil {
-				return nil, err
-			}
-			for _, elp := range elps {
-				se, err := sign(elp)
-				if err != nil {
-					return nil, err
-				}
-				postSystemActions = append(postSystemActions, se)
-			}
+	unsignedSystemActions, err := ws.generateSystemActions(ctx)
+	if err != nil {
+		return nil, err
+	}
+	for _, elp := range unsignedSystemActions {
+		se, err := sign(elp)
+		if err != nil {
+			return nil, err
 		}
+		postSystemActions = append(postSystemActions, se)
 	}
 	blkBuilder, err := ws.CreateBuilder(ctx, ap, postSystemActions, sdb.cfg.Chain.AllowedBlockGasResidue)
 	if err != nil {

diff --git a/state/factory/workingset.go b/state/factory/workingset.go
@@ -36,7 +36,8 @@ var (
 		[]string{"type"},
 	)
 
-	errUnsupportWeb3Rewarding = errors.New("unsupported web3 rewarding")
+	errUnsupportWeb3Rewarding    = errors.New("unsupported web3 rewarding")
+	errInvalidSystemActionLayout = errors.New("system action layout is invalid")
 )
 
 func init() {
@@ -412,7 +413,6 @@ func (ws *workingSet) process(ctx context.Context, actions []action.SealedEnvelo
 			}
 		}
 	}
-	// TODO: verify whether the post system actions are appended tail
 
 	receipts, err := ws.runActions(ctx, actions)
 	if err != nil {
@@ -422,6 +422,47 @@ func (ws *workingSet) process(ctx context.Context, actions []action.SealedEnvelo
 	return ws.finalize()
 }
 
+func (ws *workingSet) generateSystemActions(ctx context.Context) ([]action.Envelope, error) {
+	reg := protocol.MustGetRegistry(ctx)
+	postSystemActions := []action.Envelope{}
+	for _, p := range reg.All() {
+		if psc, ok := p.(protocol.PostSystemActionsCreator); ok {
+			elps, err := psc.CreatePostSystemActions(ctx, ws)
+			if err != nil {
+				return nil, err
+			}
+			postSystemActions = append(postSystemActions, elps...)
+		}
+	}
+	return postSystemActions, nil
+}
+
+// validateSystemActionLayout verify whether the post system actions are appended tail
+func (ws *workingSet) validateSystemActionLayout(ctx context.Context, actions []action.SealedEnvelope) error {
+	postSystemActions, err := ws.generateSystemActions(ctx)
+	if err != nil {
+		return err
+	}
+	// system actions should be at the end of the action list, and they should be continuous
+	expectedStartIdx := len(actions) - len(postSystemActions)
+	sysActCnt := 0
+	for i := range actions {
+		if action.IsSystemAction(actions[i]) {
+			if i != expectedStartIdx+sysActCnt {
+				return errors.Wrapf(errInvalidSystemActionLayout, "the %d-th action should not be a system action", i)
+			}
+			if actions[i].Envelope.Proto().String() != postSystemActions[sysActCnt].Proto().String() {
+				return errors.Wrapf(errInvalidSystemActionLayout, "the %d-th action is not the expected system action", i)
+			}
+			sysActCnt++
+		}
+	}
+	if sysActCnt != len(postSystemActions) {
+		return errors.Wrapf(errInvalidSystemActionLayout, "the number of system actions is incorrect, expected %d, got %d", len(postSystemActions), sysActCnt)
+	}
+	return nil
+}
+
 func (ws *workingSet) pickAndRunActions(
 	ctx context.Context,
 	ap actpool.ActPool,
@@ -544,6 +585,12 @@ func (ws *workingSet) ValidateBlock(ctx context.Context, blk *block.Block) error
 			return errors.Wrap(err, "failed to validate nonce")
 		}
 	}
+	if protocol.MustGetFeatureCtx(ctx).ValidateSystemAction {
+		if err := ws.validateSystemActionLayout(ctx, blk.RunnableActions().Actions()); err != nil {
+			return err
+		}
+	}
+
 	if err := ws.process(ctx, blk.RunnableActions().Actions()); err != nil {
 		log.L().Error("Failed to update state.", zap.Uint64("height", ws.height), zap.Error(err))
 		return err

diff --git a/state/factory/workingset_test.go b/state/factory/workingset_test.go
@@ -208,19 +208,19 @@ func TestWorkingSet_ValidateBlock(t *testing.T) {
 			err   error
 		}{
 			{
-				makeBlock(t, 1, hash.ZeroHash256, receiptRoot, digestHash),
+				makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, makeTransferAction(t, 1)),
 				nil,
 			},
 			{
-				makeBlock(t, 3, hash.ZeroHash256, receiptRoot, digestHash),
+				makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, makeTransferAction(t, 3)),
 				action.ErrNonceTooHigh,
 			},
 			{
-				makeBlock(t, 1, hash.ZeroHash256, hash.Hash256b([]byte("test")), digestHash),
+				makeBlock(t, hash.ZeroHash256, hash.Hash256b([]byte("test")), digestHash, makeTransferAction(t, 1)),
 				block.ErrReceiptRootMismatch,
 			},
 			{
-				makeBlock(t, 1, hash.ZeroHash256, receiptRoot, hash.Hash256b([]byte("test"))),
+				makeBlock(t, hash.ZeroHash256, receiptRoot, hash.Hash256b([]byte("test")), makeTransferAction(t, 1)),
 				block.ErrDeltaStateMismatch,
 			},
 		}
@@ -254,8 +254,105 @@ func TestWorkingSet_ValidateBlock(t *testing.T) {
 	}
 }
 
-func makeBlock(t *testing.T, nonce uint64, prevHash hash.Hash256, receiptRoot hash.Hash256, digest hash.Hash256) *block.Block {
-	var sevlps []action.SealedEnvelope
+func TestWorkingSet_ValidateBlock_SystemAction(t *testing.T) {
+	require := require.New(t)
+	cfg := Config{
+		Chain:   blockchain.DefaultConfig,
+		Genesis: genesis.TestDefault(),
+	}
+	cfg.Genesis.ToBeEnabledBlockHeight = 1 // enable validate system action
+	cfg.Genesis.InitBalanceMap[identityset.Address(28).String()] = "100000000"
+	registry := protocol.NewRegistry()
+	require.NoError(account.NewProtocol(rewarding.DepositGas).Register(registry))
+	require.NoError(rewarding.NewProtocol(cfg.Genesis.Rewarding).Register(registry))
+	var (
+		f1, _     = NewFactory(cfg, db.NewMemKVStore(), RegistryOption(registry))
+		f2, _     = NewStateDB(cfg, db.NewMemKVStore(), RegistryStateDBOption(registry))
+		factories = []Factory{f1, f2}
+	)
+
+	ctx := protocol.WithBlockCtx(
+		genesis.WithGenesisContext(context.Background(), cfg.Genesis),
+		protocol.BlockCtx{},
+	)
+	require.NoError(f1.Start(ctx))
+	require.NoError(f2.Start(ctx))
+	defer func() {
+		require.NoError(f1.Stop(ctx))
+		require.NoError(f2.Stop(ctx))
+	}()
+
+	zctx := protocol.WithBlockCtx(context.Background(),
+		protocol.BlockCtx{
+			BlockHeight: uint64(1),
+			Producer:    identityset.Address(27),
+			GasLimit:    testutil.TestGasLimit * 100000,
+		})
+	zctx = genesis.WithGenesisContext(zctx, cfg.Genesis)
+	zctx = protocol.WithFeatureCtx(protocol.WithBlockchainCtx(zctx, protocol.BlockchainCtx{
+		ChainID: 1,
+	}))
+
+	t.Run("missing system action", func(t *testing.T) {
+		digestHash, err := hash.HexStringToHash256("8f9b7694c325a4f4b0065cd382f8af0a4e913113a4ce7ef1ac899f96158c74f4")
+		require.NoError(err)
+		receiptRoot, err := hash.HexStringToHash256("f04673451e31386a8fddfcf7750665bfcf33f239f6c4919430bb11a144e1aa95")
+		require.NoError(err)
+		actions := []action.SealedEnvelope{makeTransferAction(t, 1)}
+		for _, f := range factories {
+			block := makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, actions...)
+			require.ErrorIs(f.Validate(zctx, block), errInvalidSystemActionLayout)
+		}
+	})
+	t.Run("system action not on tail", func(t *testing.T) {
+		digestHash, err := hash.HexStringToHash256("8f9b7694c325a4f4b0065cd382f8af0a4e913113a4ce7ef1ac899f96158c74f4")
+		require.NoError(err)
+		receiptRoot, err := hash.HexStringToHash256("f04673451e31386a8fddfcf7750665bfcf33f239f6c4919430bb11a144e1aa95")
+		require.NoError(err)
+		actions := []action.SealedEnvelope{makeRewardAction(t), makeTransferAction(t, 1)}
+		for _, f := range factories {
+			block := makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, actions...)
+			require.ErrorIs(f.Validate(zctx, block), errInvalidSystemActionLayout)
+		}
+	})
+	t.Run("correct system action", func(t *testing.T) {
+		digestHash, err := hash.HexStringToHash256("ade24a5c647b5af34c4e74fe0d8f1fa410f6fb115f8fc2d39e45ca2f895de9ca")
+		require.NoError(err)
+		receiptRoot, err := hash.HexStringToHash256("0aa4c3109e75d3da8c6b1cb51720a769d7a3a8c735247063cede8e3ecf90ed62")
+		require.NoError(err)
+		actions := []action.SealedEnvelope{makeTransferAction(t, 1), makeRewardAction(t)}
+		for _, f := range factories {
+			block := makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, actions...)
+			require.ErrorIs(f.Validate(zctx, block), nil)
+		}
+	})
+	t.Run("postiche system action", func(t *testing.T) {
+		digestHash, err := hash.HexStringToHash256("ade24a5c647b5af34c4e74fe0d8f1fa410f6fb115f8fc2d39e45ca2f895de9ca")
+		require.NoError(err)
+		receiptRoot, err := hash.HexStringToHash256("0aa4c3109e75d3da8c6b1cb51720a769d7a3a8c735247063cede8e3ecf90ed62")
+		require.NoError(err)
+		actions := []action.SealedEnvelope{makeTransferAction(t, 1), makeRewardAction(t), makeRewardAction(t)}
+		for _, f := range factories {
+			block := makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, actions...)
+			require.ErrorIs(f.Validate(zctx, block), errInvalidSystemActionLayout)
+		}
+	})
+	t.Run("inconsistent system action", func(t *testing.T) {
+		digestHash, err := hash.HexStringToHash256("8f9b7694c325a4f4b0065cd382f8af0a4e913113a4ce7ef1ac899f96158c74f4")
+		require.NoError(err)
+		receiptRoot, err := hash.HexStringToHash256("f04673451e31386a8fddfcf7750665bfcf33f239f6c4919430bb11a144e1aa95")
+		require.NoError(err)
+		rewardAct := makeRewardAction(t)
+		rewardAct.SetNonce(2)
+		actions := []action.SealedEnvelope{makeTransferAction(t, 1), rewardAct}
+		for _, f := range factories {
+			block := makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, actions...)
+			require.ErrorIs(f.Validate(zctx, block), errInvalidSystemActionLayout)
+		}
+	})
+}
+
+func makeTransferAction(t *testing.T, nonce uint64) action.SealedEnvelope {
 	tsf, err := action.NewTransfer(
 		uint64(nonce),
 		big.NewInt(1),
@@ -275,9 +372,26 @@ func makeBlock(t *testing.T, nonce uint64, prevHash hash.Hash256, receiptRoot ha
 		Build()
 	sevlp, err := action.Sign(evlp, identityset.PrivateKey(28))
 	require.NoError(t, err)
-	sevlps = append(sevlps, sevlp)
+	return sevlp
+}
+
+func makeRewardAction(t *testing.T) action.SealedEnvelope {
+	gb := action.GrantRewardBuilder{}
+	grant := gb.SetRewardType(action.BlockReward).SetHeight(1).Build()
+	eb2 := action.EnvelopeBuilder{}
+	evlp := eb2.SetNonce(0).
+		SetGasPrice(big.NewInt(0)).
+		SetGasLimit(grant.GasLimit()).
+		SetAction(&grant).
+		Build()
+	sevlp, err := action.Sign(evlp, identityset.PrivateKey(28))
+	require.NoError(t, err)
+	return sevlp
+}
+
+func makeBlock(t *testing.T, prevHash hash.Hash256, receiptRoot hash.Hash256, digest hash.Hash256, actions ...action.SealedEnvelope) *block.Block {
 	rap := block.RunnableActionsBuilder{}
-	ra := rap.AddActions(sevlps...).Build()
+	ra := rap.AddActions(actions...).Build()
 	blk, err := block.NewBuilder(ra).
 		SetHeight(1).
 		SetTimestamp(time.Now()).