Merge pull request #3698 from filecoin-project/fix/mpool-packing-failure

fix mpool optimal selection packing failure

chain/messagepool/messagepool_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/filecoin-project/go-address"
+	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/crypto"
 	"github.com/filecoin-project/lotus/chain/messagepool/gasguess"
 	"github.com/filecoin-project/lotus/chain/types"
@@ -56,6 +57,13 @@ func (tma *testMpoolAPI) nextBlock() *types.BlockHeader {
 	return newBlk
 }
 
+func (tma *testMpoolAPI) nextBlockWithHeight(height uint64) *types.BlockHeader {
+	newBlk := mock.MkBlock(tma.tipsets[len(tma.tipsets)-1], 1, 1)
+	newBlk.Height = abi.ChainEpoch(height)
+	tma.tipsets = append(tma.tipsets, mock.TipSet(newBlk))
+	return newBlk
+}
+
 func (tma *testMpoolAPI) applyBlock(t *testing.T, b *types.BlockHeader) {
 	t.Helper()
 	if err := tma.cb(nil, []*types.TipSet{mock.TipSet(b)}); err != nil {

chain/messagepool/selection.go

@@ -199,9 +199,11 @@ func (mp *MessagePool) selectMessagesOptimal(curTs, ts *types.TipSet, tq float64
 			gasLimit -= chainGasLimit
 
 			// resort to account for already merged chains and effective performance adjustments
-			sort.Slice(chains[i+1:], func(i, j int) bool {
+			// the sort *must* be stable or we end up getting negative gasPerfs pushed up.
+			sort.SliceStable(chains[i+1:], func(i, j int) bool {
 				return chains[i].BeforeEffective(chains[j])
 			})
+
 			continue
 		}
 
@@ -912,7 +914,9 @@ func (mc *msgChain) SetNullEffectivePerf() {
 
 func (mc *msgChain) BeforeEffective(other *msgChain) bool {
 	// move merged chains to the front so we can discard them earlier
-	return (mc.merged && !other.merged) || mc.effPerf > other.effPerf ||
+	return (mc.merged && !other.merged) ||
+		(mc.gasPerf >= 0 && other.gasPerf < 0) ||
+		mc.effPerf > other.effPerf ||
 		(mc.effPerf == other.effPerf && mc.gasPerf > other.gasPerf) ||
 		(mc.effPerf == other.effPerf && mc.gasPerf == other.gasPerf && mc.gasReward.Cmp(other.gasReward) > 0)
 }

chain/messagepool/selection_test.go

@@ -1,11 +1,16 @@
 package messagepool
 
 import (
+	"compress/gzip"
 	"context"
+	"encoding/json"
 	"fmt"
+	"io"
 	"math"
 	"math/big"
 	"math/rand"
+	"os"
+	"sort"
 	"testing"
 
 	"github.com/filecoin-project/go-address"
@@ -1281,3 +1286,177 @@ func TestGasReward(t *testing.T) {
 		})
 	}
 }
+
+func TestRealWorldSelection(t *testing.T) {
+	// load test-messages.json.gz and rewrite the messages so that
+	// 1) we map each real actor to a test actor so that we can sign the messages
+	// 2) adjust the nonces so that they start from 0
+	file, err := os.Open("test-messages.json.gz")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gzr, err := gzip.NewReader(file)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dec := json.NewDecoder(gzr)
+
+	var msgs []*types.SignedMessage
+	baseNonces := make(map[address.Address]uint64)
+
+readLoop:
+	for {
+		m := new(types.SignedMessage)
+		err := dec.Decode(m)
+		switch err {
+		case nil:
+			msgs = append(msgs, m)
+			nonce, ok := baseNonces[m.Message.From]
+			if !ok || m.Message.Nonce < nonce {
+				baseNonces[m.Message.From] = m.Message.Nonce
+			}
+
+		case io.EOF:
+			break readLoop
+
+		default:
+			t.Fatal(err)
+		}
+	}
+
+	actorMap := make(map[address.Address]address.Address)
+	actorWallets := make(map[address.Address]*wallet.Wallet)
+
+	for _, m := range msgs {
+		baseNonce := baseNonces[m.Message.From]
+
+		localActor, ok := actorMap[m.Message.From]
+		if !ok {
+			w, err := wallet.NewWallet(wallet.NewMemKeyStore())
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			a, err := w.GenerateKey(crypto.SigTypeSecp256k1)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			actorMap[m.Message.From] = a
+			actorWallets[a] = w
+			localActor = a
+		}
+
+		w, ok := actorWallets[localActor]
+		if !ok {
+			t.Fatalf("failed to lookup wallet for actor %s", localActor)
+		}
+
+		m.Message.From = localActor
+		m.Message.Nonce -= baseNonce
+
+		sig, err := w.Sign(context.TODO(), localActor, m.Message.Cid().Bytes())
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		m.Signature = *sig
+	}
+
+	mp, tma := makeTestMpool()
+
+	block := tma.nextBlockWithHeight(build.UpgradeBreezeHeight + 10)
+	ts := mock.TipSet(block)
+	tma.applyBlock(t, block)
+
+	for _, a := range actorMap {
+		tma.setBalance(a, 1000000)
+	}
+
+	tma.baseFee = types.NewInt(800_000_000)
+
+	sort.Slice(msgs, func(i, j int) bool {
+		return msgs[i].Message.Nonce < msgs[j].Message.Nonce
+	})
+
+	// add the messages
+	for _, m := range msgs {
+		mustAdd(t, mp, m)
+	}
+
+	// do message selection and check block packing
+	minGasLimit := int64(0.9 * float64(build.BlockGasLimit))
+
+	// greedy first
+	selected, err := mp.SelectMessages(ts, 1.0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gasLimit := int64(0)
+	for _, m := range selected {
+		gasLimit += m.Message.GasLimit
+	}
+	if gasLimit < minGasLimit {
+		t.Fatalf("failed to pack with tq=1.0; packed %d, minimum packing: %d", gasLimit, minGasLimit)
+	}
+
+	// high quality ticket
+	selected, err = mp.SelectMessages(ts, .8)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gasLimit = int64(0)
+	for _, m := range selected {
+		gasLimit += m.Message.GasLimit
+	}
+	if gasLimit < minGasLimit {
+		t.Fatalf("failed to pack with tq=0.8; packed %d, minimum packing: %d", gasLimit, minGasLimit)
+	}
+
+	// mid quality ticket
+	selected, err = mp.SelectMessages(ts, .4)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gasLimit = int64(0)
+	for _, m := range selected {
+		gasLimit += m.Message.GasLimit
+	}
+	if gasLimit < minGasLimit {
+		t.Fatalf("failed to pack with tq=0.4; packed %d, minimum packing: %d", gasLimit, minGasLimit)
+	}
+
+	// low quality ticket
+	selected, err = mp.SelectMessages(ts, .1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gasLimit = int64(0)
+	for _, m := range selected {
+		gasLimit += m.Message.GasLimit
+	}
+	if gasLimit < minGasLimit {
+		t.Fatalf("failed to pack with tq=0.1; packed %d, minimum packing: %d", gasLimit, minGasLimit)
+	}
+
+	// very low quality ticket
+	selected, err = mp.SelectMessages(ts, .01)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gasLimit = int64(0)
+	for _, m := range selected {
+		gasLimit += m.Message.GasLimit
+	}
+	if gasLimit < minGasLimit {
+		t.Fatalf("failed to pack with tq=0.01; packed %d, minimum packing: %d", gasLimit, minGasLimit)
+	}
+
+}