Refactor: Move VerificationGas checks to own file with tests (#68)

Author: hazim-j

pkg/modules/checks/sanitychecks.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"math/big"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/ethclient"
@@ -13,22 +12,6 @@ import (
 	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
 )
 
-// Checks that the verificationGasLimit is sufficiently low (<= MAX_VERIFICATION_GAS) and the
-// preVerificationGas is sufficiently high (enough to pay for the calldata gas cost of serializing
-// the UserOperation plus PRE_VERIFICATION_OVERHEAD_GAS)
-func checkVerificationGas(maxVerificationGas *big.Int, op *userop.UserOperation) error {
-	if op.VerificationGasLimit.Cmp(maxVerificationGas) > 0 {
-		return fmt.Errorf("verificationGasLimit: exceeds maxVerificationGas of %s", maxVerificationGas.String())
-	}
-
-	pvg := gas.NewDefaultOverhead().CalcPreVerificationGas(op)
-	if op.PreVerificationGas.Cmp(pvg) < 0 {
-		return fmt.Errorf("preVerificationGas: below expected gas of %s", pvg.String())
-	}
-
-	return nil
-}
-
 // Checks the paymasterAndData is either zero bytes or the first 20 bytes contain an address that
 //
 //  1. is not the zero address

pkg/modules/checks/standalone.go

@@ -48,7 +48,7 @@ func (s *Standalone) ValidateOpValues() modules.UserOpHandlerFunc {
 		g := new(errgroup.Group)
 		g.Go(func() error { return ValidateSender(ctx.UserOp, getCode) })
 		g.Go(func() error { return ValidateInitCode(ctx.UserOp, getStake) })
-		g.Go(func() error { return checkVerificationGas(s.maxVerificationGas, ctx.UserOp) })
+		g.Go(func() error { return ValidateVerificationGas(ctx.UserOp, s.maxVerificationGas) })
 		g.Go(func() error { return checkPaymasterAndData(s.eth, ep, ctx.UserOp) })
 		g.Go(func() error { return checkCallGasLimit(ctx.UserOp) })
 		g.Go(func() error { return checkFeePerGas(s.eth, ctx.UserOp) })

pkg/modules/checks/verificationgas.go

@@ -0,0 +1,25 @@
+package checks
+
+import (
+	"fmt"
+	"math/big"
+
+	"github.com/stackup-wallet/stackup-bundler/pkg/gas"
+	"github.com/stackup-wallet/stackup-bundler/pkg/userop"
+)
+
+// ValidateVerificationGas checks that the verificationGasLimit is sufficiently low (<= MAX_VERIFICATION_GAS)
+// and the preVerificationGas is sufficiently high (enough to pay for the calldata gas cost of serializing
+// the UserOperation plus PRE_VERIFICATION_OVERHEAD_GAS).
+func ValidateVerificationGas(op *userop.UserOperation, maxVerificationGas *big.Int) error {
+	if op.VerificationGasLimit.Cmp(maxVerificationGas) > 0 {
+		return fmt.Errorf("verificationGasLimit: exceeds maxVerificationGas of %s", maxVerificationGas.String())
+	}
+
+	pvg := gas.NewDefaultOverhead().CalcPreVerificationGas(op)
+	if op.PreVerificationGas.Cmp(pvg) < 0 {
+		return fmt.Errorf("preVerificationGas: below expected gas of %s", pvg.String())
+	}
+
+	return nil
+}

pkg/modules/checks/verificationgas_test.go

@@ -0,0 +1,79 @@
+package checks
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stackup-wallet/stackup-bundler/internal/testutils"
+	"github.com/stackup-wallet/stackup-bundler/pkg/gas"
+)
+
+// TestOpVGlessThanMaxVG calls checks.ValidateVerificationGas where verificationGas < MAX_VERIFICATION_GAS.
+// Expects nil.
+func TestOpVGlessThanMaxVG(t *testing.T) {
+	op := testutils.MockValidInitUserOp()
+	mvg := big.NewInt(0).Add(op.VerificationGasLimit, common.Big1)
+
+	if err := ValidateVerificationGas(op, mvg); err != nil {
+		t.Fatalf("got %v, want nil", err)
+	}
+}
+
+// TestOpVGEqualMaxVG calls checks.ValidateVerificationGas where verificationGas == MAX_VERIFICATION_GAS.
+// Expects nil.
+func TestOpVGEqualMaxVG(t *testing.T) {
+	op := testutils.MockValidInitUserOp()
+	mvg := big.NewInt(0).Add(op.VerificationGasLimit, common.Big0)
+
+	if err := ValidateVerificationGas(op, mvg); err != nil {
+		t.Fatalf("got %v, want nil", err)
+	}
+}
+
+// TestOpVGMoreThanMaxVG calls checks.ValidateVerificationGas where verificationGas > MAX_VERIFICATION_GAS.
+// Expects error.
+func TestOpVGMoreThanMaxVG(t *testing.T) {
+	op := testutils.MockValidInitUserOp()
+	mvg := big.NewInt(0).Sub(op.VerificationGasLimit, common.Big1)
+
+	if err := ValidateVerificationGas(op, mvg); err == nil {
+		t.Fatal("got nil, want err")
+	}
+}
+
+// TestOpPVGMoreThanOH calls checks.ValidateVerificationGas where the preVerificationGas > overhead gas.
+// Expect nil.
+func TestOpPVGMoreThanOH(t *testing.T) {
+	op := testutils.MockValidInitUserOp()
+	pvg := gas.NewDefaultOverhead().CalcPreVerificationGas(op)
+	op.PreVerificationGas = big.NewInt(0).Add(pvg, common.Big1)
+
+	if err := ValidateVerificationGas(op, op.VerificationGasLimit); err != nil {
+		t.Fatalf("got %v, want nil", err)
+	}
+}
+
+// TestOpPVGEqualOH calls checks.ValidateVerificationGas where the preVerificationGas == overhead gas. Expect
+// nil.
+func TestOpPVGEqualOH(t *testing.T) {
+	op := testutils.MockValidInitUserOp()
+	pvg := gas.NewDefaultOverhead().CalcPreVerificationGas(op)
+	op.PreVerificationGas = big.NewInt(0).Add(pvg, common.Big0)
+
+	if err := ValidateVerificationGas(op, op.VerificationGasLimit); err != nil {
+		t.Fatalf("got %v, want nil", err)
+	}
+}
+
+// TestOpPVGLessThanOH calls checks.ValidateVerificationGas where the preVerificationGas < overhead gas.
+// Expect error.
+func TestOpPVGLessThanOH(t *testing.T) {
+	op := testutils.MockValidInitUserOp()
+	pvg := gas.NewDefaultOverhead().CalcPreVerificationGas(op)
+	op.PreVerificationGas = big.NewInt(0).Sub(pvg, common.Big1)
+
+	if err := ValidateVerificationGas(op, op.VerificationGasLimit); err == nil {
+		t.Fatal("got nil, want err")
+	}
+}