Skip to content

Commit

Permalink
Fix for a Url to String conversion in info command (#1330)
Browse files Browse the repository at this point in the history
* Fix for Url conversion in contract info command

* code cleanup

* Changelog updated

* Removed duplicated url_to_string function

* Typo fixed

* Changed url() method of ExtrinsicOpts to return String
  • Loading branch information
smiasojed authored Sep 14, 2023
1 parent 3454ae9 commit 004e44e
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- 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)
- Fix for a Url to String conversion in `info` command - [#1330](https://github.com/paritytech/cargo-contract/pull/1330)

## [4.0.0-alpha]

Expand Down
4 changes: 3 additions & 1 deletion crates/cargo-contract/src/cmd/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use contract_extrinsics::{
fetch_all_contracts,
fetch_contract_info,
fetch_wasm_code,
url_to_string,
ErrorVariant,
};
use std::{
Expand Down Expand Up @@ -70,7 +71,8 @@ pub struct InfoCommand {

impl InfoCommand {
pub async fn run(&self) -> Result<(), ErrorVariant> {
let client = OnlineClient::<DefaultConfig>::from_url(&self.url).await?;
let client =
OnlineClient::<DefaultConfig>::from_url(url_to_string(&self.url)).await?;

// All flag applied
if self.all {
Expand Down
6 changes: 3 additions & 3 deletions crates/extrinsics/src/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ impl CallCommandBuilder<state::Message, state::ExtrinsicOptions> {

let signer = self.opts.extrinsic_opts.signer()?;

let url = self.opts.extrinsic_opts.url_to_string();
let client = OnlineClient::from_url(url.clone()).await?;
let url = self.opts.extrinsic_opts.url();
let client = OnlineClient::from_url(url).await?;
Ok(CallExec {
contract: self.opts.contract.clone(),
message: self.opts.message.clone(),
Expand Down Expand Up @@ -221,7 +221,7 @@ impl CallExec {
/// Returns the dry run simulation result of type [`ContractExecResult`], which
/// includes information about the simulated call, or an error in case of failure.
pub async fn call_dry_run(&self) -> Result<ContractExecResult<Balance, ()>> {
let url = self.opts.url_to_string();
let url = self.opts.url();
let token_metadata = TokenMetadata::query(&self.client).await?;
let storage_deposit_limit = self
.opts
Expand Down
17 changes: 3 additions & 14 deletions crates/extrinsics/src/extrinsic_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use anyhow::{
};

use crate::{
url_to_string,
Balance,
BalanceVariant,
ContractArtifacts,
Expand Down Expand Up @@ -169,18 +170,6 @@ impl ExtrinsicOpts {
Ok(keypair)
}

/// Convert URL to String without omitting the default port
pub fn url_to_string(&self) -> String {
let mut res = self.url.to_string();
match (self.url.port(), self.url.port_or_known_default()) {
(None, Some(port)) => {
res.insert_str(res.len() - 1, &format!(":{port}"));
res
}
_ => res,
}
}

/// Return the file path of the contract artifact.
pub fn file(&self) -> Option<&PathBuf> {
self.file.as_ref()
Expand All @@ -192,8 +181,8 @@ impl ExtrinsicOpts {
}

/// Return the URL of the substrate node.
pub fn url(&self) -> &Url {
&self.url
pub fn url(&self) -> String {
url_to_string(&self.url)
}

/// Return the secret URI of the signer.
Expand Down
4 changes: 2 additions & 2 deletions crates/extrinsics/src/instantiate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl InstantiateCommandBuilder<state::ExtrinsicOptions> {
let transcoder = artifacts.contract_transcoder()?;
let data = transcoder.encode(&self.opts.constructor, &self.opts.args)?;
let signer = self.opts.extrinsic_opts.signer()?;
let url = self.opts.extrinsic_opts.url_to_string();
let url = self.opts.extrinsic_opts.url();
let code = if let Some(code) = artifacts.code {
Code::Upload(code.0)
} else {
Expand All @@ -174,7 +174,7 @@ impl InstantiateCommandBuilder<state::ExtrinsicOptions> {
};
let salt = self.opts.salt.clone().map(|s| s.0).unwrap_or_default();

let client = OnlineClient::from_url(url.clone()).await?;
let client = OnlineClient::from_url(&url).await?;

let token_metadata = TokenMetadata::query(&client).await?;

Expand Down
38 changes: 38 additions & 0 deletions crates/extrinsics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,21 @@ pub async fn fetch_all_contracts(
Ok(contracts)
}

// Converts a Url into a String representation without excluding the default port.
pub fn url_to_string(url: &url::Url) -> String {
match (url.port(), url.port_or_known_default()) {
(None, Some(port)) => {
format!(
"{}:{port}{}",
&url[..url::Position::AfterHost],
&url[url::Position::BeforePath..]
)
.to_string()
}
_ => url.to_string(),
}
}

/// Copy of `pallet_contracts_primitives::StorageDeposit` which implements `Serialize`,
/// required for json output.
#[derive(Eq, PartialEq, Ord, PartialOrd, Clone, serde::Serialize)]
Expand Down Expand Up @@ -473,4 +488,27 @@ mod tests {
)
.is_ok())
}

#[test]
fn url_to_string_works() {
// with custom port
let url = url::Url::parse("ws://127.0.0.1:9944").unwrap();
assert_eq!(url_to_string(&url), "ws://127.0.0.1:9944/");

// with default port
let url = url::Url::parse("wss://127.0.0.1:443").unwrap();
assert_eq!(url_to_string(&url), "wss://127.0.0.1:443/");

// with default port and path
let url = url::Url::parse("wss://127.0.0.1:443/test/1").unwrap();
assert_eq!(url_to_string(&url), "wss://127.0.0.1:443/test/1");

// with default port and domain
let url = url::Url::parse("wss://test.io:443").unwrap();
assert_eq!(url_to_string(&url), "wss://test.io:443/");

// with default port, domain and path
let url = url::Url::parse("wss://test.io/test/1").unwrap();
assert_eq!(url_to_string(&url), "wss://test.io:443/test/1");
}
}
4 changes: 2 additions & 2 deletions crates/extrinsics/src/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ impl RemoveCommandBuilder<state::ExtrinsicOptions> {
artifacts_path.display()
)),
}?;
let url = self.opts.extrinsic_opts.url_to_string();
let client = OnlineClient::from_url(url.clone()).await?;
let url = self.opts.extrinsic_opts.url();
let client = OnlineClient::from_url(url).await?;

Ok(RemoveExec {
final_code_hash,
Expand Down
6 changes: 3 additions & 3 deletions crates/extrinsics/src/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ impl UploadCommandBuilder<state::ExtrinsicOptions> {
artifacts_path.display()
)
})?;
let url = self.opts.extrinsic_opts.url_to_string();
let client = OnlineClient::from_url(url.clone()).await?;
let url = self.opts.extrinsic_opts.url();
let client = OnlineClient::from_url(url).await?;
Ok(UploadExec {
opts: self.opts.extrinsic_opts.clone(),
client,
Expand All @@ -131,7 +131,7 @@ impl UploadExec {
/// then sends the request using the provided URL. This operation does not modify
/// the state of the blockchain.
pub async fn upload_code_rpc(&self) -> Result<CodeUploadResult<CodeHash, Balance>> {
let url = self.opts.url_to_string();
let url = self.opts.url();
let token_metadata = TokenMetadata::query(&self.client).await?;
let storage_deposit_limit = self
.opts
Expand Down

0 comments on commit 004e44e

Please sign in to comment.