cartesi / rollups-contracts

Smart Contracts for Cartesi Rollups
https://cartesi.github.io/rollups-contracts/
Apache License 2.0
17 stars 37 forks source link

ENS libs for delegatecall vouchers #273

Closed ZzzzHui closed 2 months ago

ZzzzHui commented 2 months ago

If we decide to take the "all-in-one" approach, I think we could think of a better name for the contract, since we're not really calling addresses by their ENS names in all the cases.

How about CallWithENS?

guidanoli commented 2 months ago

How about CallWithENS?

This name seems more general than what the contract is actually doing. It currently does:

So, we are mostly using this contract for asset transfers. Maybe AssetTransferToENS? This wouldn't be 100% accurate for the case of arbitrary message calls, but we could add a note there saying that this entry point can actually be used for more general use cases other than Ether withdrawals, and we're left with a more understandable name for 99% of use cases.

guidanoli commented 2 months ago

This needs a rebase.

ZzzzHui commented 2 months ago

rebased. Also moved internal functions to the end.

ZzzzHui commented 2 months ago

Since we're allowing sendEtherToENS to receive a payload, I think it would be good to have a test where the ENS node actually resolves to the address of a contract, and the payload encodes a function call. We could even recycle part of the "Ether mint" test case for that purpose.

agree

ZzzzHui commented 2 months ago

I've just noticed we're missing the deployment script of SafeERC20Transfer and AssetTransferToENS. They could be deployed by a deploy/03_delegatecall.ts script.

Yeah. But since SafeERC20Transfer is not relevant with this PR, the deploy script will be done in another PR. Ok? :)

ZzzzHui commented 2 months ago

Thank you very much Gui !!!

ZzzzHui commented 2 months ago

Needs @pedroargento 's approval to merge :)

ZzzzHui commented 2 months ago

Thanks very much Pedro :)