Discussion: FHERC20 standard

[architect-dev]

Discussion of the FHERC20 standard. It is currently using the draft PermitNFTs (#74) and has some temporary code in it that requires the PermitV2 contract address in the constructor, which will be removed in the future.

Here are the implementation differences compared to tokens/FHERC20.sol:

• standardization:
  • Encrypted functions renamed to enc{ERC20FunctionName}.
  • Encrypted variables renamed to e{Name}.
  • Encrypted input variables renamed to ie{Name}.
• wrap / unwrap renamed to encrypt / decrypt.
• Encrypted public variables split into enc___ (for other FHE enabled contracts) and sealed___ (for frontend display).
• Events emitted from transfer / encrypt / decrypt
• Added _encBeforeTokenTransfer and _encAfterTokenTransfer hooks to match the OpenZeppelin ERC20 standard.

Open Questions:

• How can we create a permission "plugin" system that can allow each FHE environment to plug in their own permission system while maintaining the underlying standard.
• What kind of wallet integration can we expect? Metamask snap may allow users to view their encrypted balances, but is there a way to indicate within default wallets that the user has an encrypted balance?

[eshelB]

I am for the naming changes, also nice addition the split to seal__ and enc__. Hooks - cool as well.

What is the use case of the two new Events that don't contain an amount? Maybe we don't want to raise them as, even though the event of an encrypted transfer can be known by an observer, maybe we don't want to explicitly let everybody know that?

I didn't think too much about it but I think it is possible to do the plugin system you're talking about. Or maybe as extensions like those in erc721.

[architect-dev]

Unfortunately the split sealed and enc functions runs into the problem that the new hardhat task tries to catch. The enc functions expose the user's encrypted values publicly which opens the door for a malicious contract to decrypt them.

I'm not sure about the events, without the amount I don't think they end up doing much like you said other than exposing data.

[eshelB]

Ah! So your plugin to the hardhat plugin is indeed working and catching good stuff!

Yeah, it seems that the plain enc_ functions are indeed problematic

Maybe what we could is:
a. Cause the hardhat plugin to agree to these functions if and only if they contain a modifier (such as NotFromContract or AllowPermissioned(X)), this way it is more probable that the developer is aware of the problem
b. Change the standard implementation either not to expose these variables at all, or to expose them only to a standard role of "OWNER" or something along those lines