Skip to content

fix(cdp): exact Permit2 allowance and no swap-submit retry #1409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SashaMIT wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(cdp): exact Permit2 allowance and no swap-submit retry #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 12 additions & 17 deletions ...oinbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_evm_wallet_action_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
format_units,
get_token_details,
parse_units,
retry_with_exponential_backoff,
)

TWalletProvider = TypeVar("TWalletProvider", bound=CdpEvmWalletProvider)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)

Expand All @@ -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],
)
)

Expand All @@ -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
Expand Down
31 changes: 12 additions & 19 deletions ...nbase-agentkit/coinbase_agentkit/action_providers/cdp/cdp_smart_wallet_action_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
format_units,
get_token_details,
parse_units,
retry_with_exponential_backoff,
)

TWalletProvider = TypeVar("TWalletProvider", bound=CdpSmartWalletProvider)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
Expand All @@ -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],
)
)

Expand All @@ -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
Expand Down
11 changes: 11 additions & 0 deletions typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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");
Expand Down
44 changes: 21 additions & 23 deletions typescript/agentkit/src/action-providers/cdp/cdpEvmWalletActionProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
`,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -291,7 +293,7 @@ Important notes:
data: encodeFunctionData({
abi: erc20Abi,
functionName: "approve",
args: [PERMIT2_ADDRESS, maxUint256],
args: [PERMIT2_ADDRESS, fromAmountAtomic],
}),
});

Expand All @@ -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(
Expand Down
11 changes: 11 additions & 0 deletions typescript/agentkit/src/action-providers/cdp/cdpSmartWalletActionProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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");
Expand Down
Loading
Loading