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

Add flag --all to cargo contract info #1319

Merged
merged 11 commits into from
Sep 12, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Detect `INK_STATIC_BUFFER_SIZE` env var - [#1310](https://github.com/paritytech/cargo-contract/pull/1310)
- Add `verify` command - [#1306](https://github.com/paritytech/cargo-contract/pull/1306)
- Add `--binary` flag for `info` command - [#1311](https://github.com/paritytech/cargo-contract/pull/1311/)
- Add `--all` flag for `info` command - [#1319](https://github.com/paritytech/cargo-contract/pull/1319)

## [4.0.0-alpha]

Expand Down
141 changes: 77 additions & 64 deletions crates/cargo-contract/src/cmd/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@

use super::{
basic_display_format_contract_info,
display_all_contracts,
DefaultConfig,
};
use anyhow::{
anyhow,
Result,
};
use contract_extrinsics::{
fetch_all_contracts,
fetch_contract_info,
fetch_wasm_code,
ErrorVariant,
Expand All @@ -35,14 +37,18 @@ use subxt::{
Config,
OnlineClient,
};
use tokio::runtime::Runtime;

#[derive(Debug, clap::Args)]
#[clap(name = "info", about = "Get infos from a contract")]
pub struct InfoCommand {
/// The address of the contract to display info of.
#[clap(name = "contract", long, env = "CONTRACT")]
contract: <DefaultConfig as Config>::AccountId,
#[clap(
name = "contract",
long,
env = "CONTRACT",
required_unless_present = "all"
)]
contract: Option<<DefaultConfig as Config>::AccountId>,
/// Websockets url of a substrate node.
#[clap(
name = "url",
Expand All @@ -55,76 +61,83 @@ pub struct InfoCommand {
#[clap(name = "output-json", long)]
output_json: bool,
/// Display the contract's Wasm bytecode.
#[clap(name = "binary", long)]
#[clap(name = "binary", long, conflicts_with = "all")]
binary: bool,
/// Display all contracts addresses
#[clap(name = "all", long)]
all: bool,
}

impl InfoCommand {
pub fn run(&self) -> Result<(), ErrorVariant> {
tracing::debug!(
"Getting contract information for AccountId {:?}",
self.contract
);
pub async fn run(&self) -> Result<(), ErrorVariant> {
let url = self.url.clone();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let url = self.url.clone();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

let client = OnlineClient::<DefaultConfig>::from_url(url).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let client = OnlineClient::<DefaultConfig>::from_url(url).await?;
let client = OnlineClient::<DefaultConfig>::from_url(&self.url).await?;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


Runtime::new()?.block_on(async {
let url = self.url.clone();
let client = OnlineClient::<DefaultConfig>::from_url(url).await?;
// All flag applied
if self.all {
// 1000 is max allowed value
const COUNT: u32 = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
const COUNT: u32 = 1000;
const MAX_COUNT: u32 = 1000;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

let mut from = None;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut from = None;
let mut count_from = None;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

let mut contracts = Vec::new();
loop {
let len = contracts.len();
contracts.append(&mut fetch_all_contracts(&client, COUNT, from).await?);
if contracts.len() < len + COUNT as usize {
break
}
from = contracts.last();
}

let info_result = fetch_contract_info(&self.contract, &client).await?;
if self.output_json {
let contracts_json = serde_json::json!({
"contracts": contracts
});
println!("{}", serde_json::to_string_pretty(&contracts_json)?);
} else {
display_all_contracts(&contracts)
}
Ok(())
} else {
// Contract arg shall be always present in this case, it is enforced by
// clap configuration
let contract = self
.contract
.as_ref()
.expect("Contract argument was not provided");

match info_result {
Some(info_to_json) => {
match (self.output_json, self.binary) {
(true, false) => println!("{}", info_to_json.to_json()?),
(false, false) => {
basic_display_format_contract_info(&info_to_json)
}
// Binary flag applied
(_, true) => {
let wasm_code =
fetch_wasm_code(*info_to_json.code_hash(), &client)
.await?;
match (wasm_code, self.output_json) {
(Some(code), false) => {
std::io::stdout()
.write_all(&code)
.expect("Writing to stdout failed")
}
(Some(code), true) => {
let wasm = serde_json::json!({
"wasm": format!("0x{}", hex::encode(code))
});
println!(
"{}",
serde_json::to_string_pretty(&wasm).map_err(
|err| {
anyhow!(
"JSON serialization failed: {}",
err
)
}
)?
);
}
(None, _) => {
return Err(anyhow!(
"Contract wasm code was not found"
)
.into())
}
}
}
}
Ok(())
}
None => {
Err(anyhow!(
let info_to_json =
fetch_contract_info(contract, &client)
.await?
.ok_or(anyhow!(
"No contract information was found for account id {}",
self.contract
)
.into())
contract
))?;

// Binary flag applied
if self.binary {
let wasm_code = fetch_wasm_code(&client, info_to_json.code_hash())
.await?
.ok_or(anyhow!(
"Contract wasm code was not found for account id {}",
contract
))?;

if self.output_json {
let wasm = serde_json::json!({
"wasm": format!("0x{}", hex::encode(wasm_code))
});
println!("{}", serde_json::to_string_pretty(&wasm)?);
} else {
std::io::stdout()
.write_all(&wasm_code)
.expect("Writing to stdout failed")
}
} else if self.output_json {
println!("{}", info_to_json.to_json()?)
} else {
basic_display_format_contract_info(&info_to_json)
}
})
Ok(())
}
}
}
14 changes: 13 additions & 1 deletion crates/cargo-contract/src/cmd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ use std::io::{
self,
Write,
};
pub use subxt::PolkadotConfig as DefaultConfig;
pub use subxt::{
Config,
PolkadotConfig as DefaultConfig,
};

/// Arguments required for creating and sending an extrinsic to a substrate node.
#[derive(Clone, Debug, clap::Args)]
Expand Down Expand Up @@ -230,3 +233,12 @@ pub fn basic_display_format_contract_info(info: &ContractInfo) {
MAX_KEY_COL_WIDTH
);
}

/// Display all contracts addresses in a formatted way
pub fn display_all_contracts(contracts: &[<DefaultConfig as Config>::AccountId]) {
contracts
.iter()
.for_each(|e: &<DefaultConfig as Config>::AccountId| {
name_value_println!("Contract", format!("{}", e), MAX_KEY_COL_WIDTH);
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rewrite it to display the result something like:

Found contracts:
    * <Address>
    * <Address>
    * <Address>

Copy link
Collaborator Author

@smiasojed smiasojed Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that in case when we have more than 1000 contracts, it will be hard to see this line Found contracts at the log beginning. I would opt to keep it as is. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I would enumerate them at least. Useful when you want to pipe the output into another bash command:

Found contracts:
    1. <Address>
    2. <Address>
    ...
    1000. <Address>

It is up to you if you want to keep Found contracts: or not.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful when you want to pipe the output into another bash command

In this case it is best then to have only the list of contract addresses so instead of

        Contract 5DpyotvcUDkFxTtHzkwR7Rqe37bdjc6M5Rxeiy57Gv2RQzp8
        Contract 5GsoMKLTgFEpzHQTi7c4ZKCLZC6VxtHZxwMydSi79uemABez

have

5DpyotvcUDkFxTtHzkwR7Rqe37bdjc6M5Rxeiy57Gv2RQzp8
5GsoMKLTgFEpzHQTi7c4ZKCLZC6VxtHZxwMydSi79uemABez

I like it this way, unix command compatible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

4 changes: 3 additions & 1 deletion crates/cargo-contract/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ fn exec(cmd: Command) -> Result<()> {
.map_err(|err| map_extrinsic_err(err, remove.output_json()))
})
}
Command::Info(info) => info.run().map_err(format_err),
Command::Info(info) => {
runtime.block_on(async { info.run().await.map_err(format_err) })
}
Command::Verify(verify) => {
let result = verify.run().map_err(format_err)?;

Expand Down
6 changes: 6 additions & 0 deletions crates/extrinsics/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ impl From<std::io::Error> for ErrorVariant {
}
}

impl From<serde_json::Error> for ErrorVariant {
fn from(error: serde_json::Error) -> Self {
Self::Generic(GenericError::from_message(format!("{error:?}")))
}
}

#[derive(serde::Serialize)]
pub struct ModuleError {
pub pallet: String,
Expand Down
15 changes: 15 additions & 0 deletions crates/extrinsics/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,21 @@ async fn build_upload_instantiate_info() {
.assert()
.stdout(predicate::str::contains(r#""wasm": "0x"#));

let output = cargo_contract(project_path.as_path())
.arg("info")
.arg("--all")
.output()
.expect("failed to execute process");
let stdout = str::from_utf8(&output.stdout).unwrap();
let stderr = str::from_utf8(&output.stderr).unwrap();
assert!(
output.status.success(),
"getting all contracts failed: {stderr}"
);

let contract_account_all = extract_contract_address(stdout);
assert_eq!(contract_account_all, contract_account, "{stdout:?}");

// prevent the node_process from being dropped and killed
let _ = node_process;
}
Expand Down
46 changes: 44 additions & 2 deletions crates/extrinsics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ use scale::{
Decode,
Encode,
};
use sp_core::Bytes;
use sp_core::{
hashing,
Bytes,
};
use subxt::{
blocks,
config,
Expand Down Expand Up @@ -364,7 +367,10 @@ impl ContractInfo {
}

/// Fetch the contract wasm code from the storage using the provided client and code hash.
pub async fn fetch_wasm_code(hash: CodeHash, client: &Client) -> Result<Option<Vec<u8>>> {
pub async fn fetch_wasm_code(
client: &Client,
hash: &CodeHash,
) -> Result<Option<Vec<u8>>> {
let pristine_code_address = api::storage().contracts().pristine_code(hash);

let pristine_bytes = client
Expand All @@ -378,6 +384,42 @@ pub async fn fetch_wasm_code(hash: CodeHash, client: &Client) -> Result<Option<V
Ok(pristine_bytes)
}

/// Fetch all contracts addresses from the storage using the provided client and count of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Fetch all contracts addresses from the storage using the provided client and count of
/// Fetch all contract addresses from the storage using the provided client and count of

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// requested elements starting from an optional address
pub async fn fetch_all_contracts(
client: &Client,
count: u32,
from: Option<&AccountId32>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from: Option<&AccountId32>,
count_from: Option<&AccountId32>,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

) -> Result<Vec<AccountId32>> {
let key = api::storage()
.contracts()
.contract_info_of_root()
.to_root_bytes();
let start_key = from
.map(|e| [key.clone(), hashing::twox_64(&e.0).to_vec(), e.0.to_vec()].concat());
let keys = client
.storage()
.at_latest()
.await?
.fetch_keys(key.as_ref(), count, start_key.as_deref())
.await?;

// StorageKey is a concatention of contract_info_of root key and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// StorageKey is a concatention of contract_info_of root key and
// StorageKey is a concatenation of contract_info_of root key and

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

// Twox64Concat(AccountId)
let contracts = keys
.into_iter()
.map(|e| {
let mut account =
e.0.get(key.len() + 8..)
.ok_or(anyhow!("Unexpected storage key size"))?;
AccountId32::decode(&mut account)
.map_err(|err| anyhow!("AccountId deserialization error: {}", err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would extract it into a separate function for better readability

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

})
.collect::<Result<_, _>>()?;

Ok(contracts)
}

/// Copy of `pallet_contracts_primitives::StorageDeposit` which implements `Serialize`,
/// required for json output.
#[derive(Eq, PartialEq, Ord, PartialOrd, Clone, serde::Serialize)]
Expand Down
1 change: 1 addition & 0 deletions docs/info.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ cargo contract info \
- `--url` the url of the rpc endpoint you want to specify - by default `ws://localhost:9944`.
- `--output-json` to export the output as JSON.
- `--binary` outputs Wasm code as a binary blob. If used in combination with `--output-json`, outputs Wasm code as JSON object with hex string.
- `--all` outputs all contracts addresses. It can not be used together with `--binary` flag.