-
Notifications
You must be signed in to change notification settings - Fork 120
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
Specify prod chain URL with names and check for the verifiable build upon upload #1290
Changes from 10 commits
84833a4
700849c
d7b7952
51b7513
1282148
93d51c6
cf50490
c031d2f
347480e
c9d6a2b
0044852
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ use contract_extrinsics::{ | |
url_to_string, | ||
ContractInfo, | ||
ErrorVariant, | ||
ProductionChain, | ||
TrieId, | ||
}; | ||
use ink_env::Environment; | ||
|
@@ -74,6 +75,9 @@ pub struct InfoCommand { | |
default_value = "ws://localhost:9944" | ||
)] | ||
url: url::Url, | ||
/// A name of a production chain to upload or instantiate the contract on. | ||
#[clap(name = "chain", long, conflicts_with = "url")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apart from the URL, the chain could also retain the chain configuration. So, when you specify 'alephzero', 'moonbeam', or whatever, it would also set the correct In this case chain would conflict with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be a follow up PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok |
||
chain: Option<ProductionChain>, | ||
/// Export the instantiate output in JSON format. | ||
#[clap(name = "output-json", long)] | ||
output_json: bool, | ||
|
@@ -102,7 +106,11 @@ impl InfoCommand { | |
<<C as Config>::AccountId as FromStr>::Err: | ||
Into<Box<(dyn std::error::Error)>> + Display, | ||
{ | ||
let rpc_cli = RpcClient::from_url(url_to_string(&self.url)).await?; | ||
let rpc_cli = if let Some(chain) = &self.chain { | ||
RpcClient::from_url(chain.end_point()).await? | ||
} else { | ||
RpcClient::from_url(url_to_string(&self.url)).await? | ||
}; | ||
let client = OnlineClient::<C>::from_rpc_client(rpc_cli.clone()).await?; | ||
let rpc = LegacyRpcMethods::<C>::new(rpc_cli.clone()); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,6 +72,7 @@ pub(crate) use contract_extrinsics::ErrorVariant; | |
use contract_extrinsics::{ | ||
pallet_contracts_primitives::ContractResult, | ||
BalanceVariant, | ||
ProductionChain, | ||
TokenMetadata, | ||
}; | ||
|
||
|
@@ -127,6 +128,9 @@ pub struct CLIExtrinsicOpts { | |
/// Before submitting a transaction, do not ask the user for confirmation. | ||
#[clap(short('y'), long)] | ||
skip_confirm: bool, | ||
/// A name of a production chain to upload or instantiate the contract on. | ||
#[clap(name = "chain", long, conflicts_with = "url")] | ||
chain: Option<ProductionChain>, | ||
} | ||
|
||
impl CLIExtrinsicOpts { | ||
|
@@ -310,6 +314,36 @@ where | |
Ok(arr.into()) | ||
} | ||
|
||
/// Prompt the user to confirm the upload of unverifiable code to the production chain. | ||
pub fn prompt_confirm_unverifiable_upload(chain: &str) -> Result<()> { | ||
println!( | ||
"{} (skip with --skip-validate)", | ||
"Confirm upload:".bright_white().bold() | ||
); | ||
let warning = format!( | ||
"You are trying to upload unverifiable code to {} mainnet", | ||
chain | ||
) | ||
.bold() | ||
.yellow(); | ||
print!("{}", warning); | ||
println!( | ||
"{} ({}): ", | ||
"\nContinue?".bright_white().bold(), | ||
"Y/n".bright_white().bold() | ||
); | ||
|
||
let mut buf = String::new(); | ||
io::stdout().flush()?; | ||
io::stdin().read_line(&mut buf)?; | ||
match buf.trim().to_lowercase().as_str() { | ||
// default is 'y' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if the default should be 'yes'; rather, we should recommend uploading only verifiable contracts to production chains. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do advise it, but we still want to give a user an option to upload an unverifiable contract (aka shoot their own leg) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just thinking to make |
||
"y" | "" => Ok(()), | ||
"n" => Err(anyhow!("Upload canceled!")), | ||
c => Err(anyhow!("Expected either 'y' or 'n', got '{}'", c)), | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use subxt::{ | ||
|
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.
Why do we need to pass chain to
extrinsics
crate? I thinkextrinisics
crate should just get url as it gets it now. We could keep all the prod chain logic incargo-contracts
crateI always thought about the
extrinsics
crate to be small and reusable between other tools, likee2e
tests orsmart-bench
.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.
We also want to retain info about what kind of chain it is:
Prod
orCustom
. So it is easier to encapsulate the logic this way.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.
Why do we need this information in
extrinsics
crate? The upload check is done incargo-contract
crate.In
ExtrinsicOpts
we have url and chain which is kind of data duplication.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 agree this is not belong in
extrinsics