Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

soroban-cli: refers to 'stellar asset contract' as 'token' #934

Closed
leighmcculloch opened this issue Sep 6, 2023 · 6 comments · Fixed by #1086
Closed

soroban-cli: refers to 'stellar asset contract' as 'token' #934

leighmcculloch opened this issue Sep 6, 2023 · 6 comments · Fixed by #1086
Labels
bug Something isn't working cli Related to Soroban CLI

Comments

@leighmcculloch
Copy link
Member

What version are you using?

9615897

soroban version
soroban 0.9.4 (v0.9.5-70-g9615897821492671f951169f8f442d69cafb3d81)
soroban-env 0.0.17 (eb5a9ba053a7b64a8eff605db625525378f7bea0)
soroban-env interface version 85899345977
stellar-xdr 0.0.17 (39904e09941046dab61e6e35fc89e31bf2dea1cd)
xdr next (65afa63b7f52c898143ebbe9541ef91fcf290ade)

What did you do?

soroban lab -h
Experiment with early features and expert tools

Usage: soroban lab <COMMAND>

Commands:
  token  Wrap, create, and manage token contracts
  xdr    Decode xdrsoroban lab token -h
Wrap, create, and manage token contracts

Usage: soroban lab token <COMMAND>

Commands:
  wrap  Deploy a token contract to wrap an existing Stellar classic asset for smart contract usage
  id    Compute the expected contract id for the given asset

What did you expect to see?

Instructions about deploying and interacting with a stellar asset contract for a stellar asset.

What did you see instead?

  token  Wrap, create, and manage token contracts

Discussion

Quite a few releases ago we stopped referring to the stellar asset contract as the token contract.

There are two distinct but related concepts in Soroban:

  1. token interface;
  2. and, the stellar asset contract.

The stellar asset contract implements the token interface for all stellar assets.

Contract developers can create their own contracts that implement the token interface.

The CLI provides a confusing interface because it mixes this terminology, and uses terms that don't exactly parallel the XDR or SDK. We should make these things more consistent.

A few issues:

  • The CLI refers to wrapping and creating as if there are two distinction actions. However there is only one action.
  • The CLI refers to the stellar asset contract at a high level as 'token'.
  • The CLI uses the term 'wrap' to describe an action where no new token is being created, a contract for an existing token (the stellar asset) is being deployed.
@leighmcculloch leighmcculloch added the bug Something isn't working label Sep 6, 2023
@stellarsaur
Copy link
Contributor

There could definitely be some confusion with the terminology surrounding token and wrap. Perhaps we could rename the token command: soroban lab sac ... or soroban lab contract .... Also could consider renaming wrap to something clearer, like create: soroban lab sac create ... or soroban lab contract create ....

Of course, the help text would need to be updated to be clearer as well. Open to discussion on this.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Nov 22, 2023

I'd suggest modifying these commands to be:

❯ soroban asset -h
Work with Stellar Assets.

Usage: soroban asset <COMMAND>

Commands:
  deploy Deploy the asset contract for an asset.
  address Compute the expected contract address for the given asset.
❯ soroban xdr -h
Work with XDR.

Usage: soroban xdr <COMMAND>

Commands:
...

cc @willemneal for input.

Note that the following issue will result in the contents of this XDR command being replaced, so we don't need to spend much effort on that:

@willemneal
Copy link
Member

@leighmcculloch I'm all for this change. Just wondering if it's best under the contract subcommand since it is a contract.

@leighmcculloch
Copy link
Member Author

I think you're right. In fact we don't even need unique commands for these. In the future there will be other built in contracts, so we should present this in a way that the sac is just another contract type, one of many, that can be deployed.

Maybe:

soroban contract deploy wasm --file ...
soroban contract deploy asset --asset ABC:G...
soroban contract deploy asset --asset native

The id command for that gives the user a contract is for a given asset would be useful for any contract too. So maybe that could be separate command:

soroban contract id asset ...

@stellarsaur stellarsaur added the cli Related to Soroban CLI label Dec 4, 2023
@willemneal willemneal linked a pull request Dec 4, 2023 that will close this issue
5 tasks
@mollykarcher mollykarcher moved this from Backlog to In Progress in Platform Scrum Dec 14, 2023
@mollykarcher mollykarcher moved this from In Progress to Blocked in Platform Scrum Dec 14, 2023
@janewang janewang moved this from Blocked to Backlog in Platform Scrum Dec 21, 2023
@janewang
Copy link
Contributor

Remove lab, rename token to asset. Please comment on this for your thoughts.

@leighmcculloch
Copy link
Member Author

At minimum, and rename wrap to deploy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Related to Soroban CLI
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants