feat: mpool: Reduce minimum replace fee from 1.25x to 1.1x

However, we're leaving the default at 1.25x for backwards compatibility, for now. Also: 1. Actually use the configured replace fee ratio. 2. Store said ratios as percentages instead of floats. 1.25, or 1+1/(2^2), can be represented as a float. 1.1, or 1 + 1/(2 * 5), cannot. fixes #10415
filecoin-project · Mar 8, 2023 · 625df81 · 625df81
1 parent 64b9b53
commit 625df81
Show file tree

Hide file tree

Showing 11 changed files with 103 additions and 19 deletions.
diff --git a/api/docgen/docgen.go b/api/docgen/docgen.go
@@ -401,6 +401,10 @@ func init() {
 		FromBlock: pstring("2301220"),
 		Address:   []ethtypes.EthAddress{ethaddr},
 	})
+
+	percent := types.Percent(123)
+	addExample(percent)
+	addExample(&percent)
 }
 
 func GetAPIType(name, pkg string) (i interface{}, t reflect.Type, permStruct []reflect.Type) {

diff --git a/build/openrpc/full.json.gz b/build/openrpc/full.json.gz
diff --git a/chain/messagepool/config.go b/chain/messagepool/config.go
@@ -13,7 +13,11 @@ import (
 )
 
 var (
-	ReplaceByFeeRatioDefault  = 1.25
+	ReplaceByFeePercentageMinimum types.Percent = 110
+	ReplaceByFeePercentageDefault types.Percent = 125
+)
+
+var (
 	MemPoolSizeLimitHiDefault = 30000
 	MemPoolSizeLimitLoDefault = 20000
 	PruneCooldownDefault      = time.Minute
@@ -60,9 +64,9 @@ func (mp *MessagePool) getConfig() *types.MpoolConfig {
 }
 
 func validateConfg(cfg *types.MpoolConfig) error {
-	if cfg.ReplaceByFeeRatio < ReplaceByFeeRatioDefault {
-		return fmt.Errorf("'ReplaceByFeeRatio' is less than required %f < %f",
-			cfg.ReplaceByFeeRatio, ReplaceByFeeRatioDefault)
+	if cfg.ReplaceByFeeRatio < ReplaceByFeePercentageMinimum {
+		return fmt.Errorf("'ReplaceByFeeRatio' is less than required %s < %s",
+			cfg.ReplaceByFeeRatio, ReplaceByFeePercentageMinimum)
 	}
 	if cfg.GasLimitOverestimation < 1 {
 		return fmt.Errorf("'GasLimitOverestimation' cannot be less than 1")
@@ -91,7 +95,7 @@ func DefaultConfig() *types.MpoolConfig {
 	return &types.MpoolConfig{
 		SizeLimitHigh:          MemPoolSizeLimitHiDefault,
 		SizeLimitLow:           MemPoolSizeLimitLoDefault,
-		ReplaceByFeeRatio:      ReplaceByFeeRatioDefault,
+		ReplaceByFeeRatio:      ReplaceByFeePercentageDefault,
 		PruneCooldown:          PruneCooldownDefault,
 		GasLimitOverestimation: GasLimitOverestimation,
 	}

diff --git a/chain/messagepool/messagepool.go b/chain/messagepool/messagepool.go
@@ -47,10 +47,8 @@ var log = logging.Logger("messagepool")
 
 var futureDebug = false
 
-var rbfNumBig = types.NewInt(uint64((ReplaceByFeeRatioDefault - 1) * RbfDenom))
-var rbfDenomBig = types.NewInt(RbfDenom)
-
-const RbfDenom = 256
+var rbfNumBig = types.NewInt(uint64(ReplaceByFeePercentageMinimum))
+var rbfDenomBig = types.NewInt(100)
 
 var RepublishInterval = time.Duration(10*build.BlockDelaySecs+build.PropagationDelaySecs) * time.Second
 
@@ -197,7 +195,13 @@ func newMsgSet(nonce uint64) *msgSet {
 }
 
 func ComputeMinRBF(curPrem abi.TokenAmount) abi.TokenAmount {
-	minPrice := types.BigAdd(curPrem, types.BigDiv(types.BigMul(curPrem, rbfNumBig), rbfDenomBig))
+	minPrice := types.BigDiv(types.BigMul(curPrem, rbfNumBig), rbfDenomBig)
+	return types.BigAdd(minPrice, types.NewInt(1))
+}
+
+func ComputeRBF(curPrem abi.TokenAmount, replaceByFeeRatio types.Percent) abi.TokenAmount {
+	rbfNumBig := types.NewInt(uint64(replaceByFeeRatio))
+	minPrice := types.BigDiv(types.BigMul(curPrem, rbfNumBig), rbfDenomBig)
 	return types.BigAdd(minPrice, types.NewInt(1))
 }
 

diff --git a/chain/types/mpool.go b/chain/types/mpool.go
@@ -10,7 +10,7 @@ type MpoolConfig struct {
 	PriorityAddrs          []address.Address
 	SizeLimitHigh          int
 	SizeLimitLow           int
-	ReplaceByFeeRatio      float64
+	ReplaceByFeeRatio      Percent
 	PruneCooldown          time.Duration
 	GasLimitOverestimation float64
 }

diff --git a/chain/types/percent.go b/chain/types/percent.go
@@ -0,0 +1,33 @@
+package types
+
+import (
+	"fmt"
+	"math"
+	"strconv"
+
+	"golang.org/x/xerrors"
+)
+
+// Percent stores a signed percentage as an int64. When converted to a string (or json), it's stored
+// as a decimal with two places (e.g., 100% -> 1.00).
+type Percent int64
+
+func (p Percent) String() string {
+	return fmt.Sprintf(`%d.%d`, p/100, p%100)
+}
+
+func (p Percent) MarshalJSON() ([]byte, error) {
+	return []byte(p.String()), nil
+}
+
+func (p *Percent) UnmarshalJSON(b []byte) error {
+	flt, err := strconv.ParseFloat(string(b)+"e2", 64)
+	if err != nil {
+		return xerrors.Errorf("unable to parse ratio %s: %w", string(b), err)
+	}
+	if math.Trunc(flt) != flt {
+		return xerrors.Errorf("ratio may only have two decimals: %s", string(b))
+	}
+	*p = Percent(flt)
+	return nil
+}
diff --git a/chain/types/percent_test.go b/chain/types/percent_test.go
@@ -0,0 +1,31 @@
+package types
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPercent(t *testing.T) {
+	for _, tc := range []struct {
+		p Percent
+		s string
+	}{
+		{100, "1.0"},
+		{111, "1.11"},
+		{12, "0.12"},
+		{1012, "10.12"},
+	} {
+		tc := tc
+		t.Run(fmt.Sprintf("%d <> %s", tc.p, tc.s), func(t *testing.T) {
+			m, err := tc.p.MarshalJSON()
+			require.NoError(t, err)
+			require.Equal(t, tc.s, string(m))
+			var p Percent
+			require.NoError(t, p.UnmarshalJSON([]byte(tc.s)))
+			require.Equal(t, tc.p, p)
+		})
+	}
+
+}
diff --git a/cli/mpool.go b/cli/mpool.go
@@ -461,7 +461,12 @@ var MpoolReplaceCmd = &cli.Command{
 		msg := found.Message
 
 		if cctx.Bool("auto") {
-			minRBF := messagepool.ComputeMinRBF(msg.GasPremium)
+			cfg, err := api.MpoolGetConfig(ctx)
+			if err != nil {
+				return xerrors.Errorf("failed to lookup the message pool config: %w", err)
+			}
+
+			defaultRBF := messagepool.ComputeRBF(msg.GasPremium, cfg.ReplaceByFeeRatio)
 
 			var mss *lapi.MessageSendSpec
 			if cctx.IsSet("fee-limit") {
@@ -482,7 +487,7 @@ var MpoolReplaceCmd = &cli.Command{
 				return xerrors.Errorf("failed to estimate gas values: %w", err)
 			}
 
-			msg.GasPremium = big.Max(retm.GasPremium, minRBF)
+			msg.GasPremium = big.Max(retm.GasPremium, defaultRBF)
 			msg.GasFeeCap = big.Max(retm.GasFeeCap, msg.GasPremium)
 
 			mff := func() (abi.TokenAmount, error) {

diff --git a/cli/mpool_test.go b/cli/mpool_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/filecoin-project/go-state-types/big"
 
 	"github.com/filecoin-project/lotus/api"
+	"github.com/filecoin-project/lotus/chain/messagepool"
 	"github.com/filecoin-project/lotus/chain/types"
 	"github.com/filecoin-project/lotus/chain/types/mock"
 	"github.com/filecoin-project/lotus/chain/wallet"
@@ -298,6 +299,7 @@ func TestReplace(t *testing.T) {
 			mockApi.EXPECT().ChainGetMessage(ctx, sm.Cid()).Return(&sm.Message, nil),
 			mockApi.EXPECT().ChainHead(ctx).Return(nil, nil),
 			mockApi.EXPECT().MpoolPending(ctx, types.EmptyTSK).Return([]*types.SignedMessage{sm}, nil),
+			mockApi.EXPECT().MpoolGetConfig(ctx).Return(messagepool.DefaultConfig(), nil),
 			// use gomock.any to match the message in expected api calls
 			// since the replace function modifies the message between calls, it would be pointless to try to match the exact argument
 			mockApi.EXPECT().GasEstimateMessageGas(ctx, gomock.Any(), &mss, types.EmptyTSK).Return(&sm.Message, nil),
@@ -342,6 +344,7 @@ func TestReplace(t *testing.T) {
 		gomock.InOrder(
 			mockApi.EXPECT().ChainHead(ctx).Return(nil, nil),
 			mockApi.EXPECT().MpoolPending(ctx, types.EmptyTSK).Return([]*types.SignedMessage{sm}, nil),
+			mockApi.EXPECT().MpoolGetConfig(ctx).Return(messagepool.DefaultConfig(), nil),
 			// use gomock.any to match the message in expected api calls
 			// since the replace function modifies the message between calls, it would be pointless to try to match the exact argument
 			mockApi.EXPECT().GasEstimateMessageGas(ctx, gomock.Any(), &mss, types.EmptyTSK).Return(&sm.Message, nil),
@@ -538,7 +541,7 @@ func TestConfig(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		mpoolCfg := &types.MpoolConfig{PriorityAddrs: []address.Address{senderAddr}, SizeLimitHigh: 1234567, SizeLimitLow: 6, ReplaceByFeeRatio: 0.25}
+		mpoolCfg := &types.MpoolConfig{PriorityAddrs: []address.Address{senderAddr}, SizeLimitHigh: 1234567, SizeLimitLow: 6, ReplaceByFeeRatio: types.Percent(25)}
 		gomock.InOrder(
 			mockApi.EXPECT().MpoolGetConfig(ctx).Return(mpoolCfg, nil),
 		)
@@ -566,7 +569,7 @@ func TestConfig(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		mpoolCfg := &types.MpoolConfig{PriorityAddrs: []address.Address{senderAddr}, SizeLimitHigh: 234567, SizeLimitLow: 3, ReplaceByFeeRatio: 0.33}
+		mpoolCfg := &types.MpoolConfig{PriorityAddrs: []address.Address{senderAddr}, SizeLimitHigh: 234567, SizeLimitLow: 3, ReplaceByFeeRatio: types.Percent(33)}
 		gomock.InOrder(
 			mockApi.EXPECT().MpoolSetConfig(ctx, mpoolCfg).Return(nil),
 		)

diff --git a/documentation/en/api-v0-methods.md b/documentation/en/api-v0-methods.md
@@ -2879,7 +2879,7 @@ Response:
   ],
   "SizeLimitHigh": 123,
   "SizeLimitLow": 123,
-  "ReplaceByFeeRatio": 12.3,
+  "ReplaceByFeeRatio": 1.23,
   "PruneCooldown": 60000000000,
   "GasLimitOverestimation": 12.3
 }
@@ -3167,7 +3167,7 @@ Inputs:
     ],
     "SizeLimitHigh": 123,
     "SizeLimitLow": 123,
-    "ReplaceByFeeRatio": 12.3,
+    "ReplaceByFeeRatio": 1.23,
     "PruneCooldown": 60000000000,
     "GasLimitOverestimation": 12.3
   }

diff --git a/documentation/en/api-v1-unstable-methods.md b/documentation/en/api-v1-unstable-methods.md
@@ -3882,7 +3882,7 @@ Response:
   ],
   "SizeLimitHigh": 123,
   "SizeLimitLow": 123,
-  "ReplaceByFeeRatio": 12.3,
+  "ReplaceByFeeRatio": 1.23,
   "PruneCooldown": 60000000000,
   "GasLimitOverestimation": 12.3
 }
@@ -4170,7 +4170,7 @@ Inputs:
     ],
     "SizeLimitHigh": 123,
     "SizeLimitLow": 123,
-    "ReplaceByFeeRatio": 12.3,
+    "ReplaceByFeeRatio": 1.23,
     "PruneCooldown": 60000000000,
     "GasLimitOverestimation": 12.3
   }