Prefund privy EOA for Safe x402 payments #939
Conversation
Co-authored-by: hyacinthus <488292+hyacinthus@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request implements Safe wallet support for x402 payments by introducing a prefunding mechanism. When a Safe-based agent needs to make an x402 payment, the system now checks the Privy EOA balance and transfers the required amount from the Safe wallet if needed, before the EOA signs the EIP-3009 payment.
Changes:
- Added prefunding logic to check Privy EOA balance and transfer tokens from Safe when insufficient
- Removed the previous approach that attempted to use Safe address for signing (reverted to direct EOA signing)
- Updated PrivyWalletSigner to remove the from_address parameter, now using only the EOA address
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| intentkit/skills/x402/base.py | Added core prefunding logic with methods to check payment requirements, query ERC20 balances, and transfer funds from Safe to Privy EOA |
| intentkit/skills/x402/pay.py | Integrated _prefund_safe_wallet call before x402 payment with max_value enforcement |
| intentkit/skills/x402/http_request.py | Integrated _prefund_safe_wallet call before x402 HTTP request |
| intentkit/clients/privy.py | Simplified PrivyWalletSigner by removing from_address parameter and related logic |
| tests/skills/test_x402_safe_funding.py | Added tests for Safe prefunding behavior covering sufficient and insufficient balance scenarios |
Comments suppressed due to low confidence (1)
intentkit/clients/privy.py:3041
- Except block directly handles BaseException.
except BaseException as exc:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return | ||
| requirement = self._select_payment_requirement(payment_response) | ||
| if requirement is None: | ||
| return |
There was a problem hiding this comment.
The error handling for missing or invalid payment requirements could be more specific. When requirement.asset is missing (line 315), the error is raised immediately. However, if the selected requirement is None (line 313), the function silently returns without funding. This inconsistency could make debugging difficult.
Consider raising a ValueError when requirement is None to make the error case explicit, or document why this is an acceptable no-op (e.g., if the endpoint doesn't require payment).
| return | |
| raise ValueError( | |
| "No suitable payment requirement found for Safe wallet prefunding." | |
| ) |
| privy_wallet_data.get("network_id") | ||
| or agent.network_id | ||
| or network |
There was a problem hiding this comment.
The network_id resolution logic uses a chain of fallbacks (privy_wallet_data.network_id → agent.network_id → network parameter → DEFAULT_NETWORK_ID), but the order could be confusing. The network parameter is checked last, after agent.network_id, which means an explicit network parameter passed to _ensure_safe_funding would be overridden by agent.network_id if present.
Consider reordering the fallback chain to respect explicit parameters first: network parameter → privy_wallet_data.network_id → agent.network_id → DEFAULT_NETWORK_ID. This would make the behavior more intuitive and predictable.
| privy_wallet_data.get("network_id") | |
| or agent.network_id | |
| or network | |
| network | |
| or privy_wallet_data.get("network_id") | |
| or agent.network_id |
| rpc_url=rpc_url, | ||
| ) | ||
|
|
||
| async def _prefund_safe_wallet( |
There was a problem hiding this comment.
The method name _prefund_safe_wallet suggests it prefunds the Safe wallet, but it actually prefunds the Privy EOA from the Safe wallet. The name could be misleading.
Consider renaming to _prefund_privy_eoa_from_safe or _ensure_privy_eoa_funded to better reflect what the method actually does.
| await self._prefund_safe_wallet( | ||
| method=method_upper, | ||
| request_kwargs=request_kwargs, | ||
| timeout=timeout, | ||
| ) |
There was a problem hiding this comment.
The _prefund_safe_wallet call in http_request.py does not pass a max_value parameter, unlike the call in pay.py. This means x402_http_request operations cannot enforce a maximum payment limit during prefunding, while x402_pay operations can.
Consider whether x402_http_request should also support a max_value parameter for consistency, or document why it's intentionally omitted (e.g., if http_request is meant for non-payment operations).
| return | ||
| if not requirement.asset: | ||
| raise ValueError("Payment requirement missing asset address.") | ||
| max_amount = getattr(requirement, "max_amount_required", None) |
There was a problem hiding this comment.
The code uses getattr(requirement, "max_amount_required", None) to safely access the attribute, but then checks if it's None and raises an error. This pattern is inconsistent - either the attribute is expected to exist (use direct access and handle AttributeError) or it's optional (don't raise an error if None).
Consider using hasattr() or direct attribute access with try/except for clarity, or document why max_amount_required might not exist on a valid requirement object.
| max_amount = getattr(requirement, "max_amount_required", None) | |
| try: | |
| max_amount = requirement.max_amount_required | |
| except AttributeError as exc: | |
| raise ValueError("Payment requirement missing amount value.") from exc |
| import json | ||
| from unittest.mock import AsyncMock, MagicMock, patch | ||
|
|
||
| import pytest | ||
|
|
||
| from intentkit.skills.x402.pay import X402Pay | ||
|
|
||
| INSUFFICIENT_BALANCE = 5 | ||
| REQUIRED_AMOUNT = 10 | ||
| SUFFICIENT_BALANCE = 25 | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_safe_funding_transfers_when_balance_insufficient(): | ||
| skill = X402Pay() | ||
| mock_agent = MagicMock() | ||
| mock_agent.wallet_provider = "safe" | ||
| mock_agent.id = "agent-id" | ||
| mock_agent.network_id = "base-mainnet" | ||
|
|
||
| mock_context = MagicMock() | ||
| mock_context.agent = mock_agent | ||
|
|
||
| privy_wallet_data = { | ||
| "privy_wallet_id": "wallet-id", | ||
| "privy_wallet_address": "0x1111111111111111111111111111111111111111", | ||
| "smart_wallet_address": "0x2222222222222222222222222222222222222222", | ||
| "network_id": "base-mainnet", | ||
| } | ||
| agent_data = MagicMock() | ||
| agent_data.privy_wallet_data = json.dumps(privy_wallet_data) | ||
|
|
||
| with ( | ||
| patch( | ||
| "intentkit.skills.base.IntentKitSkill.get_context", | ||
| return_value=mock_context, | ||
| ), | ||
| patch( | ||
| "intentkit.models.agent_data.AgentData.get", | ||
| new=AsyncMock(return_value=agent_data), | ||
| ), | ||
| patch.object(skill, "_resolve_rpc_url", return_value="https://rpc.example"), | ||
| patch.object( | ||
| skill, | ||
| "_get_erc20_balance", | ||
| new=AsyncMock(return_value=INSUFFICIENT_BALANCE), | ||
| ), | ||
| patch( | ||
| "intentkit.skills.x402.base.transfer_erc20_gasless", | ||
| new=AsyncMock(return_value="0xhash"), | ||
| ) as mock_transfer, | ||
| ): | ||
| await skill._ensure_safe_funding( | ||
| amount=REQUIRED_AMOUNT, | ||
| token_address="0x3333333333333333333333333333333333333333", | ||
| max_value=REQUIRED_AMOUNT, | ||
| ) | ||
|
|
||
| mock_transfer.assert_awaited_once() | ||
| call_kwargs = mock_transfer.call_args.kwargs | ||
| assert ( | ||
| call_kwargs["amount"] == REQUIRED_AMOUNT - INSUFFICIENT_BALANCE | ||
| ) # required - current_balance | ||
| assert call_kwargs["to"] == privy_wallet_data["privy_wallet_address"] | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_safe_funding_skips_when_balance_sufficient(): | ||
| skill = X402Pay() | ||
| mock_agent = MagicMock() | ||
| mock_agent.wallet_provider = "safe" | ||
| mock_agent.id = "agent-id" | ||
| mock_agent.network_id = "base-mainnet" | ||
|
|
||
| mock_context = MagicMock() | ||
| mock_context.agent = mock_agent | ||
|
|
||
| privy_wallet_data = { | ||
| "privy_wallet_id": "wallet-id", | ||
| "privy_wallet_address": "0x1111111111111111111111111111111111111111", | ||
| "smart_wallet_address": "0x2222222222222222222222222222222222222222", | ||
| "network_id": "base-mainnet", | ||
| } | ||
| agent_data = MagicMock() | ||
| agent_data.privy_wallet_data = json.dumps(privy_wallet_data) | ||
|
|
||
| with ( | ||
| patch( | ||
| "intentkit.skills.base.IntentKitSkill.get_context", | ||
| return_value=mock_context, | ||
| ), | ||
| patch( | ||
| "intentkit.models.agent_data.AgentData.get", | ||
| new=AsyncMock(return_value=agent_data), | ||
| ), | ||
| patch.object(skill, "_resolve_rpc_url", return_value="https://rpc.example"), | ||
| patch.object( | ||
| skill, | ||
| "_get_erc20_balance", | ||
| new=AsyncMock(return_value=SUFFICIENT_BALANCE), | ||
| ), | ||
| patch( | ||
| "intentkit.skills.x402.base.transfer_erc20_gasless", | ||
| new=AsyncMock(return_value="0xhash"), | ||
| ) as mock_transfer, | ||
| ): | ||
| await skill._ensure_safe_funding( | ||
| amount=REQUIRED_AMOUNT, | ||
| token_address="0x3333333333333333333333333333333333333333", | ||
| max_value=REQUIRED_AMOUNT, | ||
| ) | ||
|
|
||
| mock_transfer.assert_not_called() |
There was a problem hiding this comment.
The test suite only covers _ensure_safe_funding directly but doesn't test the complete _prefund_safe_wallet flow, which includes:
- Making the HTTP request to get payment requirements
- Selecting the appropriate payment requirement (eip3009 vs fallback)
- Error cases (missing asset, invalid amount, etc.)
Consider adding tests for:
- _prefund_safe_wallet with a 402 response containing payment requirements
- _prefund_safe_wallet with a non-402 response (should return early)
- _prefund_safe_wallet with missing asset in payment requirement
- _prefund_safe_wallet with invalid amount format
- _select_payment_requirement preferring eip3009 over other schemes
| if max_value is not None and amount > max_value: | ||
| raise ValueError( | ||
| f"Payment amount {amount} exceeds max_value {max_value}." | ||
| ) |
There was a problem hiding this comment.
The max_value validation in _ensure_safe_funding checks if the payment amount exceeds max_value, but this check occurs before the balance check. This means if a user has max_value=10, amount=15, and balance=20, the function will raise an error even though they have sufficient balance and wouldn't need any funding.
Consider moving the max_value check to after the balance check, or only validating the transfer_amount against max_value. This would allow users with pre-existing balances to make payments that exceed max_value as long as they don't require additional funding.
| agent = self.get_context().agent | ||
| if not agent or agent.wallet_provider != "safe": | ||
| return | ||
| if amount <= 0: |
There was a problem hiding this comment.
The early return when amount <= 0 (line 239-240) silently accepts zero or negative payment amounts without logging or raising an error. While this prevents invalid transfers, it could hide bugs where the amount calculation is incorrect.
Consider logging a warning when amount <= 0 is encountered, as this likely indicates an upstream issue in payment requirement parsing.
| if amount <= 0: | |
| if amount <= 0: | |
| logger.warning( | |
| "Encountered non-positive payment amount in _ensure_safe_funding; " | |
| "amount=%s, agent_id=%s, token_address=%s", | |
| amount, | |
| getattr(agent, "id", None) if agent else None, | |
| token_address, | |
| ) |
| privy_wallet_address = privy_wallet_data["privy_wallet_address"] | ||
| safe_address = privy_wallet_data["smart_wallet_address"] | ||
| except KeyError as exc: | ||
| raise ValueError("Privy wallet data missing required fields.") from exc |
There was a problem hiding this comment.
The error message "Privy wallet data missing required fields" doesn't specify which field is missing. When debugging, it would be helpful to know whether privy_wallet_id, privy_wallet_address, or smart_wallet_address is absent.
Consider including the missing field name in the error message, e.g., f"Privy wallet data missing required field: {exc}".
| raise ValueError("Privy wallet data missing required fields.") from exc | |
| raise ValueError(f"Privy wallet data missing required field: {exc}") from exc |
| async def _prefund_safe_wallet( | ||
| self, | ||
| *, | ||
| method: str, | ||
| request_kwargs: dict[str, Any], | ||
| timeout: float, | ||
| max_value: int | None = None, | ||
| ) -> None: | ||
| agent = self.get_context().agent | ||
| if not agent or agent.wallet_provider != "safe": | ||
| return | ||
| payment_response = await self._get_payment_requirement( | ||
| method=method, | ||
| request_kwargs=request_kwargs, | ||
| timeout=timeout, | ||
| ) | ||
| if payment_response is None: | ||
| return | ||
| requirement = self._select_payment_requirement(payment_response) | ||
| if requirement is None: | ||
| return | ||
| if not requirement.asset: | ||
| raise ValueError("Payment requirement missing asset address.") | ||
| max_amount = getattr(requirement, "max_amount_required", None) | ||
| if max_amount is None: | ||
| raise ValueError("Payment requirement missing amount value.") | ||
| try: | ||
| amount = int(max_amount) | ||
| except (TypeError, ValueError) as exc: | ||
| raise ValueError( | ||
| "Payment amount must be a valid integer, got " | ||
| f"{max_amount} ({type(max_amount).__name__})." | ||
| ) from exc | ||
| await self._ensure_safe_funding( | ||
| amount=amount, | ||
| token_address=requirement.asset, | ||
| max_value=max_value, | ||
| network=requirement.network, | ||
| ) |
There was a problem hiding this comment.
The _prefund_safe_wallet method makes an HTTP request to determine payment requirements, but this same request will be made again immediately after by x402HttpxClient. This results in a double HTTP request for every x402 operation with Safe wallets, which is inefficient and could cause issues if the endpoint has rate limiting or if state changes between requests.
Consider either:
- Caching the payment response and reusing it, or
- Using a different approach to get the payment amount, such as extracting it from the x402HttpxClient's initial 402 response (if the library exposes it), or
- Documenting this as an acceptable tradeoff if prefunding must happen before the x402HttpxClient is instantiated.
Safe-based agents hold funds in Safe smart wallets, but x402 payments require EIP-3009 signing by the privy EOA. The payment flow now explicitly funds the privy EOA from the Safe before signing, keeping spending limits enforced by the Safe.
x402 Safe funding flow
x402 skill integration
Tests
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.