aptos-core icon indicating copy to clipboard operation
aptos-core copied to clipboard

[Token] migrate to new token standard

Open areshand opened this issue 2 years ago • 5 comments

Description

  1. switch the token modules
  2. update the cached framework to load move modules from multiple packages for testing
  3. update the python and ts example
  4. update the ts sdk
  5. indexer doesn't need to be updated now since creation event is emitted and other events are the same

Test Plan

test python and sdk locally


This change is Reviewable

areshand avatar Jul 22 '22 17:07 areshand

Amazing PR. Given that this kills of "TokenV0" and introduces a completely new standard that we probably don't want to pivot from. I'd love for us to first audit "TokenV1" to make sure its features are sane. Specifically that both happy and negative paths are handled within reason.

davidiw avatar Jul 26 '22 04:07 davidiw

Completed audit:

  • why does a TokenData need to know its own TokenDataId as well as its name
  • can we remove token_auths for mvp
  • can we remove mint_capabilities
  • merge apis now that we can pass string::utf8
  • it isn't clear why we'd want to give the token owner the ability to mutate a token
  • direct_deposit_without_event is dangerous because it makes it easier to lose track of tokens
  • mutator should be creator and not owner as the common case

Can we add this as additional commits to this PR? We can definitely explore adding more of these after we have more time to review the nuances and maybe even leaving the appropriate data structures but without the functions before we code freeze.

davidiw avatar Jul 27 '22 22:07 davidiw

Completed audit:

  • why does a TokenData need to know its own TokenDataId as well as its name
  • can we remove token_auths for mvp
  • can we remove mint_capabilities
  • merge apis now that we can pass string::utf8
  • it isn't clear why we'd want to give the token owner the ability to mutate a token
  • direct_deposit_without_event is dangerous because it makes it easier to lose track of tokens
  • mutator should be creator and not owner as the common case

Can we add this as additional commits to this PR? We can definitely explore adding more of these after we have more time to review the nuances and maybe even leaving the appropriate data structures but without the functions before we code freeze.

Add the changes above. The changes are in the 2nd last commit https://github.com/aptos-labs/aptos-core/pull/2164#issuecomment-1197431202.

areshand avatar Jul 28 '22 02:07 areshand

Note... this is a breaking change... we may want to land this and the documentation change around Tuesday / Wednesday to avoid any issues on Devnet.

davidiw avatar Jul 29 '22 03:07 davidiw

:x: Forge test failure on 4dc6880d485bd31026bf2de321b2eafa7cd83ef1

Forge test runner terminated

github-actions[bot] avatar Jul 29 '22 22:07 github-actions[bot]

:white_check_mark: Forge test success on 9f889557ed2fef935f52ccc8dd7562780195c3ae

all up : 5227 TPS, 2084 ms latency, 2850 ms p99 latency,no expired txns

github-actions[bot] avatar Jul 31 '22 18:07 github-actions[bot]

How come we deleted the submits multiagent transaction test from the TS SDK?

banool avatar Aug 01 '22 15:08 banool

How come we deleted the submits multiagent transaction test from the TS SDK?

lol, good catch. I plan to delete the multi-agent transfer for now as I don't see many usage for that now.

areshand avatar Aug 01 '22 20:08 areshand

ah, the purpose is to demonstrate how people can do multiagent transactions. Can we bring it back on the SDK side? We just happened to use the Token transfer as an example. :). We can do it ourselves unless @areshand has concerns about using token transfer as an example.

jjleng avatar Aug 02 '22 00:08 jjleng