From 2f07e4e8b397609ff620971de3e2e967c76091ee Mon Sep 17 00:00:00 2001 From: Sasha Mitchell Date: Sat, 8 Aug 2026 09:32:30 +0700 Subject: [PATCH] fix(cdp): exact Permit2 allowance and no swap-submit retry Approve only the swap fromAmount to Permit2 (TS+Python, EVM+smart) instead of maxUint256, and submit swap once so a post-broadcast throw cannot double-execute. Co-authored-by: Cursor --- .../cdp/cdp_evm_wallet_action_provider.py | 29 +++++------ .../cdp/cdp_smart_wallet_action_provider.py | 31 +++++------- .../cdp/cdpEvmWalletActionProvider.test.ts | 11 +++++ .../cdp/cdpEvmWalletActionProvider.ts | 44 ++++++++--------- .../cdp/cdpSmartWalletActionProvider.test.ts | 11 +++++ .../cdp/cdpSmartWalletActionProvider.ts | 48 +++++++++---------- 6 files changed, 90 insertions(+), 84 deletions(-) diff --git a/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_evm_wallet_action_provider.py b/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_evm_wallet_action_provider.py index 7b5f541c5..25f692899 100644 --- a/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_evm_wallet_action_provider.py +++ b/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_evm_wallet_action_provider.py @@ -18,7 +18,6 @@ format_units, get_token_details, parse_units, - retry_with_exponential_backoff, ) TWalletProvider = TypeVar("TWalletProvider", bound=CdpEvmWalletProvider) @@ -161,7 +160,7 @@ async def _get_swap_price(): - slippage_bps: (Optional) Maximum allowed slippage in basis points (100 = 1%) Important notes: - The contract address for native ETH is "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" -- If needed, it will automatically approve the permit2 contract to spend the fromToken +- If needed, it will automatically approve the permit2 contract for this from_amount only (not unlimited) - Use from_amount units exactly as provided, do not convert to wei or any other units. """, schema=SwapSchema, @@ -210,13 +209,15 @@ async def _execute_swap(): # Get the account account = await cdp.evm.get_account(address=wallet_provider.get_address()) + from_amount_atomic = parse_units( + validated_args.from_amount, from_token_decimals + ) + # Estimate swap price first to check liquidity, token balance and permit2 approval status swap_quote = await account.quote_swap( from_token=validated_args.from_token, to_token=validated_args.to_token, - from_amount=str( - parse_units(validated_args.from_amount, from_token_decimals) - ), + from_amount=str(from_amount_atomic), network=cdp_network, ) @@ -238,20 +239,20 @@ async def _execute_swap(): "error": f"Balance is not enough to perform swap. Required: {validated_args.from_amount} {from_token_name}, but only have {format_units(swap_quote.issues.balance.current_balance, from_token_decimals)} {from_token_name} ({validated_args.from_token})", } - # Check if allowance is enough + # Approve only this swap's from_amount (never max uint256) so a later + # compromised path cannot inherit an unlimited Permit2 allowance. approval_tx_hash = None if ( hasattr(swap_quote, "issues") and swap_quote.issues and hasattr(swap_quote.issues, "allowance") ): - # Send approval transaction approve_data = ( Web3() .eth.contract(abi=ERC20_ABI) .encodeABI( fn_name="approve", - args=[PERMIT2_ADDRESS, 2**256 - 1], # Max uint256 + args=[PERMIT2_ADDRESS, from_amount_atomic], ) ) @@ -273,15 +274,9 @@ async def _execute_swap(): if receipt.status != "success": return {"success": False, "error": "Approval transaction failed"} - # Execute swap using the all-in-one pattern with retry logic - async def _perform_swap(): - return await swap_quote.execute() - - swap_result = await retry_with_exponential_backoff( - _perform_swap, - max_retries=3, - base_delay=5.0, - ) + # Submit swap once. Do not retry submission: a throw after broadcast can + # cause a second unintended swap (same class as false-failure retries). + swap_result = await swap_quote.execute() receipt = await wallet_provider.wait_for_transaction_receipt( swap_result.transaction_hash diff --git a/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_smart_wallet_action_provider.py b/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_smart_wallet_action_provider.py index 2b936286e..573c10401 100644 --- a/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_smart_wallet_action_provider.py +++ b/python/coinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_smart_wallet_action_provider.py @@ -19,7 +19,6 @@ format_units, get_token_details, parse_units, - retry_with_exponential_backoff, ) TWalletProvider = TypeVar("TWalletProvider", bound=CdpSmartWalletProvider) @@ -183,7 +182,7 @@ async def _get_swap_price(): - slippage_bps: (Optional) Maximum allowed slippage in basis points (100 = 1%) Important notes: - The contract address for native ETH is "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" -- If needed, it will automatically approve the permit2 contract to spend the fromToken +- If needed, it will automatically approve the permit2 contract for this from_amount only (not unlimited) - Use from_amount units exactly as provided, do not convert to wei or any other units. """, schema=SwapSchema, @@ -244,15 +243,15 @@ async def _execute_swap(): # Get the smart account smart_account = await wallet_provider._get_smart_account(cdp) + from_amount_atomic = parse_units( + validated_args.from_amount, token_details["from_token_decimals"] + ) + # Quote swap first to check liquidity, token balance and permit2 approval status swap_quote = await smart_account.quote_swap( from_token=validated_args.from_token, to_token=validated_args.to_token, - from_amount=str( - parse_units( - validated_args.from_amount, token_details["from_token_decimals"] - ) - ), + from_amount=str(from_amount_atomic), network=cdp_network, paymaster_url=wallet_provider._paymaster_url, ) @@ -275,20 +274,20 @@ async def _execute_swap(): "error": f"Balance is not enough to perform swap. Required: {validated_args.from_amount} {token_details['from_token_name']}, but only have {format_units(swap_quote.issues.balance.current_balance, token_details['from_token_decimals'])} {token_details['from_token_name']} ({validated_args.from_token})", } - # Check if allowance is enough + # Approve only this swap's from_amount (never max uint256) so a later + # compromised path cannot inherit an unlimited Permit2 allowance. approval_tx_hash = None if ( hasattr(swap_quote, "issues") and swap_quote.issues and hasattr(swap_quote.issues, "allowance") ): - # Send approval transaction approve_data = ( Web3() .eth.contract(abi=ERC20_ABI) .encodeABI( fn_name="approve", - args=[PERMIT2_ADDRESS, 2**256 - 1], # Max uint256 + args=[PERMIT2_ADDRESS, from_amount_atomic], ) ) @@ -304,15 +303,9 @@ async def _execute_swap(): if receipt.status != "complete": return {"success": False, "error": "Approval transaction failed"} - # Execute swap using the all-in-one pattern with retry logic - async def _perform_swap(): - return await swap_quote.execute() - - swap_result = await retry_with_exponential_backoff( - _perform_swap, - max_retries=3, - base_delay=5.0, - ) + # Submit swap once. Do not retry submission: a throw after broadcast can + # cause a second unintended swap (same class as false-failure retries). + swap_result = await swap_quote.execute() receipt = await smart_account.wait_for_user_operation( user_op_hash=swap_result.user_op_hash diff --git a/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.test.ts b/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.test.ts index f6503aa63..590c1c813 100644 --- a/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.test.ts +++ b/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { CdpClient, SpendPermissionNetwork } from "@coinbase/cdp-sdk"; +import { decodeFunctionData, erc20Abi, maxUint256 } from "viem"; import { CdpEvmWalletProvider } from "../../wallet-providers/cdpEvmWalletProvider"; import { CdpEvmWalletActionProvider } from "./cdpEvmWalletActionProvider"; import { ListSpendPermissionsSchema, UseSpendPermissionSchema, SwapSchema } from "./schemas"; @@ -51,6 +52,7 @@ describe("CDP EVM Wallet Action Provider", () => { mockRetryWithExponentialBackoff.mockImplementation(async (fn: any) => { return await fn(); }); + (swapUtils as any).PERMIT2_ADDRESS = "0x000000000022D473030F116dDEE9F6B43aC78BA3"; actionProvider = new CdpEvmWalletActionProvider(); }); @@ -557,6 +559,15 @@ describe("CDP EVM Wallet Action Provider", () => { const parsedResult = JSON.parse(result); expect(mockWalletProvider.sendTransaction).toHaveBeenCalled(); + const approvalCall = mockWalletProvider.sendTransaction.mock.calls[0][0]; + expect(approvalCall.to).toBe(mockArgs.fromToken); + const decodedApproval = decodeFunctionData({ + abi: erc20Abi, + data: approvalCall.data, + }); + expect(decodedApproval.functionName).toBe("approve"); + expect(decodedApproval.args?.[1]).toBe(100000000000000000n); // 0.1 ETH exact + expect(decodedApproval.args?.[1]).not.toBe(maxUint256); expect(parsedResult.success).toBe(true); expect(parsedResult.approvalTxHash).toBe("0xapproval123"); expect(parsedResult.transactionHash).toBe("0xswap789"); diff --git a/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.ts b/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.ts index 44038f267..50e55f921 100644 --- a/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.ts +++ b/typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.ts @@ -8,8 +8,7 @@ import { ActionProvider } from "../actionProvider"; import { UseSpendPermissionSchema, ListSpendPermissionsSchema, SwapSchema } from "./schemas"; import { listSpendPermissionsForSpender, findLatestSpendPermission } from "./spendPermissionUtils"; import { getTokenDetails, PERMIT2_ADDRESS } from "./swapUtils"; -import { Hex, formatUnits, parseUnits, maxUint256, encodeFunctionData, erc20Abi } from "viem"; -import { retryWithExponentialBackoff } from "../../utils"; +import { Hex, formatUnits, parseUnits, encodeFunctionData, erc20Abi } from "viem"; import type { Network } from "../../network"; import type { Address } from "viem"; @@ -221,7 +220,7 @@ It takes the following inputs: - slippageBps: (Optional) Maximum allowed slippage in basis points (100 = 1%) Important notes: - The contract address for native ETH is "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" -- If needed, it will automatically approve the permit2 contract to spend the fromToken +- If needed, it will automatically approve the permit2 contract for this fromAmount only (not unlimited) - Use fromAmount units exactly as provided, do not convert to wei or any other units. - Never assume token or address, they have to be provided as inputs. If only token symbol is provided, use the get_token_address tool if available to get the token address first `, @@ -248,6 +247,8 @@ Important notes: const { fromTokenDecimals, fromTokenName, toTokenName, toTokenDecimals } = await getTokenDetails(walletProvider, args.fromToken, args.toToken); + const fromAmountAtomic = parseUnits(args.fromAmount, fromTokenDecimals); + // Get the account const account = await walletProvider.getClient().evm.getAccount({ address: walletProvider.getAddress() as Hex, @@ -257,7 +258,7 @@ Important notes: const swapPrice = await walletProvider.getClient().evm.getSwapPrice({ fromToken: args.fromToken as Hex, toToken: args.toToken as Hex, - fromAmount: parseUnits(args.fromAmount, fromTokenDecimals), + fromAmount: fromAmountAtomic, // eslint-disable-next-line @typescript-eslint/no-explicit-any network: cdpNetwork as any, taker: account.address as Hex, @@ -282,7 +283,8 @@ Important notes: }); } - // Check if allowance is enough + // Approve only this swap's fromAmount (never maxUint256) so a later + // compromised path cannot inherit an unlimited Permit2 allowance. let approvalTxHash: Hex | null = null; if (swapPrice.issues.allowance) { try { @@ -291,7 +293,7 @@ Important notes: data: encodeFunctionData({ abi: erc20Abi, functionName: "approve", - args: [PERMIT2_ADDRESS, maxUint256], + args: [PERMIT2_ADDRESS, fromAmountAtomic], }), }); @@ -310,23 +312,19 @@ Important notes: } } - // Execute swap using the all-in-one pattern with retry logic - const swapResult = await retryWithExponentialBackoff( - async () => { - return (await account.swap({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - network: cdpNetwork as any, - fromToken: args.fromToken as Hex, - toToken: args.toToken as Hex, - fromAmount: parseUnits(args.fromAmount, fromTokenDecimals), - slippageBps: args.slippageBps, - signerAddress: account.address as Hex, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - })) as any; - }, - 3, - 5000, - ); // Max 3 retries with 5s base delay + // Submit swap once. Do not retry submission: a throw after broadcast can + // cause a second unintended swap (same class as false-failure retries). + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const swapResult = (await account.swap({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + network: cdpNetwork as any, + fromToken: args.fromToken as Hex, + toToken: args.toToken as Hex, + fromAmount: fromAmountAtomic, + slippageBps: args.slippageBps, + signerAddress: account.address as Hex, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + })) as any; // Check if swap was successful const swapReceipt = await walletProvider.waitForTransactionReceipt( diff --git a/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.test.ts b/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.test.ts index ad87fe1f9..414cf3106 100644 --- a/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.test.ts +++ b/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.test.ts @@ -1,5 +1,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { CdpClient, SpendPermissionNetwork } from "@coinbase/cdp-sdk"; +import { decodeFunctionData, erc20Abi, maxUint256 } from "viem"; import { CdpSmartWalletProvider } from "../../wallet-providers/cdpSmartWalletProvider"; import { CdpSmartWalletActionProvider } from "./cdpSmartWalletActionProvider"; import { ListSpendPermissionsSchema, UseSpendPermissionSchema } from "./schemas"; @@ -56,6 +57,7 @@ describe("CDP Smart Wallet Action Provider", () => { mockRetryWithExponentialBackoff.mockImplementation(async (fn: any) => { return await fn(); }); + (swapUtils as any).PERMIT2_ADDRESS = "0x000000000022D473030F116dDEE9F6B43aC78BA3"; actionProvider = new CdpSmartWalletActionProvider(); }); @@ -529,6 +531,15 @@ describe("CDP Smart Wallet Action Provider", () => { const parsedResult = JSON.parse(result); expect(mockWalletProvider.sendTransaction).toHaveBeenCalled(); + const approvalCall = mockWalletProvider.sendTransaction.mock.calls[0][0]; + expect(approvalCall.to).toBe(mockArgs.fromToken); + const decodedApproval = decodeFunctionData({ + abi: erc20Abi, + data: approvalCall.data, + }); + expect(decodedApproval.functionName).toBe("approve"); + expect(decodedApproval.args?.[1]).toBe(100000000000000000n); // 0.1 ETH exact + expect(decodedApproval.args?.[1]).not.toBe(maxUint256); expect(parsedResult.success).toBe(true); expect(parsedResult.approvalTxHash).toBe("0xapproval123"); expect(parsedResult.transactionHash).toBe("0xswap789"); diff --git a/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.ts b/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.ts index 3fe7c45f3..4c94def66 100644 --- a/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.ts +++ b/typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.ts @@ -6,8 +6,7 @@ import { ActionProvider } from "../actionProvider"; import { UseSpendPermissionSchema, ListSpendPermissionsSchema, SwapSchema } from "./schemas"; import { listSpendPermissionsForSpender, findLatestSpendPermission } from "./spendPermissionUtils"; import { getTokenDetails, PERMIT2_ADDRESS } from "./swapUtils"; -import { Hex, formatUnits, parseUnits, maxUint256, encodeFunctionData, erc20Abi } from "viem"; -import { retryWithExponentialBackoff } from "../../utils"; +import { Hex, formatUnits, parseUnits, encodeFunctionData, erc20Abi } from "viem"; import type { Network } from "../../network"; import type { Address } from "viem"; @@ -206,7 +205,7 @@ It takes the following inputs: - slippageBps: (Optional) Maximum allowed slippage in basis points (100 = 1%) Important notes: - The contract address for native ETH is "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" -- If needed, it will automatically approve the permit2 contract to spend the fromToken +- If needed, it will automatically approve the permit2 contract for this fromAmount only (not unlimited) - Use fromAmount units exactly as provided, do not convert to wei or any other units - Never assume token or address, they have to be provided as inputs. If only token symbol is provided, use the get_token_address tool if available to get the token address first `, @@ -239,11 +238,13 @@ Important notes: const { fromTokenDecimals, fromTokenName, toTokenName, toTokenDecimals } = await getTokenDetails(walletProvider, args.fromToken, args.toToken); + const fromAmountAtomic = parseUnits(args.fromAmount, fromTokenDecimals); + // Estimate swap price first to check liquidity, token balance and permit2 approval status const swapPrice = await walletProvider.getClient().evm.getSwapPrice({ fromToken: args.fromToken as Hex, toToken: args.toToken as Hex, - fromAmount: parseUnits(args.fromAmount, fromTokenDecimals), + fromAmount: fromAmountAtomic, // eslint-disable-next-line @typescript-eslint/no-explicit-any network: cdpNetwork as any, taker: walletProvider.smartAccount.address as Hex, @@ -268,7 +269,8 @@ Important notes: }); } - // Check if allowance is enough + // Approve only this swap's fromAmount (never maxUint256) so a later + // compromised path cannot inherit an unlimited Permit2 allowance. let approvalTxHash: Hex | null = null; if (swapPrice.issues.allowance) { try { @@ -277,7 +279,7 @@ Important notes: data: encodeFunctionData({ abi: erc20Abi, functionName: "approve", - args: [PERMIT2_ADDRESS, maxUint256], + args: [PERMIT2_ADDRESS, fromAmountAtomic], }), }); @@ -296,25 +298,21 @@ Important notes: } } - // Execute swap using the all-in-one pattern with retry logic - const swapResult = await retryWithExponentialBackoff( - async () => { - return (await walletProvider.smartAccount.swap({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - network: cdpNetwork as any, - fromToken: args.fromToken as Hex, - toToken: args.toToken as Hex, - fromAmount: parseUnits(args.fromAmount, fromTokenDecimals), - slippageBps: args.slippageBps, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - paymasterUrl: walletProvider.getPaymasterUrl(), - signerAddress: walletProvider.ownerAccount.address as Hex, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - })) as any; - }, - 3, - 5000, - ); // Max 3 retries with 5s base delay + // Submit swap once. Do not retry submission: a throw after broadcast can + // cause a second unintended swap (same class as false-failure retries). + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const swapResult = (await walletProvider.smartAccount.swap({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + network: cdpNetwork as any, + fromToken: args.fromToken as Hex, + toToken: args.toToken as Hex, + fromAmount: fromAmountAtomic, + slippageBps: args.slippageBps, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + paymasterUrl: walletProvider.getPaymasterUrl(), + signerAddress: walletProvider.ownerAccount.address as Hex, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + })) as any; // Check if swap was successful const swapReceipt = await walletProvider.waitForTransactionReceipt(swapResult.userOpHash);