Repository: CosmWasm/cosmwasm
Pull request #1985 (2024-01-09 00:37:26)

BackendApi improvements
addr_validate can be implemented in Go directly such that we do not need to get the canonical address through cgo into Rust and send it back to get the humanized address again.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 648f77a38..07c621932 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -78,6 +78,10 @@ and this project adheres to
   `set_withdraw_address`, `set_withdraw_addresses`, `clear_withdraw_addresses`,
   `update_ibc` and `update_staking` from `MockQuerier` and expose the underlying
   queriers directly. ([#1977])
+- cosmwasm-vm: Rename `BackendApi::canonical_address`/`::human_address` to
+  `::addr_canonicalize`/`::addr_humanize` for consistency.
+- cosmwasm-vm: Add `BackendApi::addr_validate` to avoid having to do two calls
+  from Rust into Go.
 
 [#1874]: https://github.com/CosmWasm/cosmwasm/pull/1874
 [#1876]: https://github.com/CosmWasm/cosmwasm/pull/1876
diff --git a/packages/vm/src/backend.rs b/packages/vm/src/backend.rs
index 1a6d9acee..c2c54e16b 100644
--- a/packages/vm/src/backend.rs
+++ b/packages/vm/src/backend.rs
@@ -165,12 +165,10 @@ pub trait Storage {
 /// Currently it just supports address conversion, we could add eg. crypto functions here.
 /// These should all be pure (stateless) functions. If you need state, you probably want
 /// to use the Querier.
-///
-/// We can use feature flags to opt-in to non-essential methods
-/// for backwards compatibility in systems that don't have them all.
-pub trait BackendApi: Copy + Clone + Send {
-    fn canonical_address(&self, human: &str) -> BackendResult<Vec<u8>>;
-    fn human_address(&self, canonical: &[u8]) -> BackendResult<String>;
+pub trait BackendApi: Clone + Send {
+    fn addr_validate(&self, input: &str) -> BackendResult<()>;
+    fn addr_canonicalize(&self, human: &str) -> BackendResult<Vec<u8>>;
+    fn addr_humanize(&self, canonical: &[u8]) -> BackendResult<String>;
 }
 
 pub trait Querier {
diff --git a/packages/vm/src/environment.rs b/packages/vm/src/environment.rs
index ad6bac36f..23ac34384 100644
--- a/packages/vm/src/environment.rs
+++ b/packages/vm/src/environment.rs
@@ -125,7 +125,7 @@ impl<A: BackendApi, S: Storage, Q: Querier> Clone for Environment<A, S, Q> {
     fn clone(&self) -> Self {
         Environment {
             memory: None,
-            api: self.api,
+            api: self.api.clone(),
             gas_config: self.gas_config.clone(),
             data: self.data.clone(),
         }
diff --git a/packages/vm/src/imports.rs b/packages/vm/src/imports.rs
index d1c245d30..a02bd496b 100644
--- a/packages/vm/src/imports.rs
+++ b/packages/vm/src/imports.rs
@@ -162,31 +162,15 @@ pub fn do_addr_validate<A: BackendApi + 'static, S: Storage + 'static, Q: Querie
         Err(_) => return write_to_contract(data, &mut store, b"Input is not valid UTF-8"),
     };
 
-    let (result, gas_info) = data.api.canonical_address(&source_string);
+    let (result, gas_info) = data.api.addr_validate(&source_string);
     process_gas_info(data, &mut store, gas_info)?;
-    let canonical = match result {
-        Ok(data) => data,
-        Err(BackendError::UserErr { msg, .. }) => {
-            return write_to_contract(data, &mut store, msg.as_bytes())
-        }
-        Err(err) => return Err(VmError::from(err)),
-    };
-
-    let (result, gas_info) = data.api.human_address(&canonical);
-    process_gas_info(data, &mut store, gas_info)?;
-    let normalized = match result {
-        Ok(addr) => addr,
+    match result {
+        Ok(()) => Ok(0),
         Err(BackendError::UserErr { msg, .. }) => {
-            return write_to_contract(data, &mut store, msg.as_bytes())
+            write_to_contract(data, &mut store, msg.as_bytes())
         }
-        Err(err) => return Err(VmError::from(err)),
-    };
-
-    if normalized != source_string {
-        return write_to_contract(data, &mut store, b"Address is not normalized");
+        Err(err) => Err(VmError::from(err)),
     }
-
-    Ok(0)
 }
 
 pub fn do_addr_canonicalize<A: BackendApi + 'static, S: Storage + 'static, Q: Querier + 'static>(
@@ -206,7 +190,7 @@ pub fn do_addr_canonicalize<A: BackendApi + 'static, S: Storage + 'static, Q: Qu
         Err(_) => return write_to_contract(data, &mut store, b"Input is not valid UTF-8"),
     };
 
-    let (result, gas_info) = data.api.canonical_address(&source_string);
+    let (result, gas_info) = data.api.addr_canonicalize(&source_string);
     process_gas_info(data, &mut store, gas_info)?;
     match result {
         Ok(canonical) => {
@@ -233,7 +217,7 @@ pub fn do_addr_humanize<A: BackendApi + 'static, S: Storage + 'static, Q: Querie
         MAX_LENGTH_CANONICAL_ADDRESS,
     )?;
 
-    let (result, gas_info) = data.api.human_address(&canonical);
+    let (result, gas_info) = data.api.addr_humanize(&canonical);
     process_gas_info(data, &mut store, gas_info)?;
     match result {
         Ok(human) => {
diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs
index a1f4ced07..adf82c2e2 100644
--- a/packages/vm/src/instance.rs
+++ b/packages/vm/src/instance.rs
@@ -294,7 +294,7 @@ where
 
         let env = fe.as_ref(&store);
         if let (Some(storage), Some(querier)) = env.move_out() {
-            let api = env.api;
+            let api = env.api.clone();
             Some(Backend {
                 api,
                 storage,


Test patch:
diff --git a/packages/vm/src/testing/mock.rs b/packages/vm/src/testing/mock.rs
index b5823e83c..941e8fb53 100644
--- a/packages/vm/src/testing/mock.rs
+++ b/packages/vm/src/testing/mock.rs
@@ -125,7 +125,37 @@ impl Default for MockApi {
 }
 
 impl BackendApi for MockApi {
-    fn canonical_address(&self, input: &str) -> BackendResult<Vec<u8>> {
+    fn addr_validate(&self, input: &str) -> BackendResult<()> {
+        let mut gas_total = GasInfo {
+            cost: 0,
+            externally_used: 0,
+        };
+
+        let (result, gas_info) = self.addr_canonicalize(input);
+        gas_total += gas_info;
+        let canonical = match result {
+            Ok(canonical) => canonical,
+            Err(err) => return (Err(err), gas_total),
+        };
+
+        let (result, gas_info) = self.addr_humanize(&canonical);
+        gas_total += gas_info;
+        let normalized = match result {
+            Ok(norm) => norm,
+            Err(err) => return (Err(err), gas_total),
+        };
+        if input != normalized.as_str() {
+            return (
+                Err(BackendError::user_err(
+                    "Invalid input: address not normalized",
+                )),
+                gas_total,
+            );
+        }
+        (Ok(()), gas_total)
+    }
+
+    fn addr_canonicalize(&self, input: &str) -> BackendResult<Vec<u8>> {
         let gas_info = GasInfo::with_cost(GAS_COST_CANONICALIZE);
 
         // handle error case
@@ -156,7 +186,7 @@ impl BackendApi for MockApi {
         }
     }
 
-    fn human_address(&self, canonical: &[u8]) -> BackendResult<String> {
+    fn addr_humanize(&self, canonical: &[u8]) -> BackendResult<String> {
         let gas_info = GasInfo::with_cost(GAS_COST_HUMANIZE);
 
         // handle error case
@@ -232,20 +262,20 @@ mod tests {
     }
 
     #[test]
-    fn canonical_address_works() {
+    fn addr_canonicalize_works() {
         let api = MockApi::default().with_prefix("osmo");
 
-        api.canonical_address("osmo186kh7c0k0gh4ww0wh4jqc4yhzu7n7dhswe845d")
+        api.addr_canonicalize("osmo186kh7c0k0gh4ww0wh4jqc4yhzu7n7dhswe845d")
             .0
             .unwrap();
 
         // is case insensitive
         let data1 = api
-            .canonical_address("osmo186kh7c0k0gh4ww0wh4jqc4yhzu7n7dhswe845d")
+            .addr_canonicalize("osmo186kh7c0k0gh4ww0wh4jqc4yhzu7n7dhswe845d")
             .0
             .unwrap();
         let data2 = api
-            .canonical_address("OSMO186KH7C0K0GH4WW0WH4JQC4YHZU7N7DHSWE845D")
+            .addr_canonicalize("OSMO186KH7C0K0GH4WW0WH4JQC4YHZU7N7DHSWE845D")
             .0
             .unwrap();
         assert_eq!(data1, data2);
@@ -257,29 +287,29 @@ mod tests {
 
         // simple
         let original = api.addr_make("shorty");
-        let canonical = api.canonical_address(&original).0.unwrap();
-        let (recovered, _gas_cost) = api.human_address(&canonical);
+        let canonical = api.addr_canonicalize(&original).0.unwrap();
+        let (recovered, _gas_cost) = api.addr_humanize(&canonical);
         assert_eq!(recovered.unwrap(), original);
 
         // normalizes input
         let original = "JUNO1MEPRU9FUQ4E65856ARD6068MFSFRWPGEMD0C3R";
-        let canonical = api.canonical_address(original).0.unwrap();
-        let recovered = api.human_address(&canonical).0.unwrap();
+        let canonical = api.addr_canonicalize(original).0.unwrap();
+        let recovered = api.addr_humanize(&canonical).0.unwrap();
         assert_eq!(recovered, original.to_lowercase());
 
         // Long input (Juno contract address)
         let original =
             String::from("juno1v82su97skv6ucfqvuvswe0t5fph7pfsrtraxf0x33d8ylj5qnrysdvkc95");
-        let canonical = api.canonical_address(&original).0.unwrap();
-        let recovered = api.human_address(&canonical).0.unwrap();
+        let canonical = api.addr_canonicalize(&original).0.unwrap();
+        let recovered = api.addr_humanize(&canonical).0.unwrap();
         assert_eq!(recovered, original);
     }
 
     #[test]
-    fn human_address_input_length() {
+    fn addr_humanize_input_length() {
         let api = MockApi::default();
         let input = vec![61; 256]; // too long
-        let (result, _gas_info) = api.human_address(&input);
+        let (result, _gas_info) = api.addr_humanize(&input);
         match result.unwrap_err() {
             BackendError::UserErr { .. } => {}
             err => panic!("Unexpected error: {err:?}"),
@@ -287,26 +317,26 @@ mod tests {
     }
 
     #[test]
-    fn canonical_address_min_input_length() {
+    fn addr_canonicalize_min_input_length() {
         let api = MockApi::default();
 
         // empty address should fail
         let empty = "cosmwasm1pj90vm";
         assert!(matches!(api
-            .canonical_address(empty)
+            .addr_canonicalize(empty)
             .0
             .unwrap_err(),
             BackendError::UserErr { msg } if msg.contains("address length")));
     }
 
     #[test]
-    fn canonical_address_max_input_length() {
+    fn addr_canonicalize_max_input_length() {
         let api = MockApi::default();
 
         let too_long = "cosmwasm1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqehqqkz";
 
         assert!(matches!(api
-            .canonical_address(too_long)
+            .addr_canonicalize(too_long)
             .0
             .unwrap_err(),
             BackendError::UserErr { msg } if msg.contains("address length")));
@@ -316,10 +346,10 @@ mod tests {
     fn colon_in_prefix_is_valid() {
         let mock_api = MockApi::default().with_prefix("did:com:");
         let bytes = mock_api
-            .canonical_address("did:com:1jkf0kmeyefvyzpwf56m7sne2000ay53r6upttu")
+            .addr_canonicalize("did:com:1jkf0kmeyefvyzpwf56m7sne2000ay53r6upttu")
             .0
             .unwrap();
-        let humanized = mock_api.human_address(&bytes).0.unwrap();
+        let humanized = mock_api.addr_humanize(&bytes).0.unwrap();
 
         assert_eq!(
             humanized.as_str(),