Only replace ops if both fee values increase by >= 10% (#86)

Author: hazim-j

pkg/modules/checks/pendingops.go

@@ -1,13 +1,27 @@
 package checks
 
 import (
-	"errors"
 	"fmt"
 	"math/big"
 
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
+const minPriceBump = 10
+
+// calcNewThresholds returns new threshold values where newFee = oldFee  * (100 + minPriceBump) / 100.
+func calcNewThresholds(cap *big.Int, tip *big.Int) (newCap *big.Int, newTip *big.Int) {
+	a := big.NewInt(100 + minPriceBump)
+	aFeeCap := big.NewInt(0).Mul(a, cap)
+	aTip := big.NewInt(0).Mul(a, tip)
+
+	b := big.NewInt(100)
+	newCap = aFeeCap.Div(aFeeCap, b)
+	newTip = aTip.Div(aTip, b)
+
+	return newCap, newTip
+}
+
 // ValidatePendingOps checks the pending UserOperations by the same sender and only passes if:
 //
 //  1. Sender doesn't have another UserOperation already present in the pool.
@@ -33,16 +47,13 @@ func ValidatePendingOps(
 		}
 
 		if oldOp != nil {
-			if op.MaxPriorityFeePerGas.Cmp(oldOp.MaxPriorityFeePerGas) <= 0 {
-				return errors.New(
-					"pending ops: sender has op in mempool with same or higher priority fee",
-				)
-			}
+			newMf, newMpf := calcNewThresholds(oldOp.MaxFeePerGas, oldOp.MaxPriorityFeePerGas)
 
-			diff := big.NewInt(0).Sub(op.MaxPriorityFeePerGas, oldOp.MaxPriorityFeePerGas)
-			mf := big.NewInt(0).Add(oldOp.MaxFeePerGas, diff)
-			if op.MaxFeePerGas.Cmp(mf) != 0 {
-				return errors.New("pending ops: replaced op must have an equally higher max fee")
+			if op.MaxFeePerGas.Cmp(newMf) < 0 || op.MaxPriorityFeePerGas.Cmp(newMpf) < 0 {
+				return fmt.Errorf(
+					"pending ops: replacement op must increase maxFeePerGas and MaxPriorityFeePerGas by >= %d%%",
+					minPriceBump,
+				)
 			}
 		} else if !dep.Staked && len(penOps) >= maxOpsForUnstakedSender {
 			return fmt.Errorf(

pkg/modules/checks/pendingops_test.go

@@ -62,121 +62,3 @@ func TestPendingOpsStaked(t *testing.T) {
 		t.Fatalf("got err %v, want nil", err)
 	}
 }
-
-// TestReplaceOp calls checks.ValidatePendingOps with a valid UserOperation that replaces a pending
-// UserOperation. Expect nil.
-func TestReplaceOp(t *testing.T) {
-	penOp := testutils.MockValidInitUserOp()
-	penOps := []*userop.UserOperation{penOp}
-	op := testutils.MockValidInitUserOp()
-	op.MaxPriorityFeePerGas = big.NewInt(0).Add(penOp.MaxPriorityFeePerGas, common.Big1)
-	op.MaxFeePerGas = big.NewInt(0).Add(penOp.MaxFeePerGas, common.Big1)
-	err := ValidatePendingOps(
-		op,
-		penOps,
-		testutils.MaxOpsForUnstakedSender,
-		testutils.MockGetNotStakeZeroDeposit,
-	)
-
-	if err != nil {
-		t.Fatalf("got err %v, want nil", err)
-	}
-}
-
-// TestReplaceOpLowerMPF calls checks.ValidatePendingOps with a UserOperation that replaces a pending
-// UserOperation but has a lower MaxPriorityFeePerGas. Expect error.
-func TestReplaceOpLowerMPF(t *testing.T) {
-	penOp := testutils.MockValidInitUserOp()
-	penOps := []*userop.UserOperation{penOp}
-	op := testutils.MockValidInitUserOp()
-	op.MaxPriorityFeePerGas = big.NewInt(0).Sub(penOp.MaxPriorityFeePerGas, common.Big1)
-	err := ValidatePendingOps(
-		op,
-		penOps,
-		testutils.MaxOpsForUnstakedSender,
-		testutils.MockGetNotStakeZeroDeposit,
-	)
-
-	if err == nil {
-		t.Fatal("got nil, want err")
-	}
-}
-
-// TestReplaceOpEqualMPF calls checks.ValidatePendingOps with a UserOperation that replaces a pending
-// UserOperation but has an equal MaxPriorityFeePerGas. Expect error.
-func TestReplaceOpEqualMPF(t *testing.T) {
-	penOp := testutils.MockValidInitUserOp()
-	penOps := []*userop.UserOperation{penOp}
-	op := testutils.MockValidInitUserOp()
-	op.MaxPriorityFeePerGas = big.NewInt(0).Add(penOp.MaxPriorityFeePerGas, common.Big0)
-	err := ValidatePendingOps(
-		op,
-		penOps,
-		testutils.MaxOpsForUnstakedSender,
-		testutils.MockGetNotStakeZeroDeposit,
-	)
-
-	if err == nil {
-		t.Fatal("got nil, want err")
-	}
-}
-
-// TestReplaceOpNotEqualIncMF calls checks.ValidatePendingOps with a UserOperation that replaces a pending
-// UserOperation but does not have an equally increasing MaxFeePerGas. Expect error.
-func TestReplaceOpNotEqualIncMF(t *testing.T) {
-	penOp := testutils.MockValidInitUserOp()
-	penOps := []*userop.UserOperation{penOp}
-	op := testutils.MockValidInitUserOp()
-	op.MaxPriorityFeePerGas = big.NewInt(0).Add(penOp.MaxPriorityFeePerGas, common.Big2)
-	op.MaxFeePerGas = big.NewInt(0).Add(penOp.MaxFeePerGas, common.Big1)
-	err := ValidatePendingOps(
-		op,
-		penOps,
-		testutils.MaxOpsForUnstakedSender,
-		testutils.MockGetNotStakeZeroDeposit,
-	)
-
-	if err == nil {
-		t.Fatal("got nil, want err")
-	}
-}
-
-// TestReplaceOpSameMF calls checks.ValidatePendingOps with a UserOperation that replaces a pending
-// UserOperation but does not increase MaxFeePerGas. Expect error.
-func TestReplaceOpSameMF(t *testing.T) {
-	penOp := testutils.MockValidInitUserOp()
-	penOps := []*userop.UserOperation{penOp}
-	op := testutils.MockValidInitUserOp()
-	op.MaxPriorityFeePerGas = big.NewInt(0).Add(penOp.MaxPriorityFeePerGas, common.Big1)
-	op.MaxFeePerGas = big.NewInt(0).Add(penOp.MaxFeePerGas, common.Big0)
-	err := ValidatePendingOps(
-		op,
-		penOps,
-		testutils.MaxOpsForUnstakedSender,
-		testutils.MockGetNotStakeZeroDeposit,
-	)
-
-	if err == nil {
-		t.Fatal("got nil, want err")
-	}
-}
-
-// TestReplaceOpDecMF calls checks.ValidatePendingOps with a UserOperation that replaces a pending
-// UserOperation but has a decreasing MaxFeePerGas. Expect error.
-func TestReplaceOpDecMF(t *testing.T) {
-	penOp := testutils.MockValidInitUserOp()
-	penOps := []*userop.UserOperation{penOp}
-	op := testutils.MockValidInitUserOp()
-	op.MaxPriorityFeePerGas = big.NewInt(0).Add(penOp.MaxPriorityFeePerGas, common.Big1)
-	op.MaxFeePerGas = big.NewInt(0).Sub(penOp.MaxFeePerGas, common.Big1)
-	err := ValidatePendingOps(
-		op,
-		penOps,
-		testutils.MaxOpsForUnstakedSender,
-		testutils.MockGetNotStakeZeroDeposit,
-	)
-
-	if err == nil {
-		t.Fatal("got nil, want err")
-	}
-}