From f5ae64bf9408c28f0b59db6e97d9316a28e9f045 Mon Sep 17 00:00:00 2001 From: whyrusleeping Date: Wed, 25 Sep 2019 20:48:53 -0700 Subject: [PATCH 1/3] filter out messages that have insufficient funds from block production --- chain/types/message.go | 7 +++++++ miner/miner.go | 24 ++++++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/chain/types/message.go b/chain/types/message.go index 579329eca..a66671e68 100644 --- a/chain/types/message.go +++ b/chain/types/message.go @@ -66,3 +66,10 @@ func (m *Message) Cid() cid.Cid { return b.Cid() } + +func (m *Message) RequiredFunds() BigInt { + return BigAdd( + m.Value, + BigMul(m.GasPrice, m.GasLimit), + ) +} diff --git a/miner/miner.go b/miner/miner.go index bd659d496..4f841e1ef 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -299,7 +299,10 @@ func (m *Miner) createBlock(base *MiningBase, ticket *types.Ticket, proof types. return nil, errors.Wrapf(err, "failed to get pending messages") } - msgs := m.selectMessages(pending) + msgs, err := m.selectMessages(context.TODO(), base, pending) + if err != nil { + return nil, xerrors.Errorf("message filtering failed: %w", err) + } uts := time.Now().Unix() // TODO: put smallest valid timestamp @@ -307,7 +310,20 @@ func (m *Miner) createBlock(base *MiningBase, ticket *types.Ticket, proof types. return m.api.MinerCreateBlock(context.TODO(), m.addresses[0], base.ts, append(base.tickets, ticket), proof, msgs, uint64(uts)) } -func (m *Miner) selectMessages(msgs []*types.SignedMessage) []*types.SignedMessage { - // TODO: filter and select 'best' message if too many to fit in one block - return msgs +func (m *Miner) selectMessages(ctx context.Context, base *MiningBase, msgs []*types.SignedMessage) ([]*types.SignedMessage, error) { + out := make([]*types.SignedMessage, 0, len(msgs)) + for _, msg := range msgs { + act, err := m.api.StateGetActor(ctx, msg.Message.From, base.ts) + if err != nil { + return nil, xerrors.Errorf("failed to check message sender balance: %w", err) + } + + if act.Balance.LessThan(msg.Message.RequiredFunds()) { + log.Warningf("message in mempool does not have enough funds: %s", msg.Cid()) + continue + } + + out = append(out, msg) + } + return out, nil } From 8337dd52fe70d94d9ffeb1753b73f4d97c988d07 Mon Sep 17 00:00:00 2001 From: whyrusleeping Date: Wed, 25 Sep 2019 20:53:52 -0700 Subject: [PATCH 2/3] handle messages with invalid nonces after balance filtering --- miner/miner.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/miner/miner.go b/miner/miner.go index 4f841e1ef..ba3b33c28 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -312,17 +312,30 @@ func (m *Miner) createBlock(base *MiningBase, ticket *types.Ticket, proof types. func (m *Miner) selectMessages(ctx context.Context, base *MiningBase, msgs []*types.SignedMessage) ([]*types.SignedMessage, error) { out := make([]*types.SignedMessage, 0, len(msgs)) + inclNonces := make(map[address.Address]uint64) for _, msg := range msgs { - act, err := m.api.StateGetActor(ctx, msg.Message.From, base.ts) + from := msg.Message.From + act, err := m.api.StateGetActor(ctx, from, base.ts) if err != nil { return nil, xerrors.Errorf("failed to check message sender balance: %w", err) } + if _, ok := inclNonces[from]; !ok { + inclNonces[from] = act.Nonce + } + if act.Balance.LessThan(msg.Message.RequiredFunds()) { log.Warningf("message in mempool does not have enough funds: %s", msg.Cid()) continue } + if msg.Message.Nonce > inclNonces[from] { + log.Warningf("message in mempool has too high of a nonce: %s", msg.Cid()) + continue + } + + inclNonces[from] = msg.Message.Nonce + out = append(out, msg) } return out, nil From d82a1f8184d90760cb9ddbd085c86cbfbb243ea8 Mon Sep 17 00:00:00 2001 From: whyrusleeping Date: Thu, 26 Sep 2019 13:47:34 -0700 Subject: [PATCH 3/3] fix nonce checking and also check cumulative balance --- chain/types/bigint.go | 10 ++--- miner/miner.go | 24 ++++++++--- miner/miner_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 miner/miner_test.go diff --git a/chain/types/bigint.go b/chain/types/bigint.go index 6a0a875bb..11946fa2a 100644 --- a/chain/types/bigint.go +++ b/chain/types/bigint.go @@ -81,18 +81,18 @@ func BigCmp(a, b BigInt) int { return a.Int.Cmp(b.Int) } -func (bi *BigInt) Nil() bool { +func (bi BigInt) Nil() bool { return bi.Int == nil } // LessThan returns true if bi < o -func (bi *BigInt) LessThan(o BigInt) bool { - return BigCmp(*bi, o) < 0 +func (bi BigInt) LessThan(o BigInt) bool { + return BigCmp(bi, o) < 0 } // LessThan returns true if bi > o -func (bi *BigInt) GreaterThan(o BigInt) bool { - return BigCmp(*bi, o) > 0 +func (bi BigInt) GreaterThan(o BigInt) bool { + return BigCmp(bi, o) > 0 } func (bi *BigInt) MarshalJSON() ([]byte, error) { diff --git a/miner/miner.go b/miner/miner.go index ba3b33c28..df47f3f14 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -299,7 +299,7 @@ func (m *Miner) createBlock(base *MiningBase, ticket *types.Ticket, proof types. return nil, errors.Wrapf(err, "failed to get pending messages") } - msgs, err := m.selectMessages(context.TODO(), base, pending) + msgs, err := selectMessages(context.TODO(), m.api.StateGetActor, base, pending) if err != nil { return nil, xerrors.Errorf("message filtering failed: %w", err) } @@ -310,31 +310,41 @@ func (m *Miner) createBlock(base *MiningBase, ticket *types.Ticket, proof types. return m.api.MinerCreateBlock(context.TODO(), m.addresses[0], base.ts, append(base.tickets, ticket), proof, msgs, uint64(uts)) } -func (m *Miner) selectMessages(ctx context.Context, base *MiningBase, msgs []*types.SignedMessage) ([]*types.SignedMessage, error) { +type actorLookup func(context.Context, address.Address, *types.TipSet) (*types.Actor, error) + +func selectMessages(ctx context.Context, al actorLookup, base *MiningBase, msgs []*types.SignedMessage) ([]*types.SignedMessage, error) { out := make([]*types.SignedMessage, 0, len(msgs)) inclNonces := make(map[address.Address]uint64) + inclBalances := make(map[address.Address]types.BigInt) for _, msg := range msgs { from := msg.Message.From - act, err := m.api.StateGetActor(ctx, from, base.ts) + act, err := al(ctx, from, base.ts) if err != nil { return nil, xerrors.Errorf("failed to check message sender balance: %w", err) } if _, ok := inclNonces[from]; !ok { inclNonces[from] = act.Nonce + inclBalances[from] = act.Balance } - if act.Balance.LessThan(msg.Message.RequiredFunds()) { - log.Warningf("message in mempool does not have enough funds: %s", msg.Cid()) + if inclBalances[from].LessThan(msg.Message.RequiredFunds()) { + log.Warnf("message in mempool does not have enough funds: %s", msg.Cid()) continue } if msg.Message.Nonce > inclNonces[from] { - log.Warningf("message in mempool has too high of a nonce: %s", msg.Cid()) + log.Warnf("message in mempool has too high of a nonce (%d > %d) %s", msg.Message.Nonce, inclNonces[from], msg.Cid()) continue } - inclNonces[from] = msg.Message.Nonce + if msg.Message.Nonce < inclNonces[from] { + log.Warnf("message in mempool has already used nonce (%d < %d) %s", msg.Message.Nonce, inclNonces[from], msg.Cid()) + continue + } + + inclNonces[from] = msg.Message.Nonce + 1 + inclBalances[from] = types.BigSub(inclBalances[from], msg.Message.RequiredFunds()) out = append(out, msg) } diff --git a/miner/miner_test.go b/miner/miner_test.go new file mode 100644 index 000000000..dc79bf129 --- /dev/null +++ b/miner/miner_test.go @@ -0,0 +1,99 @@ +package miner + +import ( + "context" + "testing" + + "github.com/filecoin-project/go-lotus/chain/address" + "github.com/filecoin-project/go-lotus/chain/types" +) + +func mustIDAddr(i uint64) address.Address { + a, err := address.NewIDAddress(i) + if err != nil { + panic(err) + } + + return a +} + +func TestMessageFiltering(t *testing.T) { + ctx := context.TODO() + a1 := mustIDAddr(1) + a2 := mustIDAddr(2) + + actors := map[address.Address]*types.Actor{ + a1: &types.Actor{ + Nonce: 3, + Balance: types.NewInt(1200), + }, + a2: &types.Actor{ + Nonce: 1, + Balance: types.NewInt(1000), + }, + } + + af := func(ctx context.Context, addr address.Address, ts *types.TipSet) (*types.Actor, error) { + return actors[addr], nil + } + + msgs := []types.Message{ + types.Message{ + From: a1, + Nonce: 3, + Value: types.NewInt(500), + GasLimit: types.NewInt(50), + GasPrice: types.NewInt(1), + }, + types.Message{ + From: a1, + Nonce: 4, + Value: types.NewInt(500), + GasLimit: types.NewInt(50), + GasPrice: types.NewInt(1), + }, + types.Message{ + From: a2, + Nonce: 1, + Value: types.NewInt(800), + GasLimit: types.NewInt(100), + GasPrice: types.NewInt(1), + }, + types.Message{ + From: a2, + Nonce: 0, + Value: types.NewInt(800), + GasLimit: types.NewInt(100), + GasPrice: types.NewInt(1), + }, + types.Message{ + From: a2, + Nonce: 2, + Value: types.NewInt(150), + GasLimit: types.NewInt(100), + GasPrice: types.NewInt(1), + }, + } + + outmsgs, err := selectMessages(ctx, af, &MiningBase{}, wrapMsgs(msgs)) + if err != nil { + t.Fatal(err) + } + + if len(outmsgs) != 3 { + t.Fatal("filtering didnt work as expected") + } + + m1 := outmsgs[2].Message + if m1.From != msgs[2].From || m1.Nonce != msgs[2].Nonce { + t.Fatal("filtering bad") + } +} + +func wrapMsgs(msgs []types.Message) []*types.SignedMessage { + var out []*types.SignedMessage + for _, m := range msgs { + out = append(out, &types.SignedMessage{Message: m}) + } + return out +}