From 0b3685e5299c24b6e5112bcb181b6de0ad63250a Mon Sep 17 00:00:00 2001 From: vyzo Date: Sat, 5 Sep 2020 21:30:02 +0300 Subject: [PATCH 1/2] fix selection bug; priority messages were not included if other's chains were negative --- chain/messagepool/selection.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chain/messagepool/selection.go b/chain/messagepool/selection.go index 5ba679d76..df1d7b801 100644 --- a/chain/messagepool/selection.go +++ b/chain/messagepool/selection.go @@ -102,7 +102,7 @@ func (mp *MessagePool) selectMessagesOptimal(curTs, ts *types.TipSet, tq float64 if len(chains) != 0 && chains[0].gasPerf < 0 { log.Warnw("all messages in mpool have non-positive gas performance", "bestGasPerf", chains[0].gasPerf) - return nil, nil + return result, nil } // 3. Parition chains into blocks (without trimming) @@ -351,7 +351,7 @@ func (mp *MessagePool) selectMessagesGreedy(curTs, ts *types.TipSet) ([]*types.S if len(chains) != 0 && chains[0].gasPerf < 0 { log.Warnw("all messages in mpool have non-positive gas performance", "bestGasPerf", chains[0].gasPerf) - return nil, nil + return result, nil } // 3. Merge the head chains to produce the list of messages selected for inclusion, subject to From 5037282b984c0eef90f59852f01f17262244a8e0 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sat, 5 Sep 2020 22:15:18 +0300 Subject: [PATCH 2/2] add test for priority selection with negative chains --- chain/messagepool/messagepool_test.go | 5 +- chain/messagepool/selection_test.go | 94 +++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/chain/messagepool/messagepool_test.go b/chain/messagepool/messagepool_test.go index 484c72746..c97c03166 100644 --- a/chain/messagepool/messagepool_test.go +++ b/chain/messagepool/messagepool_test.go @@ -34,6 +34,8 @@ type testMpoolAPI struct { tipsets []*types.TipSet published int + + baseFee types.BigInt } func newTestMpoolAPI() *testMpoolAPI { @@ -41,6 +43,7 @@ func newTestMpoolAPI() *testMpoolAPI { bmsgs: make(map[cid.Cid][]*types.SignedMessage), statenonce: make(map[address.Address]uint64), balance: make(map[address.Address]types.BigInt), + baseFee: types.NewInt(100), } genesis := mock.MkBlock(nil, 1, 1) tma.tipsets = append(tma.tipsets, mock.TipSet(genesis)) @@ -182,7 +185,7 @@ func (tma *testMpoolAPI) LoadTipSet(tsk types.TipSetKey) (*types.TipSet, error) } func (tma *testMpoolAPI) ChainComputeBaseFee(ctx context.Context, ts *types.TipSet) (types.BigInt, error) { - return types.NewInt(100), nil + return tma.baseFee, nil } func assertNonce(t *testing.T, mp *MessagePool, addr address.Address, val uint64) { diff --git a/chain/messagepool/selection_test.go b/chain/messagepool/selection_test.go index a9ead3c01..d9ed3af9c 100644 --- a/chain/messagepool/selection_test.go +++ b/chain/messagepool/selection_test.go @@ -728,6 +728,100 @@ func TestPriorityMessageSelection2(t *testing.T) { } } +func TestPriorityMessageSelection3(t *testing.T) { + mp, tma := makeTestMpool() + + // the actors + w1, err := wallet.NewWallet(wallet.NewMemKeyStore()) + if err != nil { + t.Fatal(err) + } + + a1, err := w1.GenerateKey(crypto.SigTypeSecp256k1) + if err != nil { + t.Fatal(err) + } + + w2, err := wallet.NewWallet(wallet.NewMemKeyStore()) + if err != nil { + t.Fatal(err) + } + + a2, err := w2.GenerateKey(crypto.SigTypeSecp256k1) + if err != nil { + t.Fatal(err) + } + + block := tma.nextBlock() + ts := mock.TipSet(block) + tma.applyBlock(t, block) + + gasLimit := gasguess.Costs[gasguess.CostKey{Code: builtin.StorageMarketActorCodeID, M: 2}] + + tma.setBalance(a1, 1) // in FIL + tma.setBalance(a2, 1) // in FIL + + mp.cfg.PriorityAddrs = []address.Address{a1} + + tma.baseFee = types.NewInt(1000) + nMessages := 10 + for i := 0; i < nMessages; i++ { + bias := (nMessages - i) / 3 + m := makeTestMessage(w1, a1, a2, uint64(i), gasLimit, uint64(1000+i%3+bias)) + mustAdd(t, mp, m) + // messages from a2 have negative performance + m = makeTestMessage(w2, a2, a1, uint64(i), gasLimit, 100) + mustAdd(t, mp, m) + } + + // test greedy selection + msgs, err := mp.SelectMessages(ts, 1.0) + if err != nil { + t.Fatal(err) + } + + expectedMsgs := 10 + if len(msgs) != expectedMsgs { + t.Fatalf("expected %d messages but got %d", expectedMsgs, len(msgs)) + } + + // all messages must be from a1 + nextNonce := uint64(0) + for _, m := range msgs { + if m.Message.From != a1 { + t.Fatal("expected messages from a1 before messages from a2") + } + if m.Message.Nonce != nextNonce { + t.Fatalf("expected nonce %d but got %d", nextNonce, m.Message.Nonce) + } + nextNonce++ + } + + // test optimal selection + msgs, err = mp.SelectMessages(ts, 0.1) + if err != nil { + t.Fatal(err) + } + + expectedMsgs = 10 + if len(msgs) != expectedMsgs { + t.Fatalf("expected %d messages but got %d", expectedMsgs, len(msgs)) + } + + // all messages must be from a1 + nextNonce = uint64(0) + for _, m := range msgs { + if m.Message.From != a1 { + t.Fatal("expected messages from a1 before messages from a2") + } + if m.Message.Nonce != nextNonce { + t.Fatalf("expected nonce %d but got %d", nextNonce, m.Message.Nonce) + } + nextNonce++ + } + +} + func TestOptimalMessageSelection1(t *testing.T) { // this test uses just a single actor sending messages with a low tq // the chain depenent merging algorithm should pick messages from the actor