feat: refactor errors

Author: CarsonRoscoe

go/mechanisms/evm/batched/client/extensions.go

@@ -64,8 +64,26 @@ func (c *BatchedEvmScheme) CreatePaymentPayloadWithExtensions(
 	}
 
 	// Only Permit2 deposits gas-sponsor through these extensions; ERC-3009
-	// deposits authorize the transfer in the same signature.
-	if !isPermit2Deposit(requirements, result) {
+	// deposits authorize the transfer in the same signature. Read
+	// `assetTransferMethod` from requirements.Extra (TS source of truth)
+	// and fall back to inspecting the deposit authorization shape so a
+	// missing wire field still routes correctly.
+	isPermit2 := false
+	if requirements.Extra != nil {
+		if v, ok := requirements.Extra["assetTransferMethod"].(string); ok && v != "" {
+			isPermit2 = v == "permit2"
+		}
+	}
+	if !isPermit2 && result.Payload != nil {
+		if dep, ok := result.Payload["deposit"].(map[string]interface{}); ok {
+			if auth, ok := dep["authorization"].(map[string]interface{}); ok {
+				if _, has := auth["permit2Authorization"]; has {
+					isPermit2 = true
+				}
+			}
+		}
+	}
+	if !isPermit2 {
 		return result, nil
 	}
 
@@ -81,28 +99,6 @@ func (c *BatchedEvmScheme) CreatePaymentPayloadWithExtensions(
 	return result, nil
 }
 
-// isPermit2Deposit reports whether the resolved deposit payload uses the
-// Permit2 transfer method. Reads requirements.Extra first (TS source of
-// truth) and falls back to inspecting the deposit authorization shape, so a
-// missing `assetTransferMethod` field on the wire still routes correctly.
-func isPermit2Deposit(requirements types.PaymentRequirements, payload types.PaymentPayload) bool {
-	if requirements.Extra != nil {
-		if v, ok := requirements.Extra["assetTransferMethod"].(string); ok && v != "" {
-			return v == "permit2"
-		}
-	}
-	if payload.Payload != nil {
-		if dep, ok := payload.Payload["deposit"].(map[string]interface{}); ok {
-			if auth, ok := dep["authorization"].(map[string]interface{}); ok {
-				if _, has := auth["permit2Authorization"]; has {
-					return true
-				}
-			}
-		}
-	}
-	return false
-}
-
 // trySignEip2612Permit attempts to sign an EIP-2612 permit authorizing Permit2
 // to spend the deposit amount. Returns nil (no error) when the extension is
 // not advertised, the token does not expose name/version, the signer cannot
@@ -147,8 +143,19 @@ func (c *BatchedEvmScheme) trySignEip2612Permit(
 
 	tokenAddress := evm.NormalizeAddress(requirements.Asset)
 
-	depositAmount, ok := readDepositAmount(result)
-	if !ok {
+	// Pull `payload.deposit.amount` from the freshly-built batched deposit
+	// payload. The signed EIP-2612 `info.amount` must equal the BATCH
+	// deposit (what the facilitator's `validateBatchEip2612Permit` checks),
+	// not `requirements.Amount` (the per-request charge).
+	var depositAmount string
+	if result.Payload != nil {
+		if dep, ok := result.Payload["deposit"].(map[string]interface{}); ok {
+			if amt, ok := dep["amount"].(string); ok && amt != "" {
+				depositAmount = amt
+			}
+		}
+	}
+	if depositAmount == "" {
 		return nil, nil
 	}
 
@@ -159,7 +166,22 @@ func (c *BatchedEvmScheme) trySignEip2612Permit(
 		return nil, nil
 	}
 
-	deadline := readPermit2DeadlineFromBatchedDeposit(result)
+	// Align EIP-2612 deadline with the just-signed Permit2 authorization
+	// (`payload.deposit.authorization.permit2Authorization.deadline`) so
+	// both signatures expire together. Falls back to
+	// `now + maxTimeoutSeconds` when the field isn't present.
+	deadline := ""
+	if result.Payload != nil {
+		if dep, ok := result.Payload["deposit"].(map[string]interface{}); ok {
+			if auth, ok := dep["authorization"].(map[string]interface{}); ok {
+				if permit2, ok := auth["permit2Authorization"].(map[string]interface{}); ok {
+					if d, ok := permit2["deadline"].(string); ok {
+						deadline = d
+					}
+				}
+			}
+		}
+	}
 	if deadline == "" {
 		deadline = fmt.Sprintf("%d", time.Now().Unix()+int64(requirements.MaxTimeoutSeconds))
 	}
@@ -214,14 +236,12 @@ func (c *BatchedEvmScheme) trySignErc20Approval(
 	tokenAddress := evm.NormalizeAddress(requirements.Asset)
 
 	if readSigner, hasRead := c.signer.(evm.ClientEvmSignerWithReadContract); hasRead {
-		// Use deposit amount, not requirements.Amount, since the deposit is
-		// what the facilitator will pull through Permit2 — that's what needs
-		// to fit under the existing allowance.
-		needed := requirements.Amount
-		if v, ok := readDepositAmountFromAnyPayload(c, requirements); ok {
-			needed = v
-		}
-		if hasSufficientPermit2Allowance(ctx, readSigner, tokenAddress, c.signer.Address(), needed) {
+		// Approximate the deposit amount with the per-request amount —
+		// sufficient for the allowance short-circuit since `deposit ≥
+		// requirements.Amount` always holds for batched flows. Skip the
+		// short-circuit when no per-request amount is available.
+		if requirements.Amount != "" &&
+			hasSufficientPermit2Allowance(ctx, readSigner, tokenAddress, c.signer.Address(), requirements.Amount) {
 			return nil, nil
 		}
 	}
@@ -238,59 +258,6 @@ func (c *BatchedEvmScheme) trySignErc20Approval(
 	}, nil
 }
 
-// readDepositAmount pulls `payload.deposit.amount` from a freshly built
-// batched deposit payload map. Returns ("", false) for non-deposit payloads.
-func readDepositAmount(payload types.PaymentPayload) (string, bool) {
-	if payload.Payload == nil {
-		return "", false
-	}
-	dep, ok := payload.Payload["deposit"].(map[string]interface{})
-	if !ok {
-		return "", false
-	}
-	amt, ok := dep["amount"].(string)
-	if !ok || amt == "" {
-		return "", false
-	}
-	return amt, true
-}
-
-// readDepositAmountFromAnyPayload is a fallback for callers that don't have
-// the just-built payload in hand (e.g. `trySignErc20Approval` running before
-// the deposit payload has been re-resolved). Approximates with the per-request
-// amount — sufficient for the allowance short-circuit since deposit ≥ amount
-// always holds for batched flows.
-func readDepositAmountFromAnyPayload(_ *BatchedEvmScheme, requirements types.PaymentRequirements) (string, bool) {
-	if requirements.Amount == "" {
-		return "", false
-	}
-	return requirements.Amount, true
-}
-
-// readPermit2DeadlineFromBatchedDeposit extracts the deadline from the batched
-// deposit's Permit2 authorization map: payload.deposit.authorization.permit2Authorization.deadline.
-// Mirrors the TS scheme which aligns the EIP-2612 deadline with the Permit2
-// deadline so both expire together.
-func readPermit2DeadlineFromBatchedDeposit(payload types.PaymentPayload) string {
-	if payload.Payload == nil {
-		return ""
-	}
-	dep, ok := payload.Payload["deposit"].(map[string]interface{})
-	if !ok {
-		return ""
-	}
-	auth, ok := dep["authorization"].(map[string]interface{})
-	if !ok {
-		return ""
-	}
-	permit2, ok := auth["permit2Authorization"].(map[string]interface{})
-	if !ok {
-		return ""
-	}
-	d, _ := permit2["deadline"].(string)
-	return d
-}
-
 // hasSufficientPermit2Allowance returns true when `owner` has already
 // approved at least `requiredAmount` to Permit2 for `tokenAddress`. Returns
 // false on any RPC error so we conservatively sign an extension rather than

go/mechanisms/evm/batched/client/extensions_test.go

@@ -290,7 +290,15 @@ func TestCreatePaymentPayloadWithExtensions_Eip2612DeadlineFromPermit2(t *testin
 	}
 	wrapper := out.Extensions[eip2612gassponsor.EIP2612GasSponsoring.Key()].(map[string]interface{})
 	info := wrapper["info"].(*eip2612gassponsor.Info)
-	depDeadline := readPermit2DeadlineFromBatchedDeposit(out)
+	// Read the deadline straight off the deposit payload — the alignment
+	// invariant being tested is that whatever value lands in the Permit2
+	// authorization (`payload.deposit.authorization.permit2Authorization.deadline`)
+	// is reused for the EIP-2612 permit. Inlined here because the helper
+	// was removed (single-call site) and the path is short.
+	dep, _ := out.Payload["deposit"].(map[string]interface{})
+	auth, _ := dep["authorization"].(map[string]interface{})
+	permit2, _ := auth["permit2Authorization"].(map[string]interface{})
+	depDeadline, _ := permit2["deadline"].(string)
 	if depDeadline == "" {
 		t.Fatal("permit2 deadline missing from deposit payload")
 	}

go/mechanisms/evm/batched/facilitator/deposit.go

@@ -214,16 +214,30 @@ func VerifyDeposit(
 			collectorData,
 		)
 		if simErr != nil {
-			invalidReason := ErrDepositSimulationFailed //nolint:ineffassign // overwritten on diagnosis
+			// Diagnose the most common standard-Permit2-path simulation
+			// revert: the user hasn't approved Permit2. We probe
+			// `allowance(payer, Permit2)` and surface the dedicated reason
+			// when it's below the deposit amount; any other revert (signature
+			// invalidation, balance, etc.) passes through as the generic
+			// ErrDepositSimulationFailed. Mirrors exact's
+			// `CheckPermit2Prerequisites` diagnosis. RPC failures during the
+			// probe also fall through to the generic reason.
+			invalidReason := ErrDepositSimulationFailed
 			if transferMethod == batched.AssetTransferMethodPermit2 &&
 				(permit2Branch == nil || permit2Branch.kind == permit2BranchStandard) {
-				if probedReason := diagnosePermit2AllowanceShortfall(ctx, signer, config, depositAmount); probedReason != "" {
-					invalidReason = probedReason
-				} else {
-					invalidReason = ErrDepositSimulationFailed
+				if allowanceResult, allowErr := signer.ReadContract(
+					ctx,
+					config.Token,
+					evm.ERC20AllowanceABI,
+					"allowance",
+					common.HexToAddress(config.Payer),
+					common.HexToAddress(evm.PERMIT2Address),
+				); allowErr == nil {
+					if allowance, ok := allowanceResult.(*big.Int); ok && allowance != nil &&
+						allowance.Cmp(depositAmount) < 0 {
+						invalidReason = ErrPermit2AllowanceRequired
+					}
 				}
-			} else {
-				invalidReason = ErrDepositSimulationFailed
 			}
 			return &x402.VerifyResponse{ //nolint:nilerr // simulation failure → error encoded in response
 				IsValid:       false,
@@ -691,40 +705,3 @@ func verifyPermit2DepositAuthorization(
 	}
 	return "", nil
 }
-
-// diagnosePermit2AllowanceShortfall is called after a standard-path Permit2
-// deposit simulation reverts. It probes the on-chain ERC-20 allowance
-// payer→Permit2 and returns `ErrPermit2AllowanceRequired` when the allowance
-// is strictly less than the deposit amount (the canonical cause of the most
-// common standard-path simulation revert). On any RPC error or sufficient
-// allowance the helper returns "" so the caller falls back to the generic
-// `ErrDepositSimulationFailed` reason. Mirrors exact's
-// `CheckPermit2Prerequisites` diagnosis pattern but kept inline because the
-// batched scheme needs only this single check (other reverts pass through as
-// generic simulation failures).
-func diagnosePermit2AllowanceShortfall(
-	ctx context.Context,
-	signer evm.FacilitatorEvmSigner,
-	config batched.ChannelConfig,
-	depositAmount *big.Int,
-) string {
-	allowanceResult, err := signer.ReadContract(
-		ctx,
-		config.Token,
-		evm.ERC20AllowanceABI,
-		"allowance",
-		common.HexToAddress(config.Payer),
-		common.HexToAddress(evm.PERMIT2Address),
-	)
-	if err != nil {
-		return ""
-	}
-	allowance, ok := allowanceResult.(*big.Int)
-	if !ok || allowance == nil {
-		return ""
-	}
-	if allowance.Cmp(depositAmount) < 0 {
-		return ErrPermit2AllowanceRequired
-	}
-	return ""
-}