[cli] transfer:erc20 and balance commands #7753
Conversation
gastonponti
requested review from
barbaraliau,
critesjosh,
mcortesi,
medhakothari and
timmoreton
as code owners
|
|
||
| static flags = { | ||
| ...BaseCommand.flags, | ||
| erc20Address: Flags.address({ |
There was a problem hiding this comment.
we can define this flag once and reuse it across the commands
There was a problem hiding this comment.
Don't know if we require this in more that these two commands. We could leave it this way and if we see another pattern, we could extract it in the future
| @@ -6,6 +6,7 @@ export enum CeloContract { | |||
| DowntimeSlasher = 'DowntimeSlasher', | |||
| Election = 'Election', | |||
| EpochRewards = 'EpochRewards', | |||
| ERC20 = 'ERC20', | |||
There was a problem hiding this comment.
this seems like the wrong place for this to live
There was a problem hiding this comment.
is "ERC20" a CeloContract?
There was a problem hiding this comment.
We should define what we think it is a CeloContract. In a way, we think of those as the contracts supported/maintained by celo. The ERC20 is a standard accepted by everyone.
Let's say that the contract does not exist in the ethereum network, we would probably want to handle and support that contract.
So, in a way, it is something standard for celo too, we are just taking the advantage of reuse it from other network.
If we think of the CeloContracts as only the ones that live on the registry, we should also remove the multisig from that list, for example.
If we think of the CeloContracts, as the one accepted by the community, all the ercXXX accepted, should be supported too
Co-authored-by: Yorke Rhodes <yorhodes@cLabs.co>
### Description `RegisteredContracts` list used in ContractKit for comprehensive registry lookup and governance proposal decoding (among others) errantly includes `CeloContract.ERC20` introduced in #7753 despite this not being a registered contract. This PR adds `CeloContract.ERC20` to `AuxiliaryContracts` to be filtered out. ### Tested Before change, `Error: ERC20 not yet deployed for this chain` is emitted on many CLI changes. After change, error is no longer observed.
Description
Command to transfer and check balance of erc20 contracts
Tested
Manually in alfajores
Backwards compatibility
Yes