From 1c0e6d76f0da61aaf955dd5824af55c9d910a152 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sat, 5 Sep 2020 20:09:46 +0300 Subject: [PATCH 1/4] add defensive check for minimum GasFeeCap for inclusion within the next 20 blocks --- chain/messagepool/messagepool.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/chain/messagepool/messagepool.go b/chain/messagepool/messagepool.go index b20bc8b91a8..68924480b42 100644 --- a/chain/messagepool/messagepool.go +++ b/chain/messagepool/messagepool.go @@ -11,7 +11,6 @@ import ( "sync" "time" - "github.com/filecoin-project/specs-actors/actors/abi" "github.com/filecoin-project/specs-actors/actors/crypto" "github.com/hashicorp/go-multierror" lru "github.com/hashicorp/golang-lru" @@ -49,6 +48,7 @@ const RbfDenom = 256 var RepublishInterval = pubsub.TimeCacheDuration + time.Duration(5*build.BlockDelaySecs+build.PropagationDelaySecs)*time.Second var minimumBaseFee = types.NewInt(uint64(build.MinimumBaseFee)) +var baseFeeLowerBoundFactor = types.NewInt(10) var MaxActorPendingMessages = 1000 @@ -355,12 +355,30 @@ func (mp *MessagePool) addLocal(m *types.SignedMessage, msgb []byte) error { return nil } -func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, epoch abi.ChainEpoch) error { +func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, curTs *types.TipSet) error { + epoch := curTs.Height() minGas := vm.PricelistByEpoch(epoch).OnChainMessage(m.ChainLength()) if err := m.VMMessage().ValidForBlockInclusion(minGas.Total()); err != nil { return xerrors.Errorf("message will not be included in a block: %w", err) } + + // this checks if the GasFeeCap is suffisciently high for inclusion in the next 20 blocks + // if the GasFeeCap is too low, we soft reject the message (Ignore in pubsub) and rely + // on republish to push it through later, if the baseFee has fallen. + // this is a defensive check that stops minimum baseFee spam attacks from overloading validation + // queues. + baseFee, err := mp.api.ChainComputeBaseFee(context.TODO(), curTs) + if err != nil { + return xerrors.Errorf("error computing base fee: %w", err) + } + + baseFeeLowerBound := types.BigDiv(baseFee, baseFeeLowerBoundFactor) + if m.Message.GasFeeCap.LessThan(baseFeeLowerBound) { + return xerrors.Errorf("GasFeeCap doesn't meet base fee lower bound for inclusion in the next 20 blocks (GasFeeCap: %s, baseFeeLowerBound: %s): %w", + m.Message.GasFeeCap, baseFeeLowerBound, ErrSoftValidationFailure) + } + return nil } @@ -523,7 +541,7 @@ func (mp *MessagePool) addTs(m *types.SignedMessage, curTs *types.TipSet) error mp.lk.Lock() defer mp.lk.Unlock() - if err := mp.verifyMsgBeforeAdd(m, curTs.Height()); err != nil { + if err := mp.verifyMsgBeforeAdd(m, curTs); err != nil { return err } @@ -557,7 +575,7 @@ func (mp *MessagePool) addLoaded(m *types.SignedMessage) error { mp.lk.Lock() defer mp.lk.Unlock() - if err := mp.verifyMsgBeforeAdd(m, curTs.Height()); err != nil { + if err := mp.verifyMsgBeforeAdd(m, curTs); err != nil { return err } @@ -743,7 +761,7 @@ func (mp *MessagePool) PushWithNonce(ctx context.Context, addr address.Address, return nil, ErrTryAgain } - if err := mp.verifyMsgBeforeAdd(msg, curTs.Height()); err != nil { + if err := mp.verifyMsgBeforeAdd(msg, curTs); err != nil { return nil, err } From f9492691a6ef91519709f3f792c687fe933f399a Mon Sep 17 00:00:00 2001 From: vyzo Date: Sat, 5 Sep 2020 20:31:16 +0300 Subject: [PATCH 2/4] don't check baseFee lower bound for local messages --- chain/messagepool/messagepool.go | 34 ++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/chain/messagepool/messagepool.go b/chain/messagepool/messagepool.go index 68924480b42..173c72e8e72 100644 --- a/chain/messagepool/messagepool.go +++ b/chain/messagepool/messagepool.go @@ -355,7 +355,7 @@ func (mp *MessagePool) addLocal(m *types.SignedMessage, msgb []byte) error { return nil } -func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, curTs *types.TipSet) error { +func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, curTs *types.TipSet, local bool) error { epoch := curTs.Height() minGas := vm.PricelistByEpoch(epoch).OnChainMessage(m.ChainLength()) @@ -368,15 +368,19 @@ func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, curTs *types.T // on republish to push it through later, if the baseFee has fallen. // this is a defensive check that stops minimum baseFee spam attacks from overloading validation // queues. - baseFee, err := mp.api.ChainComputeBaseFee(context.TODO(), curTs) - if err != nil { - return xerrors.Errorf("error computing base fee: %w", err) - } + // Note that we don't do that for local messages, so that they can be accepted and republished + // automatically + if !local { + baseFee, err := mp.api.ChainComputeBaseFee(context.TODO(), curTs) + if err != nil { + return xerrors.Errorf("error computing base fee: %w", err) + } - baseFeeLowerBound := types.BigDiv(baseFee, baseFeeLowerBoundFactor) - if m.Message.GasFeeCap.LessThan(baseFeeLowerBound) { - return xerrors.Errorf("GasFeeCap doesn't meet base fee lower bound for inclusion in the next 20 blocks (GasFeeCap: %s, baseFeeLowerBound: %s): %w", - m.Message.GasFeeCap, baseFeeLowerBound, ErrSoftValidationFailure) + baseFeeLowerBound := types.BigDiv(baseFee, baseFeeLowerBoundFactor) + if m.Message.GasFeeCap.LessThan(baseFeeLowerBound) { + return xerrors.Errorf("GasFeeCap doesn't meet base fee lower bound for inclusion in the next 20 blocks (GasFeeCap: %s, baseFeeLowerBound: %s): %w", + m.Message.GasFeeCap, baseFeeLowerBound, ErrSoftValidationFailure) + } } return nil @@ -400,7 +404,7 @@ func (mp *MessagePool) Push(m *types.SignedMessage) (cid.Cid, error) { } mp.curTsLk.Lock() - if err := mp.addTs(m, mp.curTs); err != nil { + if err := mp.addTs(m, mp.curTs, true); err != nil { mp.curTsLk.Unlock() return cid.Undef, err } @@ -461,7 +465,7 @@ func (mp *MessagePool) Add(m *types.SignedMessage) error { mp.curTsLk.Lock() defer mp.curTsLk.Unlock() - return mp.addTs(m, mp.curTs) + return mp.addTs(m, mp.curTs, false) } func sigCacheKey(m *types.SignedMessage) (string, error) { @@ -528,7 +532,7 @@ func (mp *MessagePool) checkBalance(m *types.SignedMessage, curTs *types.TipSet) return nil } -func (mp *MessagePool) addTs(m *types.SignedMessage, curTs *types.TipSet) error { +func (mp *MessagePool) addTs(m *types.SignedMessage, curTs *types.TipSet, local bool) error { snonce, err := mp.getStateNonce(m.Message.From, curTs) if err != nil { return xerrors.Errorf("failed to look up actor state nonce: %s: %w", err, ErrSoftValidationFailure) @@ -541,7 +545,7 @@ func (mp *MessagePool) addTs(m *types.SignedMessage, curTs *types.TipSet) error mp.lk.Lock() defer mp.lk.Unlock() - if err := mp.verifyMsgBeforeAdd(m, curTs); err != nil { + if err := mp.verifyMsgBeforeAdd(m, curTs, local); err != nil { return err } @@ -575,7 +579,7 @@ func (mp *MessagePool) addLoaded(m *types.SignedMessage) error { mp.lk.Lock() defer mp.lk.Unlock() - if err := mp.verifyMsgBeforeAdd(m, curTs); err != nil { + if err := mp.verifyMsgBeforeAdd(m, curTs, true); err != nil { return err } @@ -761,7 +765,7 @@ func (mp *MessagePool) PushWithNonce(ctx context.Context, addr address.Address, return nil, ErrTryAgain } - if err := mp.verifyMsgBeforeAdd(msg, curTs); err != nil { + if err := mp.verifyMsgBeforeAdd(msg, curTs, true); err != nil { return nil, err } From ffb2640736b338f1359ee18afb58c51cd0bcec84 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sat, 5 Sep 2020 21:03:22 +0300 Subject: [PATCH 3/4] use faster lookup for base fee --- chain/messagepool/messagepool.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/chain/messagepool/messagepool.go b/chain/messagepool/messagepool.go index 173c72e8e72..0d62e5423f2 100644 --- a/chain/messagepool/messagepool.go +++ b/chain/messagepool/messagepool.go @@ -370,12 +370,8 @@ func (mp *MessagePool) verifyMsgBeforeAdd(m *types.SignedMessage, curTs *types.T // queues. // Note that we don't do that for local messages, so that they can be accepted and republished // automatically - if !local { - baseFee, err := mp.api.ChainComputeBaseFee(context.TODO(), curTs) - if err != nil { - return xerrors.Errorf("error computing base fee: %w", err) - } - + if !local && len(curTs.Blocks()) > 0 { + baseFee := curTs.Blocks()[0].ParentBaseFee baseFeeLowerBound := types.BigDiv(baseFee, baseFeeLowerBoundFactor) if m.Message.GasFeeCap.LessThan(baseFeeLowerBound) { return xerrors.Errorf("GasFeeCap doesn't meet base fee lower bound for inclusion in the next 20 blocks (GasFeeCap: %s, baseFeeLowerBound: %s): %w", From e24a29b1460f9a11a11cb4ba5c24b4cf58eab194 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sat, 5 Sep 2020 21:26:21 +0300 Subject: [PATCH 4/4] fix tests that touch the mpool; mock block must have a ParentBaseFee --- chain/types/mock/chain.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/chain/types/mock/chain.go b/chain/types/mock/chain.go index 33b13d4086a..b535f203ac6 100644 --- a/chain/types/mock/chain.go +++ b/chain/types/mock/chain.go @@ -9,6 +9,7 @@ import ( "github.com/filecoin-project/specs-actors/actors/crypto" "github.com/ipfs/go-cid" + "github.com/filecoin-project/lotus/build" "github.com/filecoin-project/lotus/chain/types" "github.com/filecoin-project/lotus/chain/wallet" ) @@ -80,6 +81,7 @@ func MkBlock(parents *types.TipSet, weightInc uint64, ticketNonce uint64) *types Height: height, ParentStateRoot: pstateRoot, BlockSig: &crypto.Signature{Type: crypto.SigTypeBLS, Data: []byte("boo! im a signature")}, + ParentBaseFee: types.NewInt(uint64(build.MinimumBaseFee)), } }