feat(cast): ask for confirmation on erc20-token transfer

[grandizzy]

Component

Cast

Describe the feature you would like

Would be nice to have a safe check when we retrieve decimals and token name from erc20 contract and promp something like:

Confirm transfer of 10 TOKEN to address 0x...[y/n]

With prompting original amount / address if we cannot retrieve decimals or token name from contract.

Additional context

No response

[Syzygy106]

@grandizzy, sounds interesting. Could I take this?

Also, could you provide a bit more context? (Maybe you already have some thoughts.)

Otherwise, I can try to imagine how to implement it.

[grandizzy]

@Syzygy106 sure, thank you! You could use the decimals to apply to amount and show how many tokens will be transferred

https://github.com/foundry-rs/foundry/blob/51f168f73282a007c36468b90958f840891597e2/crates/cast/src/cmd/erc20.rs#L290

and the symbol to show which erc20 token will be transferred

https://github.com/foundry-rs/foundry/blob/51f168f73282a007c36468b90958f840891597e2/crates/cast/src/cmd/erc20.rs#L280

[Syzygy106]

@Syzygy106 sure, thank you! You could use the decimals to apply to amount and show how many tokens will be transferred

foundry/crates/cast/src/cmd/erc20.rs

Line 290 in 51f168f

let decimals = IERC20::new(token, &provider) and the symbol to show which erc20 token will be transferred

foundry/crates/cast/src/cmd/erc20.rs

Line 280 in 51f168f

let symbol = IERC20::new(token, &provider)

Got it. Set assignee to me.

[Syzygy106]

@grandizzy, I took a closer look at how cast is designed and it led me to some thoughts.

From what I can see:

1. Cast is primarily a tool for developers, not end users
2. Cast maintains atomicity of operations with explicit behavior (you call one thing - exactly that thing executes)

The problem is that the most straightforward stateless solution would violate this design. When a developer wants to do Transfer/Approve, we'd make 2 extra read calls behind their back that they might not have intended.

If we consider a non-stateless approach with caching, we'd expect the user to manually call decimals() and symbol()/name() first to populate the cache. But that's implicit behavior and unlikely to catch on. An alternative could be introducing a separate cast command for parsing all ERC20 metadata upfront, which would then enable an adapted UX - now that seems like an interesting idea worth exploring.

If we stick with stateless but preserve operation atomicity by default, we could add the nice UX check behind a flag (like --confirm). Unfortunately, that defeats the purpose. Confirmations are meant to protect users who are rushing - and those users definitely won't add an extra verification flag. So we end up with functionality that careful users don't need and careless users won't use.

What do you think? It seems to me this issue should be closed, but having dedicated functionality for parsing standard contract types (ERC20/721/1155, etc.) for better UX - that actually sounds like a solid idea.

[grandizzy]

I still think this would be valuable(you can yes | to confirm) @onbjerg @zerosnacks @0xrusowsky pls chime in if we should support such. Thanks!

[Syzygy106]

I still think this would be valuable(you can yes | to confirm) @onbjerg @zerosnacks @0xrusowsky pls chime in if we should support such. Thanks!

We could add a whole "Bettet UX" mode which will parse the needed cache in a single step. I feel that more convenient.

UPD: Or maybe I misunderstood you, and we’re actually talking about the same thing?

[Syzygy106]

...An alternative could be introducing a separate cast command for parsing all ERC20 metadata upfront, which would then enable an adapted UX - now that seems like an interesting idea worth exploring.

[grandizzy]

@Syzygy106 imo should just do something simple that warns / asks for confirmation upon transfer by leveraging token info as decimals and token name / ticker

[Syzygy106]

Or

@Syzygy106 imo should just do something simple that warns / asks for confirmation upon transfer by leveraging token info as decimals and token name / ticker

Via a separate flag like "--confirm"? Or as a default behavior?

[grandizzy]

Imo default is fine, can be circumvented with yes | if needed

[Syzygy106]

Imo default is fine, can be circumvented with yes | if needed

Got it. If other people feel the same, I'll implement that.

[zerosnacks]

Sounds good

[Syzygy106]

In progress

[Syzygy106]

@grandizzy , done. Check it, I'm interested in your point of view. Maybe flag naming, etc.