feat: add safe ERC-20 approve command#58
Conversation
Add an approve command that makes ERC-20 allowance updates explicit and safer for developers, with current-allowance readback, unlimited confirmation, and overwrite protection for tokens that require zeroing first. Made-with: Cursor
| import { withSpinner, printKeyValueBox, green, dim } from "../lib/ui.js"; | ||
| import { parseAmount, fetchTokenDecimals } from "./send/shared.js"; | ||
|
|
||
| const NATIVE_TOKEN_ADDRESS = "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE" as Address; |
There was a problem hiding this comment.
is this just a placeholder for now?
There was a problem hiding this comment.
good question - we will keep this for the native token check below. This is the standard address used across defi to indicate the native token on a chain
There was a problem hiding this comment.
Btw - what is "NATIVE" here? Is it ETH? Should we have a "native" token per network mapping? I think there were some discussion in another thread regarding this
There was a problem hiding this comment.
Oops - left this comment after you left yours - is that the case for solana as well?
There was a problem hiding this comment.
@lohnim I think the approve command will be EVM only as this is specific to the erc20 allowance/approval flow (which isn't a solana concept).
I'm not as familiar with solana but I think we will probably want a different command for token permissions in that ecosystem
src/commands/approve.ts
Outdated
| function formatTokenAmount(rawAmount: bigint, decimals: number): string { | ||
| const str = rawAmount.toString().padStart(decimals + 1, "0"); |
There was a problem hiding this comment.
nit: Are we using this sort of formatting elsewhere? Better to add to some sort of formatting file?
There was a problem hiding this comment.
Yeah that's a good call - I can move this to a shared file
src/commands/approve.ts
Outdated
| program | ||
| .command("approve") | ||
| .description("Approve an ERC-20 token allowance for a spender") | ||
| .argument("<token_address>", "ERC-20 token contract address") |
There was a problem hiding this comment.
What's the framework behind keeping this as argument vs other as options? From what the description reads - it feels more like we're approving the spender to an amount. So I wonder if it should be something like
alchemy approve {spender} --token_address {} --amount
Might be missing something here though! But feel like it should read like english like I approve this person, I want to swap this token, etc.
There was a problem hiding this comment.
Yeah, I think that’s a fair question. I think the current framework will feel pretty natural to people/agents familiar with the ERC-20 approval flow.
The token contract is the primary resource the command operates on, so I used that as the positional argument, with the spender as a required named parameter. this maps directly to the erc-20 approve(spender, amount) call on the token contract. which I think maps pretty well with this alchemy approve 0xUSDC --spender 0xRouter --amount 100
| import { withSpinner, printKeyValueBox, green, dim } from "../lib/ui.js"; | ||
| import { parseAmount, fetchTokenDecimals } from "./send/shared.js"; | ||
|
|
||
| const NATIVE_TOKEN_ADDRESS = "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE" as Address; |
There was a problem hiding this comment.
Btw - what is "NATIVE" here? Is it ETH? Should we have a "native" token per network mapping? I think there were some discussion in another thread regarding this
Make the approve CLI read more naturally by taking the spender as the positional argument, and extract shared token amount formatting so approve and swap use the same display logic. Made-with: Cursor
3 participants
Summary
alchemy approvefor ERC-20 allowance updates with explicit--amount,--unlimited, and--revokemodes--reset-firstfor tokens that require zeroing an existing allowance before updating itTest plan
pnpm lintpnpm vitest run tests/commands/approve.test.tspnpm test:live -- tests/live/approve.live.test.tsMade with Cursor