Skip to content

Add additional USDT approve transaction to account for must-reset-to-0 behavior#413

Merged
peachbits merged 3 commits intomasterfrom
matthew/common-approvals
Nov 4, 2025
Merged

Add additional USDT approve transaction to account for must-reset-to-0 behavior#413
peachbits merged 3 commits intomasterfrom
matthew/common-approvals

Conversation

@peachbits
Copy link
Contributor

@peachbits peachbits commented Oct 28, 2025

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Description

none

Note

Adds automatic zero-then-amount ERC20 approvals (for tokens like USDT) and migrates swap flows to support multiple pre-transactions across DEX plugins.

  • Core/Utils:
    • Introduces createEvmApprovalEdgeTransactions to build one or two ERC20 approval pre-txs, including a zero-allowance reset for non-standard tokens (e.g., ethereum USDT).
    • Refactors getEvmApprovalData to encode approve calldata locally.
  • Swap Flow:
    • Replaces single preTx with preTxs throughout and pipes them via pendingTxs in makeSpend/makeTx.
    • Updates fee aggregation, approval signing/broadcast order, and max-amount calculation to account for multiple pre-txs.
  • Plugins Updated:
    • lifi, rango, thorchain/swapkit, thorchain/thorchainCommon, unizen: use createEvmApprovalEdgeTransactions and return preTxs.
    • spookySwap, tombSwap, velodrome: migrate to preTxs handling.
  • CHANGELOG:
    • Notes added for zero-reset approvals and multi pre-transaction support.

Written by Cursor Bugbot for commit ea8d818. This will update automatically on new commits. Configure here.


USDT requires an allowance to be zero before it can be nonzero. Most, like 99% of tokens, don't require this behavior.

We would ideally check the allowance first before adding any approval transactions to the swap to avoid unnecessary transactions. Instead of adding a bunch of node lists and queries here maybe we could plumb something through the engine or tools to access those node lists.
@peachbits peachbits force-pushed the matthew/common-approvals branch from 76d7c22 to ea8d818 Compare November 4, 2025 06:16
@peachbits peachbits enabled auto-merge November 4, 2025 06:16
@peachbits peachbits merged commit 61c1507 into master Nov 4, 2025
2 checks passed
const approvalData = getEvmApprovalData({
contractAddress: tokenContractAddress,
nativeAmount: amount
})
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect ERC-20 approve data leading to failures

The createApprovalTx function passes tokenContractAddress to getEvmApprovalData for the contractAddress parameter. The ERC-20 approve function expects the spender address (the recipientAddress) as its first argument. This generates incorrect approval transaction data, leading to failed approval transactions.

Fix in Cursor Fix in Web

NON_STANDARD_APPROVAL_TOKENS[
request.fromWallet.currencyInfo.pluginId
].includes(request.fromTokenId)
) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Undefined lookup leads to TypeError on non-ethereum tokens

Accessing NON_STANDARD_APPROVAL_TOKENS for a pluginId other than 'ethereum' results in a TypeError. The lookup returns undefined, and then .includes() is called on that undefined value.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants