Add tip to context and fix MaxFee calculation (#197)

hazim-j · web-flow · commit e1ad241a74f7 · 2023-07-02T22:50:57.000+10:00
diff --git a/internal/start/private.go b/internal/start/private.go
@@ -111,6 +111,7 @@ func PrivateMode() {
 	// Init Bundler
 	b := bundler.New(mem, chain, conf.SupportedEntryPoints)
 	b.SetGetBaseFeeFunc(gasprice.GetBaseFeeWithEthClient(eth))
+	b.SetGetGasTipFunc(gasprice.GetGasTipWithEthClient(eth))
 	b.SetGetLegacyGasPriceFunc(gasprice.GetLegacyGasPriceWithEthClient(eth))
 	b.UseLogger(logr)
 	b.UseModules(
diff --git a/internal/start/searcher.go b/internal/start/searcher.go
@@ -104,6 +104,7 @@ func SearcherMode() {
 	// Init Bundler
 	b := bundler.New(mem, chain, conf.SupportedEntryPoints)
 	b.SetGetBaseFeeFunc(gasprice.GetBaseFeeWithEthClient(eth))
+	b.SetGetGasTipFunc(gasprice.GetGasTipWithEthClient(eth))
 	b.SetGetLegacyGasPriceFunc(gasprice.GetLegacyGasPriceWithEthClient(eth))
 	b.UseLogger(logr)
 	b.UseModules(
diff --git a/pkg/bundler/bundler.go b/pkg/bundler/bundler.go
@@ -28,6 +28,7 @@ type Bundler struct {
 	stop                 func()
 	maxBatch             int
 	gbf                  gasprice.GetBaseFeeFunc
+	ggt                  gasprice.GetGasTipFunc
 	ggp                  gasprice.GetLegacyGasPriceFunc
 }
 
@@ -45,6 +46,7 @@ func New(mempool *mempool.Mempool, chainID *big.Int, supportedEntryPoints []comm
 		stop:                 func() {},
 		maxBatch:             0,
 		gbf:                  gasprice.NoopGetBaseFeeFunc(),
+		ggt:                  gasprice.NoopGetGasTipFunc(),
 		ggp:                  gasprice.NoopGetLegacyGasPriceFunc(),
 	}
 }
@@ -59,6 +61,11 @@ func (i *Bundler) SetGetBaseFeeFunc(gbf gasprice.GetBaseFeeFunc) {
 	i.gbf = gbf
 }
 
+// SetGetGasTipFunc defines the function used to retrieve an estimate for gas tip during each bundler run.
+func (i *Bundler) SetGetGasTipFunc(ggt gasprice.GetGasTipFunc) {
+	i.ggt = ggt
+}
+
 // SetGetLegacyGasPriceFunc defines the function used to retrieve an estimate for gas price during each
 // bundler run.
 func (i *Bundler) SetGetLegacyGasPriceFunc(ggp gasprice.GetLegacyGasPriceFunc) {
@@ -91,6 +98,16 @@ func (i *Bundler) Process(ep common.Address) (*modules.BatchHandlerCtx, error) {
 		return nil, err
 	}
 
+	// Get suggested gas tip
+	var gt *big.Int
+	if bf != nil {
+		gt, err = i.ggt()
+		if err != nil {
+			l.Error(err, "bundler run error")
+			return nil, err
+		}
+	}
+
 	// Get suggested gas price (for networks that don't support EIP-1559)
 	gp, err := i.ggp()
 	if err != nil {
@@ -111,7 +128,7 @@ func (i *Bundler) Process(ep common.Address) (*modules.BatchHandlerCtx, error) {
 	batch = adjustBatchSize(i.maxBatch, batch)
 
 	// Create context and execute modules.
-	ctx := modules.NewBatchHandlerContext(batch, ep, i.chainID, bf, gp)
+	ctx := modules.NewBatchHandlerContext(batch, ep, i.chainID, bf, gt, gp)
 	if err := i.batchHandler(ctx); err != nil {
 		l.Error(err, "bundler run error")
 		return nil, err
diff --git a/pkg/entrypoint/transaction/gas.go b/pkg/entrypoint/transaction/gas.go
@@ -1,41 +1,33 @@
 package transaction
 
 import (
-	"context"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
 // SuggestMeanGasTipCap suggests a Max Priority Fee for an EIP-1559 transaction to submit a batch of
-// UserOperations to the EntryPoint. It returns the larger value between the gas tip suggested by the
-// underlying node (i.e. eth_maxPriorityFeePerGas) or the average maxPriorityFeePerGas of the entire batch.
-func SuggestMeanGasTipCap(eth *ethclient.Client, batch []*userop.UserOperation) (*big.Int, error) {
-	tip, err := eth.SuggestGasTipCap(context.Background())
-	if err != nil {
-		return nil, err
-	}
-
+// UserOperations to the EntryPoint. It returns the larger value between the suggested gas tip or the average
+// maxPriorityFeePerGas of the entire batch.
+func SuggestMeanGasTipCap(tip *big.Int, batch []*userop.UserOperation) *big.Int {
 	sum := big.NewInt(0)
 	for _, op := range batch {
 		sum = big.NewInt(0).Add(sum, op.MaxPriorityFeePerGas)
 	}
 	avg := big.NewInt(0).Div(sum, big.NewInt(int64(len(batch))))
 
 	if avg.Cmp(tip) == 1 {
-		return avg, nil
+		return avg
 	}
-	return tip, nil
+	return tip
 }
 
 // SuggestMeanGasFeeCap suggests a Max Fee for an EIP-1559 transaction to submit a batch of UserOperations to
 // the EntryPoint. It returns the larger value between the recommended max fee or the average maxFeePerGas of
 // the entire batch.
-func SuggestMeanGasFeeCap(basefee *big.Int, batch []*userop.UserOperation) *big.Int {
-	mf := big.NewInt(0).Mul(basefee, common.Big2)
-
+func SuggestMeanGasFeeCap(basefee *big.Int, tip *big.Int, batch []*userop.UserOperation) *big.Int {
+	mf := big.NewInt(0).Add(tip, big.NewInt(0).Mul(basefee, common.Big2))
 	sum := big.NewInt(0)
 	for _, op := range batch {
 		sum = big.NewInt(0).Add(sum, op.MaxFeePerGas)
diff --git a/pkg/entrypoint/transaction/gas_test.go b/pkg/entrypoint/transaction/gas_test.go
@@ -5,58 +5,38 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/common/hexutil"
-	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/stackup-wallet/stackup-bundler/internal/testutils"
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
 // TestSuggestMeanGasTipCapForNormalLoad simulates a scenario of normal network load. In this case the average
-// tip from userOps is assumed to not be above the tip suggested by the underlying node. In which case the
-// node's suggested tip is the optimal choice.
+// tip from userOps is assumed to not be above the suggested tip. In which case the suggested tip is the
+// optimal choice.
 func TestSuggestMeanGasTipCapForNormalLoad(t *testing.T) {
-	expected := big.NewInt(2)
-	be := testutils.EthMock(testutils.MethodMocks{
-		"eth_maxPriorityFeePerGas": hexutil.EncodeBig(expected),
-	})
-	eth, err := ethclient.Dial(be.URL)
-	if err != nil {
-		panic(err)
-	}
-
 	op1 := testutils.MockValidInitUserOp()
 	op1.MaxPriorityFeePerGas = big.NewInt(1)
 	op2 := testutils.MockValidInitUserOp()
 	op2.MaxPriorityFeePerGas = big.NewInt(1)
 	batch := []*userop.UserOperation{op1, op2}
-	if tip, err := SuggestMeanGasTipCap(eth, batch); err != nil {
-		t.Fatalf("got %v, want %d", err, expected.Int64())
-	} else if tip.Cmp(expected) != 0 {
+
+	expected := big.NewInt(2)
+	if tip := SuggestMeanGasTipCap(expected, batch); tip.Cmp(expected) != 0 {
 		t.Fatalf("got %d, want %d", tip.Int64(), expected.Int64())
 	}
 }
 
 // TestSuggestMeanGasTipCapForHighLoad simulates a scenario of high network load. In this case the average tip
-// from userOps is assumed to be above the tip suggested by the underlying node (i.e. userOps want to be
-// included quickly). In which case the average tip from userOps is the optimal choice.
+// from userOps is assumed to be above the suggested tip (i.e. userOps want to be included quickly). In which
+// case the average tip from userOps is the optimal choice.
 func TestSuggestMeanGasTipCapForHighLoad(t *testing.T) {
-	be := testutils.EthMock(testutils.MethodMocks{
-		"eth_maxPriorityFeePerGas": hexutil.EncodeBig(big.NewInt(2)),
-	})
-	eth, err := ethclient.Dial(be.URL)
-	if err != nil {
-		panic(err)
-	}
-
 	op1 := testutils.MockValidInitUserOp()
 	op1.MaxPriorityFeePerGas = big.NewInt(5)
 	op2 := testutils.MockValidInitUserOp()
 	op2.MaxPriorityFeePerGas = big.NewInt(10)
-	expected := big.NewInt(7)
 	batch := []*userop.UserOperation{op1, op2}
-	if tip, err := SuggestMeanGasTipCap(eth, batch); err != nil {
-		t.Fatalf("got %v, want %d", err, expected.Int64())
-	} else if tip.Cmp(expected) != 0 {
+
+	expected := big.NewInt(7)
+	if tip := SuggestMeanGasTipCap(big.NewInt(2), batch); tip.Cmp(expected) != 0 {
 		t.Fatalf("got %d, want %d", tip.Int64(), expected.Int64())
 	}
 }
@@ -72,8 +52,9 @@ func TestSuggestMeanGasFeeCapNormalLoad(t *testing.T) {
 	batch := []*userop.UserOperation{op1, op2}
 
 	bf := big.NewInt(1)
+	tip := big.NewInt(0)
 	expected := big.NewInt(0).Mul(bf, common.Big2)
-	mf := SuggestMeanGasFeeCap(bf, batch)
+	mf := SuggestMeanGasFeeCap(bf, tip, batch)
 	if mf.Cmp(expected) != 0 {
 		t.Fatalf("got %d, want %d", mf.Int64(), expected.Int64())
 	}
@@ -90,8 +71,9 @@ func TestSuggestMeanGasFeeCapHighLoad(t *testing.T) {
 	batch := []*userop.UserOperation{op1, op2}
 
 	bf := big.NewInt(1)
+	tip := big.NewInt(0)
 	expected := big.NewInt(7)
-	mf := SuggestMeanGasFeeCap(bf, batch)
+	mf := SuggestMeanGasFeeCap(bf, tip, batch)
 	if mf.Cmp(expected) != 0 {
 		t.Fatalf("got %d, want %d", mf.Int64(), expected.Int64())
 	}
diff --git a/pkg/entrypoint/transaction/handleops.go b/pkg/entrypoint/transaction/handleops.go
@@ -35,6 +35,7 @@ type Opts struct {
 
 	// Options for the EOA transaction
 	BaseFee     *big.Int
+	Tip         *big.Int
 	GasPrice    *big.Int
 	GasLimit    uint64
 	WaitTimeout time.Duration
@@ -110,21 +111,13 @@ func HandleOps(opts *Opts) (txn *types.Transaction, err error) {
 	}
 	auth.Nonce = big.NewInt(int64(nonce))
 
-	if opts.BaseFee != nil {
-		if tip, err := SuggestMeanGasTipCap(opts.Eth, opts.Batch); err != nil {
-			return nil, err
-		} else {
-			// Note: If gas price was to spike, we can run into an edge case where the suggested gas tip from
-			// the underlying node is greater than either of the suggested gas fee caps. This is fine since
-			// the eth client will throw an error and the userOp will not be submitted until either the gas
-			// tip comes back down or the userOp gets replaced with 10% higher fees.
-			auth.GasFeeCap = SuggestMeanGasFeeCap(opts.BaseFee, opts.Batch)
-			auth.GasTipCap = tip
-		}
+	if opts.BaseFee != nil && opts.Tip != nil {
+		auth.GasTipCap = SuggestMeanGasTipCap(opts.Tip, opts.Batch)
+		auth.GasFeeCap = SuggestMeanGasFeeCap(opts.BaseFee, opts.Tip, opts.Batch)
 	} else if opts.GasPrice != nil {
 		auth.GasPrice = SuggestMeanGasPrice(opts.GasPrice, opts.Batch)
 	} else {
-		return nil, errors.New("transaction: opts.BaseFee and opts.GasPrice cannot both be nil")
+		return nil, errors.New("transaction: either the dynamic or legacy gas fees must be set")
 	}
 
 	txn, err = ep.HandleOps(auth, toAbiType(opts.Batch), opts.Beneficiary)
diff --git a/pkg/modules/context.go b/pkg/modules/context.go
@@ -18,6 +18,7 @@ type BatchHandlerCtx struct {
 	EntryPoint     common.Address
 	ChainID        *big.Int
 	BaseFee        *big.Int
+	Tip            *big.Int
 	GasPrice       *big.Int
 	Data           map[string]any
 }
@@ -28,6 +29,7 @@ func NewBatchHandlerContext(
 	entryPoint common.Address,
 	chainID *big.Int,
 	baseFee *big.Int,
+	tip *big.Int,
 	gasPrice *big.Int,
 ) *BatchHandlerCtx {
 	var copy []*userop.UserOperation
@@ -39,6 +41,7 @@ func NewBatchHandlerContext(
 		EntryPoint:     entryPoint,
 		ChainID:        chainID,
 		BaseFee:        baseFee,
+		Tip:            tip,
 		GasPrice:       gasPrice,
 		Data:           make(map[string]any),
 	}
diff --git a/pkg/modules/gasprice/filter.go b/pkg/modules/gasprice/filter.go
@@ -1,18 +1,21 @@
 package gasprice
 
 import (
+	"math/big"
+
 	"github.com/stackup-wallet/stackup-bundler/pkg/modules"
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
 // FilterUnderpriced returns a BatchHandlerFunc that will filter out all the userOps that are below either the
-// BaseFee or GasPrice set in the context.
+// dynamic or legacy GasPrice set in the context.
 func FilterUnderpriced() modules.BatchHandlerFunc {
 	return func(ctx *modules.BatchHandlerCtx) error {
 		b := []*userop.UserOperation{}
 		for _, op := range ctx.Batch {
-			if ctx.BaseFee != nil {
-				if op.GetDynamicGasPrice(ctx.BaseFee).Cmp(ctx.BaseFee) >= 0 {
+			if ctx.BaseFee != nil && ctx.Tip != nil {
+				gp := big.NewInt(0).Add(ctx.BaseFee, ctx.Tip)
+				if op.GetDynamicGasPrice(ctx.BaseFee).Cmp(gp) >= 0 {
 					b = append(b, op)
 				}
 			} else if ctx.GasPrice != nil {
diff --git a/pkg/modules/gasprice/fliter_test.go b/pkg/modules/gasprice/fliter_test.go
@@ -10,9 +10,12 @@ import (
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
-// TestFilterUnderpricedBaseFee verifies that FilterUnderpriced will remove all UserOperations from a batch
-// where the effective gas price is less than the context BaseFee.
-func TestFilterUnderpricedBaseFee(t *testing.T) {
+// TestFilterUnderpricedDynamic verifies that FilterUnderpriced will remove all UserOperations from a batch
+// where the effective gas price is less than the expected bundler transaction's gas price.
+func TestFilterUnderpricedDynamic(t *testing.T) {
+	bf := big.NewInt(4)
+	tip := big.NewInt(1)
+
 	op1 := testutils.MockValidInitUserOp()
 	op1.MaxFeePerGas = big.NewInt(4)
 	op1.MaxPriorityFeePerGas = big.NewInt(3)
@@ -31,7 +34,8 @@ func TestFilterUnderpricedBaseFee(t *testing.T) {
 		[]*userop.UserOperation{op1, op2, op3},
 		testutils.ValidAddress1,
 		testutils.ChainID,
-		big.NewInt(5),
+		bf,
+		tip,
 		big.NewInt(10),
 	)
 	if err := gasprice.FilterUnderpriced()(ctx); err != nil {
@@ -67,6 +71,7 @@ func TestFilterUnderpricedGasPrice(t *testing.T) {
 		testutils.ValidAddress1,
 		testutils.ChainID,
 		nil,
+		nil,
 		big.NewInt(5),
 	)
 	if err := gasprice.FilterUnderpriced()(ctx); err != nil {
diff --git a/pkg/modules/gasprice/sort_test.go b/pkg/modules/gasprice/sort_test.go
@@ -10,9 +10,12 @@ import (
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
-// TestSortByGasPriceBaseFee verifies that SortByGasPrice sorts the UserOperations in a batch by highest
+// TestSortByGasPriceBaseDynamic verifies that SortByGasPrice sorts the UserOperations in a batch by highest
 // effective Gas Price first.
-func TestSortByGasPriceBaseFee(t *testing.T) {
+func TestSortByGasPriceBaseDynamic(t *testing.T) {
+	bf := big.NewInt(3)
+	tip := big.NewInt(0)
+
 	op1 := testutils.MockValidInitUserOp()
 	op1.MaxFeePerGas = big.NewInt(4)
 	op1.MaxPriorityFeePerGas = big.NewInt(3)
@@ -31,7 +34,8 @@ func TestSortByGasPriceBaseFee(t *testing.T) {
 		[]*userop.UserOperation{op1, op2, op3},
 		testutils.ValidAddress1,
 		testutils.ChainID,
-		big.NewInt(3),
+		bf,
+		tip,
 		big.NewInt(6),
 	)
 	if err := gasprice.SortByGasPrice()(ctx); err != nil {
@@ -69,6 +73,7 @@ func TestSortByGasPriceLegacy(t *testing.T) {
 		testutils.ValidAddress1,
 		testutils.ChainID,
 		nil,
+		nil,
 		big.NewInt(4),
 	)
 	if err := gasprice.SortByGasPrice()(ctx); err != nil {
diff --git a/pkg/modules/gasprice/tip.go b/pkg/modules/gasprice/tip.go
@@ -0,0 +1,30 @@
+package gasprice
+
+import (
+	"context"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/ethclient"
+)
+
+// GetGasTipFunc provides a general interface for retrieving the closest estimate for gas tip to allow for
+// timely execution of a transaction.
+type GetGasTipFunc = func() (*big.Int, error)
+
+// NoopGetGasTipFunc returns nil gas tip and nil error.
+func NoopGetGasTipFunc() GetGasTipFunc {
+	return func() (*big.Int, error) {
+		return nil, nil
+	}
+}
+
+// GetGasTipWithEthClient returns a GetGasTipFunc using an eth client.
+func GetGasTipWithEthClient(eth *ethclient.Client) GetGasTipFunc {
+	return func() (*big.Int, error) {
+		gt, err := eth.SuggestGasTipCap(context.Background())
+		if err != nil {
+			return nil, err
+		}
+		return gt, nil
+	}
+}
diff --git a/pkg/modules/relay/relayer.go b/pkg/modules/relay/relayer.go
@@ -211,6 +211,7 @@ func (r *Relayer) SendUserOperation() modules.BatchHandlerFunc {
 			Batch:       ctx.Batch,
 			Beneficiary: r.beneficiary,
 			BaseFee:     ctx.BaseFee,
+			Tip:         ctx.Tip,
 			GasPrice:    ctx.GasPrice,
 			GasLimit:    0,
 			WaitTimeout: r.waitTimeout,
