Entropy-Foundation / aptos-core

Aptos is a layer 1 blockchain built to support the widespread use of blockchain through better technology and user experience.
https://aptoslabs.com
Other
0 stars 2 forks source link

Ensure that `Coin`s cannot be converted to `FungibleAsset`s and vice-versa #91

Closed isaacdoidge closed 1 month ago

isaacdoidge commented 1 month ago

We inherited a near-complete roll-out of the Fungible Asset Standard when we upgraded to aptos-core v1.16. However, we are not ready to support this standard yet. For example, the process that we recommend to use for tracking account balance changes will need to be reworked as the events emitted for fungible assets do not have access to the same type information as the events for coins. This represents a risk for clients that need to track balance changes (e.g. token deposits to an exchange).

To mitigate this risk we need to deactivate the functionality that enables conversion between Coins and FungibleAssets. Simply deactivating the corresponding feature flag, COIN_TO_FUNGIBLE_ASSET_MIGRATION, causes most of the unit tests to fail and it is not clear that this is safe to do now that it is a part of the default feature set. Consequently, the safest option appears to reduce the visibility of migrate_to_fungible_store and coin_to_fungible_asset functions in coin.move to public(friend). Since the signatures of private functions (which includes public(friend) functions) may change arbitrarily, we can restore these functions to their previous levels when we are confident that we fully support FAs.

Note that this solution will only work for mainnet, as the testnet has already been deployed with these functions being publicly accessible. We might consider implementing an alternative solution for testnet, such as returning an error from these functions, but this will require more changes to the unit tests. Since testnet tokens do not hold real value we should be okay to leave these functions active there for now. We can apply a patch if clients report issues related to their use.

isaacdoidge commented 1 month ago

I came up with a way to achieve the stated goal whilst maintaining backwards compatibility. We should be able to deploy this change to testnet via a framework upgrade.