-
Notifications
You must be signed in to change notification settings - Fork 76
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
Add support for contract id alias name when deploying and invoking contracts. #1356
Conversation
Overall a great first big rust PR. As mentioned above I do think that these functions would work best in the config so that we can separate them from the commands. And I mentioned yesterday that I think that aliases should be tied to the network where the contract was deployed to. Perhaps for now we can use |
@willemneal can you please do a second review pass? Will extract everything to config::Args in a separate pr, because I noticed the change surface it's way too big. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just had the thought, that install and deploy are separate, and maybe there's also value in aliases for contracts installed, and not just aliases for deployments of installed contracts. I don't think it changes anything significant about what we're doing here, but just keeping that in mind that we may also end up with aliases pointing to different things. In this case they could still be separate ID spaces so I think what we're doing here, making aliases specifically for contract IDs makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a bug, that doesn't actually result in negative outcome for the user as it is invisible really, but the behaviour doesn't seem correct. See below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I would make the changed noted below if you're up for it. It is a more common idiom in Rust than using empty strings to signal an absent value. I hadn't mentioned it before, but mentioning it now because I think that was the root cause of the bug that was fixed.
The RPC tests look like they're failing for reasons that aren't obvious. They aren't failing on other PRs that are open, so it might be something to do with this change. |
@leighmcculloch @willemneal I've changed the alias file to be scoped by network passphrases. This means we'll have only one file for everything. $ soroban contract deploy --wasm ./target/wasm32-unknown-unknown/release/hello_world.wasm --network standalone --source alice --alias alice --global
CCOZHG2BHABPGVIYBJP675PHDFCT5LPVVY5GAX5CGOWI274DAN6PPUZ5
$ cat ~/.config/soroban/contract-ids/alice.json | jq
{
"ids": {
"Test SDF Network ; September 2015": "CDMXLF7V4XQ4QZ62D6MT2J2SGY4YLBHPOQTT3WPXRHTGQ4CUFUE3JW5E",
"Standalone Network ; February 2017": "CCOZHG2BHABPGVIYBJP675PHDFCT5LPVVY5GAX5CGOWI274DAN6PPUZ5"
}
}
$ soroban contract invoke --network testnet --source alice --id alice --global -- hello --to fnando
["Hello","fnando"]
$ soroban contract invoke --network standalone --source alice --id alice --global -- hello --to fnando
["Hello","fnando"] Can you please review the latest changes and suggest only extremely required changes? I'd like to merge this in so I can fix the contract init's generated file before the next CLI release (today-tomorrow). We can iterate and apply other changes in further pull requests. :) |
What
soroban contract deploy
This pr introduces two different switches:
--alias ALIAS
: the alias name, effectively the file name that will be created under.soroban/contract-ids/:network_name/:alias.txt
. If--global
is provided, then the alias will be created under~/config/.soroban/contract-ids/:network_name/:alias.txt
.soroban contract invoke
When invoking a contract, if the user provides a valid alias to
--id
, then the respective contract id will be loaded. If the alias doesn't exist, then an error message likeerror: cannot parse contract ID foo: the strkey is invalid
will be returned.Why
When deploying wasms, it's common to redirect the contract id to a file so it can be reused later on. To simplify the whole process and make this a non-*nix feature, let's introduce the concept of contract id aliasing.
Known limitations
Tickets