fix(USDT-like tokens): implement handling for requiring allowance reset before approval #499
Conversation
…e reset before approval
| const txRequirements: TransactionRequirement[] = []; | ||
|
|
||
| // Handle USDT-like tokens that require reset to 0 before changing allowance. | ||
| if (APPROVE_ONLY_ONCE_TOKENS[chainId]?.includes(token) && amount > 0n) { |
There was a problem hiding this comment.
- Do we have a test suite ?
- Should we add tests for such critical element update?
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c493571596
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else if (useSimpleTransfer) { | ||
| if ( | ||
| APPROVE_ONLY_ONCE_TOKENS[data.chainId]?.includes(address) && | ||
| erc20Allowances["bundler3.generalAdapter1"] > 0n | ||
| ) | ||
| operations.push({ | ||
| type: "Erc20_Approve", | ||
| sender: from, | ||
| address, | ||
| args: { | ||
| amount: 0n, | ||
| spender: generalAdapter1, | ||
| }, | ||
| }); | ||
| operations.push({ | ||
| type: "Erc20_Approve", | ||
| sender: from, |
There was a problem hiding this comment.
For approve-only-once tokens (e.g., USDT), simulateOperation still uses handleErc20ApproveOperation to throw NonZeroAllowanceError when the current allowance is non‑zero and a new non‑zero approve is simulated (packages/simulation-sdk/src/handlers/erc20/approve.ts). By removing the explicit 0-approve operation here, the bundle simulation will now fail whenever a user already has a non‑zero allowance to generalAdapter1 (or permit2 in the other branch) and needs to increase it, even though encodeErc20Approval later adds a reset only at encoding time. That means bundling can abort before encoding in the common “non‑zero existing allowance” case for USDT-like tokens.
Useful? React with 👍 / 👎.
Rubilmax
left a comment
Rubilmax
left a comment
There was a problem hiding this comment.
See codex comment and let's chat if blocked
This PR moves the reset-approval logic for USDT-like tokens to ensure that each approval properly accounts for the reset when required.
The error came from fallback logic when supportSignature was set to false, which didn’t account for the approval reset.