diff --git a/FULL_HELP_DOCS.md b/FULL_HELP_DOCS.md index 42f753f00b..05ff78a7eb 100644 --- a/FULL_HELP_DOCS.md +++ b/FULL_HELP_DOCS.md @@ -4145,7 +4145,7 @@ Encode a transaction envelope from JSON to XDR Decode and encode XDR -**Usage:** `stellar xdr [CHANNEL] ` +**Usage:** `stellar xdr ` ###### **Subcommands:** @@ -4158,14 +4158,6 @@ Decode and encode XDR - `xfile` — Preprocess XDR .x files - `version` — Print version information -###### **Arguments:** - -- `` — Channel of XDR to operate on - - Default value: `+curr` - - Possible values: `+curr`, `+next` - ## `stellar xdr types` View information about types diff --git a/cmd/crates/soroban-test/tests/it/config.rs b/cmd/crates/soroban-test/tests/it/config.rs index f004a25136..9fb79f4550 100644 --- a/cmd/crates/soroban-test/tests/it/config.rs +++ b/cmd/crates/soroban-test/tests/it/config.rs @@ -994,3 +994,210 @@ fn contract_alias_add_rejects_path_traversal() { .stderr(predicate::str::contains("Invalid name")); }); } + +#[test] +fn native_alias_resolves_to_native_asset_contract() { + TestEnv::with_default(|sandbox| { + let native_sac = sandbox + .new_assert_cmd("contract") + .args(["id", "asset", "--asset", "native"]) + .assert() + .success() + .stdout_as_str(); + + let resolved = sandbox + .new_assert_cmd("contract") + .args(["alias", "show", "native"]) + .assert() + .success() + .stdout_as_str(); + + assert_eq!(native_sac, resolved); + }); +} + +#[test] +fn cannot_add_reserved_native_alias() { + TestEnv::with_default(|sandbox| { + sandbox + .new_assert_cmd("contract") + .arg("alias") + .arg("add") + .arg("native") + .arg("--id=CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE") + .assert() + .failure() + .stderr(predicate::str::contains("reserved")); + }); +} + +#[test] +fn can_remove_shadowed_native_alias() { + TestEnv::with_default(|sandbox| { + // A `native` alias created before it became reserved can still be + // removed to clean up the shadowed file. + let contract_ids = sandbox.config_dir().join("contract-ids"); + fs::create_dir_all(&contract_ids).unwrap(); + fs::write( + contract_ids.join("native.json"), + r#"{"ids":{"Standalone Network ; February 2017":"CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"}}"#, + ) + .unwrap(); + + sandbox + .new_assert_cmd("contract") + .args(["alias", "remove", "native"]) + .assert() + .success(); + + // The shadowed entry is gone; only the built-in remains. + sandbox + .new_assert_cmd("contract") + .args(["alias", "ls"]) + .assert() + .success() + .stdout( + predicate::str::contains("(built-in)") + .and(predicate::str::contains("(disabled)").not()), + ); + }); +} + +#[test] +fn alias_ls_always_shows_builtin_native() { + TestEnv::with_default(|sandbox| { + // A user alias makes a network group appear in the listing. + sandbox + .new_assert_cmd("contract") + .args([ + "alias", + "add", + "my-token", + "--id=CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE", + ]) + .assert() + .success(); + + sandbox + .new_assert_cmd("contract") + .args(["alias", "ls"]) + .assert() + .success() + .stdout( + predicate::str::contains("native:").and(predicate::str::contains("(built-in)")), + ); + }); +} + +#[test] +fn alias_ls_marks_preexisting_native_alias_disabled() { + TestEnv::with_default(|sandbox| { + // Simulate a `native` alias created before it became a reserved + // built-in. It is now shadowed and should be listed as disabled. + let contract_ids = sandbox.config_dir().join("contract-ids"); + fs::create_dir_all(&contract_ids).unwrap(); + fs::write( + contract_ids.join("native.json"), + r#"{"ids":{"Standalone Network ; February 2017":"CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"}}"#, + ) + .unwrap(); + + sandbox + .new_assert_cmd("contract") + .args(["alias", "ls"]) + .assert() + .success() + .stdout( + predicate::str::contains("(disabled)").and(predicate::str::contains("(built-in)")), + ); + }); +} + +#[test] +fn remove_native_without_stored_file_reports_reserved() { + TestEnv::with_default(|sandbox| { + // No stored `native.json` exists (the normal case, since `alias add + // native` is blocked). Removal must say the alias is reserved, matching + // what `ls` and `show` report, not "no contract found". + sandbox + .new_assert_cmd("contract") + .args(["alias", "remove", "native"]) + .assert() + .failure() + .stderr( + predicate::str::contains("reserved") + .and(predicate::str::contains("no contract found").not()), + ); + }); +} + +#[test] +fn cannot_generate_reserved_native_key() { + TestEnv::with_default(|sandbox| { + sandbox + .new_assert_cmd("keys") + .arg("generate") + .arg("--seed") + .arg("0000000000000000") + .arg("native") + .assert() + .failure() + .stderr(predicate::str::contains("reserved")); + }); +} + +#[test] +fn cannot_add_reserved_native_key() { + TestEnv::with_default(|sandbox| { + sandbox + .new_assert_cmd("keys") + .arg("add") + .arg("native") + .env( + "STELLAR_SECRET_KEY", + "SBEQMTXGCLDFQG3OXMRSMGLKJCPROAHB5GZCCGVZERDI645LCCCRLFGY", + ) + .assert() + .failure() + .stderr(predicate::str::contains("reserved")); + }); +} + +#[test] +fn shadowed_native_alias_errors_on_show() { + TestEnv::with_default(|sandbox| { + // A `native` alias stored before the name became reserved points at a + // different contract; resolving it must error rather than silently + // returning the native asset contract. + let contract_ids = sandbox.config_dir().join("contract-ids"); + fs::create_dir_all(&contract_ids).unwrap(); + fs::write( + contract_ids.join("native.json"), + r#"{"ids":{"Standalone Network ; February 2017":"CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"}}"#, + ) + .unwrap(); + + sandbox + .new_assert_cmd("contract") + .args(["alias", "show", "native"]) + .assert() + .failure() + .stderr(predicate::str::contains("reserved")); + }); +} + +#[test] +fn cannot_deploy_with_reserved_native_alias() { + // The reserved alias must be rejected before building, simulating, or + // deploying, so this fails fast without needing a network. + TestEnv::with_default(|sandbox| { + sandbox + .new_assert_cmd("contract") + .arg("deploy") + .arg("--alias=native") + .arg("--wasm=/does/not/matter.wasm") + .assert() + .failure() + .stderr(predicate::str::contains("reserved")); + }); +} diff --git a/cmd/soroban-cli/src/commands/contract/alias/add.rs b/cmd/soroban-cli/src/commands/contract/alias/add.rs index 3ff192653e..985cc2dc8a 100644 --- a/cmd/soroban-cli/src/commands/contract/alias/add.rs +++ b/cmd/soroban-cli/src/commands/contract/alias/add.rs @@ -49,6 +49,9 @@ impl Cmd { pub fn run(&self, global_args: &global::Args) -> Result<(), Error> { let print = Print::new(global_args.quiet); let alias = &self.alias; + + crate::config::alias::validate_reserved_aliases(alias)?; + let network = self.network.get(&self.config_locator)?; let network_passphrase = &network.network_passphrase; diff --git a/cmd/soroban-cli/src/commands/contract/alias/ls.rs b/cmd/soroban-cli/src/commands/contract/alias/ls.rs index 733f639da3..34dcf300ee 100644 --- a/cmd/soroban-cli/src/commands/contract/alias/ls.rs +++ b/cmd/soroban-cli/src/commands/contract/alias/ls.rs @@ -2,8 +2,8 @@ use clap::Parser; use std::collections::HashMap; use std::ffi::OsStr; use std::fmt::Debug; +use std::fs; use std::path::Path; -use std::{fs, process}; use crate::commands::config::network; use crate::config::locator::{print_deprecation_warning, Location}; @@ -32,6 +32,7 @@ pub enum Error { struct AliasEntry { alias: String, contract: String, + builtin: bool, } impl Cmd { @@ -45,7 +46,7 @@ impl Cmd { print_deprecation_warning(&config_dir); } } - Location::Global(config_dir) => Self::read_from_config_dir(&config_dir)?, + Location::Global(config_dir) => self.read_from_config_dir(&config_dir)?, } } @@ -76,6 +77,7 @@ impl Cmd { let entry = AliasEntry { alias: alias.clone(), contract: contract_id.clone(), + builtin: false, }; map.entry(network_passphrase.clone()) @@ -88,29 +90,45 @@ impl Cmd { Ok(map) } - fn read_from_config_dir(config_dir: &Path) -> Result<(), Error> { + fn read_from_config_dir(&self, config_dir: &Path) -> Result<(), Error> { let mut map = Self::collect_aliases(config_dir)?; - let mut found = false; + + // The built-in `native` alias resolves to the native asset contract for + // a network, so make sure every known network is listed even when it has + // no stored aliases of its own. + for (_, network, _) in self.config_locator.list_networks_long()? { + map.entry(network.network_passphrase).or_default(); + } for (network_passphrase, list) in &mut map { + if let Some(contract) = alias::resolve_reserved(alias::NATIVE, network_passphrase) { + list.push(AliasEntry { + alias: alias::NATIVE.to_string(), + contract: format!("{contract}"), + builtin: true, + }); + } + println!("ℹ️ Aliases available for network '{network_passphrase}'"); list.sort_by(|a, b| a.alias.cmp(&b.alias)); for entry in list.iter() { - found = true; - println!("{}: {}", entry.alias, entry.contract); + let note = if entry.builtin { + " (built-in)" + } else if alias::is_reserved(&entry.alias) { + // A user alias whose name is now reserved is shadowed by the + // built-in alias of the same name, so it no longer resolves. + " (disabled)" + } else { + "" + }; + println!("{}: {}{note}", entry.alias, entry.contract); } println!(); } - if !found { - eprintln!("⚠️ No aliases defined for network"); - - process::exit(1); - } - Ok(()) } } diff --git a/cmd/soroban-cli/src/commands/contract/alias/remove.rs b/cmd/soroban-cli/src/commands/contract/alias/remove.rs index 2cb86cfe12..ed6f0063b4 100644 --- a/cmd/soroban-cli/src/commands/contract/alias/remove.rs +++ b/cmd/soroban-cli/src/commands/contract/alias/remove.rs @@ -3,7 +3,7 @@ use std::fmt::Debug; use clap::Parser; use crate::commands::{config::network, global}; -use crate::config::{address::AliasName, locator}; +use crate::config::{address::AliasName, alias, locator}; use crate::print::Print; #[derive(Parser, Debug, Clone)] @@ -41,10 +41,20 @@ impl Cmd { let network = self.network.get(&self.config_locator)?; let network_passphrase = &network.network_passphrase; + // Use the stored value so a reserved alias reflects the shadowed file + // being removed, not its built-in resolution. let Some(contract) = self .config_locator - .get_contract_id(&self.alias, network_passphrase)? + .get_stored_contract_id(&self.alias, network_passphrase)? else { + // Without a stored file there's nothing to remove. For a reserved + // alias, say so truthfully instead of "no contract found" — `ls` + // and `show` both report the built-in exists, so that error would + // contradict them. + if alias::is_reserved(&self.alias) { + return Err(locator::Error::ContractAliasReserved(self.alias.to_string()).into()); + } + return Err(Error::NoContract { alias: alias.to_string(), network_passphrase: network_passphrase.into(), diff --git a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs index bb97b8325e..b2617ca9a9 100644 --- a/cmd/soroban-cli/src/commands/contract/arg_parsing.rs +++ b/cmd/soroban-cli/src/commands/contract/arg_parsing.rs @@ -1158,6 +1158,30 @@ mod tests { // A real account strkey that should pass through resolve_address unchanged. const TEST_G_ADDRESS: &str = "GD5KD2KEZJIGTC63IGW6UMUSMVUVG5IHG64HUTFWCHVZH2N2IBOQN7PS"; + #[test] + fn resolve_aliases_resolves_native_to_asset_contract_address() { + let ty = ScSpecTypeDef::Address; + let spec = Spec(Some(vec![])); + let config = crate::config::Args::default(); + + let mut value = serde_json::json!("native"); + let mutated = resolve_aliases_in_json(&mut value, &ty, &spec, &config).unwrap(); + assert!( + mutated, + "native should resolve to the native asset contract" + ); + + let network_passphrase = config.get_network().unwrap().network_passphrase; + let expected = format!( + "{}", + crate::utils::contract_id_hash_from_asset( + &crate::xdr::Asset::Native, + &network_passphrase, + ) + ); + assert_eq!(value, serde_json::Value::String(expected)); + } + #[test] fn resolve_aliases_in_json_walks_vec_of_address() { use stellar_xdr::ScSpecTypeVec; diff --git a/cmd/soroban-cli/src/commands/contract/deploy/asset.rs b/cmd/soroban-cli/src/commands/contract/deploy/asset.rs index e65b21275c..5539448d6c 100644 --- a/cmd/soroban-cli/src/commands/contract/deploy/asset.rs +++ b/cmd/soroban-cli/src/commands/contract/deploy/asset.rs @@ -98,6 +98,12 @@ pub struct Cmd { impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { + // Validate the alias before simulating or deploying, so a reserved alias + // fails fast instead of after an on-chain deploy. + if let Some(alias) = &self.alias { + crate::config::alias::validate_reserved_aliases(alias)?; + } + let res = self .execute(&self.config, global_args.quiet, global_args.no_cache) .await? diff --git a/cmd/soroban-cli/src/commands/contract/deploy/wasm.rs b/cmd/soroban-cli/src/commands/contract/deploy/wasm.rs index 94ed1d0b58..9afdada059 100644 --- a/cmd/soroban-cli/src/commands/contract/deploy/wasm.rs +++ b/cmd/soroban-cli/src/commands/contract/deploy/wasm.rs @@ -167,6 +167,9 @@ pub enum Error { #[error("--alias is not supported when deploying multiple contracts; aliases are derived from package names automatically")] AliasNotSupported, + #[error("workspace package '{0}' resolves to the reserved contract alias '{0}'; rename the package, or deploy it on its own with `--package {0} --alias `")] + ReservedPackageAlias(String), + #[error("--salt is not supported when deploying multiple contracts")] SaltNotSupported, @@ -192,6 +195,14 @@ impl Cmd { let built_contracts = self.resolve_contracts(global_args)?; + // Aliases derived from workspace package names are assigned per-iteration + // inside the deploy loop, so validate them all up front: a package named + // after a reserved alias must fail before any contract is deployed + // on-chain, not partway through the loop. + if let Some(name) = reserved_package_alias(self.alias.as_ref(), &built_contracts) { + return Err(Error::ReservedPackageAlias(name)); + } + // When --wasm-hash is used, no built contracts are returned. // Deploy directly with the hash. if built_contracts.is_empty() { @@ -230,6 +241,14 @@ impl Cmd { } async fn run_single(cmd: &Cmd, global_args: &global::Args) -> Result<(), Error> { + // Validate the finalized alias (explicit or package-derived) at the + // point of use, before any on-chain work. `run` rejects a reserved + // package name up front to avoid a partial multi-contract deploy; this + // is the single guard for the single-contract and `--wasm-hash` paths. + if let Some(alias) = &cmd.alias { + crate::config::alias::validate_reserved_aliases(alias)?; + } + let res = cmd .execute(&cmd.config, global_args.quiet, global_args.no_cache) .await? @@ -445,6 +464,23 @@ impl Cmd { } } +/// Returns the name of the first built contract whose package-derived alias +/// would be reserved. Explicit `--alias` is validated separately (and rejected +/// entirely for multi-contract deploys), so an explicit alias short-circuits. +fn reserved_package_alias( + explicit_alias: Option<&AliasName>, + built_contracts: &[build::BuiltContract], +) -> Option { + if explicit_alias.is_some() { + return None; + } + + built_contracts.iter().find_map(|contract| { + (!contract.name.is_empty() && crate::config::alias::is_reserved(&contract.name)) + .then(|| contract.name.clone()) + }) +} + fn build_create_contract_tx( wasm_hash: Hash, sequence: i64, @@ -524,4 +560,39 @@ mod tests { assert!(result.is_ok()); } + + fn built(name: &str) -> build::BuiltContract { + build::BuiltContract { + name: name.to_string(), + path: std::path::PathBuf::new(), + } + } + + #[test] + fn reserved_package_alias_flags_reserved_package_before_deploy() { + let native = crate::config::alias::NATIVE; + let contracts = [built("adapter"), built(native), built("token")]; + + assert_eq!( + reserved_package_alias(None, &contracts), + Some(native.to_string()) + ); + } + + #[test] + fn reserved_package_alias_ignores_regular_packages() { + let contracts = [built("adapter"), built("token")]; + + assert_eq!(reserved_package_alias(None, &contracts), None); + } + + #[test] + fn reserved_package_alias_skipped_with_explicit_alias() { + // An explicit --alias is validated on its own path; a reserved package + // name is irrelevant because the derived alias is never used. + let alias = "my-contract".parse::().unwrap(); + let contracts = [built(crate::config::alias::NATIVE)]; + + assert_eq!(reserved_package_alias(Some(&alias), &contracts), None); + } } diff --git a/cmd/soroban-cli/src/config/alias.rs b/cmd/soroban-cli/src/config/alias.rs index 734925c4ef..0d4cfc758c 100644 --- a/cmd/soroban-cli/src/config/alias.rs +++ b/cmd/soroban-cli/src/config/alias.rs @@ -1,14 +1,55 @@ use std::{collections::HashMap, convert::Infallible, str::FromStr}; use serde::{Deserialize, Serialize}; +use stellar_strkey::Contract; use super::locator; +use crate::utils::contract_id_hash_from_asset; +use crate::xdr::Asset; #[derive(Serialize, Deserialize, Default)] pub struct Data { pub ids: HashMap, } +/// The reserved, built-in contract alias. It resolves to the native asset (XLM) +/// Stellar Asset Contract for the current network and cannot be created, +/// overwritten, or removed by users. +pub const NATIVE: &str = "native"; + +/// Returns `true` if `alias` is the reserved, built-in alias that users cannot +/// create, overwrite, or remove. +#[must_use] +pub fn is_reserved(alias: &str) -> bool { + alias == NATIVE +} + +/// Resolves the reserved, built-in alias to its contract for `network_passphrase`, +/// or returns `None` if `alias` is not reserved. This is the single source of +/// truth for what the reserved alias points to, so resolution stays consistent +/// across `get_contract_id`, `alias show`, and `alias ls`. +#[must_use] +pub fn resolve_reserved(alias: &str, network_passphrase: &str) -> Option { + if is_reserved(alias) { + Some(contract_id_hash_from_asset( + &Asset::Native, + network_passphrase, + )) + } else { + None + } +} + +/// Errors if `alias` is a reserved, built-in alias. Call this before doing any +/// work (building, simulating, deploying, or writing config) so that a reserved +/// alias fails fast. +pub fn validate_reserved_aliases(alias: &str) -> Result<(), locator::Error> { + if is_reserved(alias) { + return Err(locator::Error::ContractAliasReserved(alias.to_owned())); + } + Ok(()) +} + /// Address can be either a contract address, C.. or eventually an alias of a contract address. #[derive(Clone, Debug)] pub enum UnresolvedContract { @@ -57,3 +98,20 @@ impl UnresolvedContract { .ok_or_else(|| locator::Error::ContractNotFound(alias.to_owned())) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn native_is_reserved() { + assert!(is_reserved("native")); + assert!(validate_reserved_aliases("native").is_err()); + } + + #[test] + fn regular_aliases_are_not_reserved() { + assert!(!is_reserved("my-token")); + assert!(validate_reserved_aliases("my-token").is_ok()); + } +} diff --git a/cmd/soroban-cli/src/config/locator.rs b/cmd/soroban-cli/src/config/locator.rs index 7a02091f55..f36c91938a 100644 --- a/cmd/soroban-cli/src/config/locator.rs +++ b/cmd/soroban-cli/src/config/locator.rs @@ -92,6 +92,10 @@ pub enum Error { UpgradeCheckWriteFailed { path: PathBuf, error: io::Error }, #[error("Contract alias {0}, cannot overlap with key")] ContractAliasCannotOverlapWithKey(String), + #[error("'{0}' is reserved for the built-in native asset contract and cannot be added, overwritten, or removed")] + ContractAliasReserved(String), + #[error("alias '{alias}' is reserved for the native asset contract, but a stored alias points to {stored}; remove it with `stellar contract alias remove {alias}`, or use the contract id directly")] + ShadowedReservedAlias { alias: String, stored: Contract }, #[error("Key cannot {0} cannot overlap with contract alias")] KeyCannotOverlapWithContractAlias(String), #[error(transparent)] @@ -191,6 +195,7 @@ impl Args { } pub fn write_identity(&self, name: &str, secret: &Secret) -> Result { + alias::validate_reserved_aliases(name)?; if let Ok(Some(_)) = self.load_contract_from_alias(name) { return Err(Error::KeyCannotOverlapWithContractAlias(name.to_owned())); } @@ -206,6 +211,7 @@ impl Args { } pub fn write_key(&self, name: &str, key: &Key) -> Result { + alias::validate_reserved_aliases(name)?; KeyType::Identity.write(name, key, &self.config_dir()?) } @@ -456,6 +462,7 @@ impl Args { contract_id: &stellar_strkey::Contract, alias: &str, ) -> Result<(), Error> { + alias::validate_reserved_aliases(alias)?; if self.read_identity(alias).is_ok() { return Err(Error::ContractAliasCannotOverlapWithKey(alias.to_owned())); } @@ -493,6 +500,8 @@ impl Args { } pub fn remove_contract_id(&self, network_passphrase: &str, alias: &str) -> Result<(), Error> { + // Reserved aliases cannot be added or overwritten, but a stored file + // that shadows one (created before it became reserved) may be removed. let path = self.alias_path(alias)?; if !path.is_file() { @@ -513,6 +522,34 @@ impl Args { &self, alias: &str, network_passphrase: &str, + ) -> Result, Error> { + // A reserved alias (e.g. `native`) always resolves to its built-in + // contract. If a stored (shadowed) alias file points somewhere else, + // refuse to silently override it: resolving to the built-in anyway + // would misdirect the command to the wrong contract. The stored file + // can still be removed with `contract alias remove `. + if let Some(reserved) = alias::resolve_reserved(alias, network_passphrase) { + if let Some(stored) = self.get_stored_contract_id(alias, network_passphrase)? { + if stored != reserved { + return Err(Error::ShadowedReservedAlias { + alias: alias.to_owned(), + stored, + }); + } + } + + return Ok(Some(reserved)); + } + + self.get_stored_contract_id(alias, network_passphrase) + } + + /// Reads a contract id from a stored alias file, ignoring built-in reserved + /// aliases. Returns `None` when no matching alias file entry exists. + pub fn get_stored_contract_id( + &self, + alias: &str, + network_passphrase: &str, ) -> Result, Error> { let Some(alias_data) = self.load_contract_from_alias(alias)? else { return Ok(None); @@ -1005,6 +1042,107 @@ mod tests { ); } + #[test] + fn save_contract_id_rejects_reserved_native_alias() { + let dir = tempfile::tempdir().unwrap(); + let args = Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let contract = "CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE" + .parse() + .unwrap(); + let native = alias::NATIVE; + + let err = args + .save_contract_id("Test Network", &contract, native) + .unwrap_err(); + + assert!(matches!(err, Error::ContractAliasReserved(alias) if alias == native)); + } + + #[test] + fn get_contract_id_resolves_native_alias() { + let dir = tempfile::tempdir().unwrap(); + let args = Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let network_passphrase = "Test Network"; + let native = alias::NATIVE; + + let resolved = args + .get_contract_id(native, network_passphrase) + .unwrap() + .expect("native alias should resolve"); + let expected = + crate::utils::contract_id_hash_from_asset(&xdr::Asset::Native, network_passphrase); + + assert_eq!(resolved, expected); + } + + #[test] + fn get_contract_id_errors_when_native_alias_is_shadowed() { + let dir = tempfile::tempdir().unwrap(); + let args = Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let network_passphrase = "Test Network"; + let native = alias::NATIVE; + + // A native alias stored before the name became reserved, pointing at a + // different contract than the native asset SAC. + let contract_ids = dir.path().join("contract-ids"); + std::fs::create_dir_all(&contract_ids).unwrap(); + std::fs::write( + contract_ids.join(format!("{native}.json")), + format!( + r#"{{"ids":{{"{network_passphrase}":"CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"}}}}"# + ), + ) + .unwrap(); + + let err = args + .get_contract_id(native, network_passphrase) + .unwrap_err(); + + assert!(matches!(err, Error::ShadowedReservedAlias { alias, .. } if alias == native)); + } + + #[test] + fn write_identity_rejects_reserved_native_name() { + use crate::config::secret::Secret; + use std::str::FromStr; + + let dir = tempfile::tempdir().unwrap(); + let args = Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let secret = + Secret::from_str("SBEQMTXGCLDFQG3OXMRSMGLKJCPROAHB5GZCCGVZERDI645LCCCRLFGY").unwrap(); + let native = alias::NATIVE; + + let err = args.write_identity(native, &secret).unwrap_err(); + + assert!(matches!(err, Error::ContractAliasReserved(name) if name == native)); + } + + #[test] + fn write_key_rejects_reserved_native_name() { + use crate::config::key::Key; + use std::str::FromStr; + + let dir = tempfile::tempdir().unwrap(); + let args = Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let key = + Key::from_str("SBEQMTXGCLDFQG3OXMRSMGLKJCPROAHB5GZCCGVZERDI645LCCCRLFGY").unwrap(); + let native = alias::NATIVE; + + let err = args.write_key(native, &key).unwrap_err(); + + assert!(matches!(err, Error::ContractAliasReserved(name) if name == native)); + } + #[test] fn test_write_sets_file_permissions_to_0600() { use std::os::unix::fs::PermissionsExt; diff --git a/cmd/soroban-cli/src/config/sc_address.rs b/cmd/soroban-cli/src/config/sc_address.rs index 96f2b8512f..ff36a1fc4d 100644 --- a/cmd/soroban-cli/src/config/sc_address.rs +++ b/cmd/soroban-cli/src/config/sc_address.rs @@ -2,7 +2,7 @@ use std::str::FromStr; use crate::xdr; -use super::{key, locator, UnresolvedContract}; +use super::{alias, key, locator, UnresolvedContract}; /// `ScAddress` can be either a resolved `xdr::ScAddress` or an alias of a `Contract` or `MuxedAccount`. #[allow(clippy::module_name_repetitions)] @@ -26,6 +26,8 @@ pub enum Error { Key(#[from] key::Error), #[error("Account alias \"{0}\" not Found")] AccountAliasNotFound(String), + #[error("alias '{0}' is reserved for the native asset contract but also matches a stored key; pass an explicit contract (C...) or account (G...) address instead")] + ReservedAliasShadowsKey(String), } impl FromStr for UnresolvedScAddress { @@ -54,6 +56,12 @@ impl UnresolvedScAddress { let key = locator.read_key(&alias); match (contract, key) { (Ok(contract), Ok(_)) => { + // A reserved built-in alias (e.g. `native`) shadows an on-disk + // key of the same name. Preferring either side could send funds + // to the wrong address, so refuse and ask for an explicit one. + if alias::is_reserved(&alias) { + return Err(Error::ReservedAliasShadowsKey(alias)); + } eprintln!( "Warning: ScAddress alias {alias} is ambiguous, assuming it is a contract" ); @@ -64,6 +72,11 @@ impl UnresolvedScAddress { (Ok(contract), _) => Ok(xdr::ScAddress::Contract(stellar_xdr::ContractId( xdr::Hash(contract.0), ))), + // Surface a shadowed reserved-alias collision rather than masking it + // with a generic "not found" error. This must precede the key arm: + // when both a stored `native` alias and a `native` key exist, the + // collision has to win so resolution can't silently pick the key. + (Err(err @ locator::Error::ShadowedReservedAlias { .. }), _) => Err(err.into()), (_, Ok(key)) => Ok(xdr::ScAddress::Account( key.muxed_account(hd_path)?.account_id(), )), @@ -71,3 +84,70 @@ impl UnresolvedScAddress { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::key::Key; + use crate::config::locator::KeyType; + use std::str::FromStr; + + #[test] + fn resolve_errors_when_reserved_alias_shadows_key() { + let dir = tempfile::tempdir().unwrap(); + let locator = locator::Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let network_passphrase = "Test Network"; + let native = alias::NATIVE; + + // A key named after the native alias, created before it became + // reserved. Written directly since `write_identity` now rejects it. + let key = + Key::from_str("SBEQMTXGCLDFQG3OXMRSMGLKJCPROAHB5GZCCGVZERDI645LCCCRLFGY").unwrap(); + KeyType::Identity.write(native, &key, dir.path()).unwrap(); + + let err = UnresolvedScAddress::Alias(native.to_string()) + .resolve(&locator, network_passphrase, None) + .unwrap_err(); + + assert!(matches!(err, Error::ReservedAliasShadowsKey(alias) if alias == native)); + } + + #[test] + fn resolve_errors_when_reserved_alias_shadowed_by_stored_alias_and_key() { + let dir = tempfile::tempdir().unwrap(); + let locator = locator::Args { + config_dir: Some(dir.path().to_path_buf()), + }; + let network_passphrase = "Test Network"; + let native = alias::NATIVE; + + // Both pre-upgrade artifacts exist: a key named after the native alias + // and a stored alias of the same name pointing at another contract. The + // shadowed-alias error must win rather than the resolution silently + // picking the key. + let key = + Key::from_str("SBEQMTXGCLDFQG3OXMRSMGLKJCPROAHB5GZCCGVZERDI645LCCCRLFGY").unwrap(); + KeyType::Identity.write(native, &key, dir.path()).unwrap(); + + let contract_ids = dir.path().join("contract-ids"); + std::fs::create_dir_all(&contract_ids).unwrap(); + std::fs::write( + contract_ids.join(format!("{native}.json")), + format!( + r#"{{"ids":{{"{network_passphrase}":"CA3D5KRYM6CB7OWQ6TWYRR3Z4T7GNZLKERYNZGGA5SOAOPIFY6YQGAXE"}}}}"# + ), + ) + .unwrap(); + + let err = UnresolvedScAddress::Alias(native.to_string()) + .resolve(&locator, network_passphrase, None) + .unwrap_err(); + + assert!(matches!( + err, + Error::Locator(locator::Error::ShadowedReservedAlias { alias, .. }) if alias == native + )); + } +}