Merge pull request #1743 from filecoin-project/fix/message-validation-checks

reuse logic between message selection and validation

chain/sync.go

@@ -31,7 +31,6 @@ import (
 	amt "github.com/filecoin-project/go-amt-ipld/v2"
 	"github.com/filecoin-project/sector-storage/ffiwrapper"
 	"github.com/filecoin-project/specs-actors/actors/abi"
-	"github.com/filecoin-project/specs-actors/actors/abi/big"
 	"github.com/filecoin-project/specs-actors/actors/builtin"
 	"github.com/filecoin-project/specs-actors/actors/builtin/power"
 	"github.com/filecoin-project/specs-actors/actors/crypto"
@@ -844,38 +843,9 @@ func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock
 
 	checkMsg := func(m *types.Message) error {
 		// Phase 1: syntactic validation, as defined in the spec
-
-		if m.Version != 0 {
-			return xerrors.New("'Version' unsupported")
-		}
-
-		if m.To == address.Undef {
-			return xerrors.New("'To' address cannot be empty")
-		}
-
-		if m.From == address.Undef {
-			return xerrors.New("'From' address cannot be empty")
-		}
-
-		if m.Value.LessThan(big.Zero()) {
-			return xerrors.New("'Value' field cannot be negative")
-		}
-
-		if m.Value.GreaterThan(types.TotalFilecoinInt) {
-			return xerrors.New("'Value' field cannot be greater than total filecoin supply")
-		}
-
-		if m.GasPrice.LessThan(big.Zero()) {
-			return xerrors.New("'GasPrice' field cannot be negative")
-		}
-
-		if m.GasLimit > build.BlockGasLimit {
-			return xerrors.New("'GasLimit' field cannot be greater than a block's gas limit")
-		}
-
-		// since prices might vary with time, this is technically semantic validation
-		if m.GasLimit < vm.PricelistByEpoch(baseTs.Height()).OnChainMessage(m.ChainLength()) {
-			return xerrors.New("'GasLimit' field cannot be less than the cost of storing a message on chain")
+		minGas := vm.PricelistByEpoch(baseTs.Height()).OnChainMessage(m.ChainLength())
+		if err := m.ValidForBlockInclusion(minGas); err != nil {
+			return err
 		}
 
 		// Phase 2: (Partial) semantic validation:

chain/types/message.go

@@ -4,10 +4,13 @@ import (
 	"bytes"
 	"fmt"
 
+	"github.com/filecoin-project/lotus/build"
 	"github.com/filecoin-project/specs-actors/actors/abi"
+	"github.com/filecoin-project/specs-actors/actors/abi/big"
 	block "github.com/ipfs/go-block-format"
 	"github.com/ipfs/go-cid"
 	"github.com/multiformats/go-multihash"
+	xerrors "golang.org/x/xerrors"
 
 	"github.com/filecoin-project/go-address"
 )
@@ -121,3 +124,40 @@ func (m *Message) VMMessage() *Message {
 func (m *Message) Equals(o *Message) bool {
 	return m.Cid() == o.Cid()
 }
+
+func (m *Message) ValidForBlockInclusion(minGas int64) error {
+	if m.Version != 0 {
+		return xerrors.New("'Version' unsupported")
+	}
+
+	if m.To == address.Undef {
+		return xerrors.New("'To' address cannot be empty")
+	}
+
+	if m.From == address.Undef {
+		return xerrors.New("'From' address cannot be empty")
+	}
+
+	if m.Value.LessThan(big.Zero()) {
+		return xerrors.New("'Value' field cannot be negative")
+	}
+
+	if m.Value.GreaterThan(TotalFilecoinInt) {
+		return xerrors.New("'Value' field cannot be greater than total filecoin supply")
+	}
+
+	if m.GasPrice.LessThan(big.Zero()) {
+		return xerrors.New("'GasPrice' field cannot be negative")
+	}
+
+	if m.GasLimit > build.BlockGasLimit {
+		return xerrors.New("'GasLimit' field cannot be greater than a block's gas limit")
+	}
+
+	// since prices might vary with time, this is technically semantic validation
+	if m.GasLimit < minGas {
+		return xerrors.New("'GasLimit' field cannot be less than the cost of storing a message on chain")
+	}
+
+	return nil
+}

miner/miner.go

@@ -18,6 +18,7 @@ import (
 	"github.com/filecoin-project/lotus/chain/gen"
 	"github.com/filecoin-project/lotus/chain/store"
 	"github.com/filecoin-project/lotus/chain/types"
+	"github.com/filecoin-project/lotus/chain/vm"
 
 	logging "github.com/ipfs/go-log/v2"
 	"go.opencensus.io/trace"
@@ -421,6 +422,12 @@ func SelectMessages(ctx context.Context, al ActorLookup, ts *types.TipSet, msgs
 
 	for _, msg := range msgs {
 
+		minGas := vm.PricelistByEpoch(ts.Height()).OnChainMessage(msg.VMMessage().ChainLength()) // TODO: really should be doing just msg.ChainLength() but the sync side of this code doesnt seem to have access to that
+		if err := msg.VMMessage().ValidForBlockInclusion(minGas); err != nil {
+			log.Warnf("invalid message in message pool: %s", err)
+			continue
+		}
+
 		// TODO: this should be in some more general 'validate message' call
 		if msg.Message.GasLimit > build.BlockGasLimit {
 			log.Warnf("message in mempool had too high of a gas limit (%d)", msg.Message.GasLimit)

miner/miner_test.go

@@ -80,7 +80,7 @@ func TestMessageFiltering(t *testing.T) {
 		},
 	}
 
-	outmsgs, err := SelectMessages(ctx, af, nil, wrapMsgs(msgs))
+	outmsgs, err := SelectMessages(ctx, af, &types.TipSet{}, wrapMsgs(msgs))
 	if err != nil {
 		t.Fatal(err)
 	}