-
Notifications
You must be signed in to change notification settings - Fork 175
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
Introduced by-name deployment and declare-deploy
command
#2331
base: master
Are you sure you want to change the base?
Conversation
declare-deploy
command
crates/sncast/src/main.rs
Outdated
let deploy: DeployResolved = if deploy.class_hash.is_some() { | ||
deploy.try_into().unwrap() | ||
} else { | ||
let contract = | ||
deploy.build_artifacts_and_get_compiled_contract(cli.json, &cli.profile)?; | ||
let class_hash = contract.sierra_class_hash; | ||
|
||
if !contract.is_declared(&provider).await? { | ||
bail!("Contract with class hash {:x} is not declared", class_hash); | ||
} | ||
|
||
deploy.resolved_with_class_hash(class_hash) | ||
}; | ||
|
||
deploy.validate()?; | ||
|
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.
So now we will explicitly fail if user tries to deploy contract that wasn't declared? What was the previous behavior. This might be worth including in a changelog if this behavior changed.
@@ -233,6 +247,76 @@ async fn run_async_command( | |||
Ok(()) | |||
} | |||
|
|||
Commands::DeclareDeploy(declare_deploy) => { |
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.
Should we explicitly document that declare-deploy
behave exactly as deploy if the contract has been already declared?
use sncast::helpers::{deploy::DeployArgs, fee::FeeToken, rpc::RpcArgs}; | ||
|
||
#[derive(Args)] | ||
#[command(about = "Deploy a contract on Starknet")] |
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.
Shouldn't it be Declare and deploy a contract on Starknet
?
/// Calldata for the contract constructor | ||
#[clap(short, long, value_delimiter = ' ', num_args = 1..)] | ||
pub constructor_calldata: Vec<FieldElement>, |
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.
What happened to this argument and these below
@@ -0,0 +1,32 @@ | |||
# Joining the Declaration and Deployment Together |
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.
Maybe this would be clearer to the users? Or something similar
# Joining the Declaration and Deployment Together | |
# Deploying an Undeclared Contract |
# Joining the Declaration and Deployment Together | ||
|
||
`sncast` allows declaring and deploying contracts through dedicated commands. | ||
The `declare-deploy` command simplifies this pipeline by performing these two actions one after the other. It is used to declare a contract and deploy it immediately. |
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.
The `declare-deploy` command simplifies this pipeline by performing these two actions one after the other. It is used to declare a contract and deploy it immediately. | |
The `declare-deploy` command simplifies this pipeline by performing these two actions at the same time. It is used to declare a contract and deploy it immediately. |
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 know what you wrote is technically correct, but I think my suggestion is a better description for the user
|
||
### General Example | ||
|
||
Make sure you have a `Scarb.toml` in your project directory. Suppose we would like to declare and instantly deploy an example contract named `SimpleBalance`, defined in the default project. |
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 don't think we have a contract like that in template. Maybe let's use the name of the contract that's defined there?
… into integraledelebesgue/feature-2279
… into integraledelebesgue/feature-2279
Closes #2279
Introduced changes
sncast deploy
now allows to deploy a contract by its name instead of class hash;--contract-name
and--class-hash
arguments are mutually exclusivedeclare-deploy
, used to quickly combine declaration with deploymentdeclare
,declare-deploy
- alwaysdeploy
- when depolying by nameChecklist
CHANGELOG.md