Merge pull request #443 from CosmWasm/do_canonicalize_address-error

Allow canonicalize_address/humanize_address to return an error string

Author: webmaster128

CHANGELOG.md

@@ -82,6 +82,9 @@
   contract. A standard library should ensure this never happens by correctly
   encoding string input values.
 - Merge trait `ReadonlyStorage` into `Storage`.
+- The imports `canonicalize_address` and `humanize_address` now return a memory
+  address to an error `Region`. If this address is 0, the call succeeded.
+- Bump `cosmwasm_vm_version_1` to `cosmwasm_vm_version_2`.
 
 ## 0.8.1 (2020-06-08)
 

README.md

@@ -138,8 +138,8 @@ extern "C" {
     #[cfg(feature = "iterator")]
     fn db_next(iterator_id: u32) -> u32;
 
-    fn canonicalize_address(source: u32, destination: u32) -> i32;
-    fn humanize_address(source: u32, destination: u32) -> i32;
+    fn canonicalize_address(source: u32, destination: u32) -> u32;
+    fn humanize_address(source: u32, destination: u32) -> u32;
 
     /// Executes a query on the chain (import). Not to be confused with the
     /// query export, which queries the state of the contract.

packages/std/src/entry_points.rs

@@ -96,7 +96,7 @@ macro_rules! create_entry_points {
 
             $crate::create_entry_points!(@migration; $contract, $migration);
 
-            // Other C externs like cosmwasm_vm_version_1, allocate, deallocate are available
+            // Other C externs like cosmwasm_vm_version_2, allocate, deallocate are available
             // automatically because we `use cosmwasm_std`.
         }
     };

packages/std/src/exports.rs

@@ -1,6 +1,6 @@
 //! exports exposes the public wasm API
 //!
-//! cosmwasm_vm_version_1, allocate and deallocate turn into Wasm exports
+//! cosmwasm_vm_version_2, allocate and deallocate turn into Wasm exports
 //! as soon as cosmwasm_std is `use`d in the contract, even privately.
 //!
 //! do_init and do_wrapper should be wrapped with a extern "C" entry point
@@ -26,7 +26,7 @@ extern "C" fn requires_staking() -> () {}
 /// They can be checked by cosmwasm_vm.
 /// Update this whenever the Wasm VM interface breaks.
 #[no_mangle]
-extern "C" fn cosmwasm_vm_version_1() -> () {}
+extern "C" fn cosmwasm_vm_version_2() -> () {}
 
 /// allocate reserves the given number of bytes in wasm memory and returns a pointer
 /// to a Region defining this data. This space is managed by the calling process

packages/std/src/imports.rs

@@ -28,8 +28,8 @@ extern "C" {
     #[cfg(feature = "iterator")]
     fn db_next(iterator_id: u32) -> u32;
 
-    fn canonicalize_address(source: u32, destination: u32) -> i32;
-    fn humanize_address(source: u32, destination: u32) -> i32;
+    fn canonicalize_address(source: u32, destination: u32) -> u32;
+    fn humanize_address(source: u32, destination: u32) -> u32;
 
     /// Executes a query on the chain (import). Not to be confused with the
     /// query export, which queries the state of the contract.
@@ -157,9 +157,13 @@ impl Api for ExternalApi {
         let send_ptr = &*send as *const Region as u32;
         let canon = alloc(CANONICAL_ADDRESS_BUFFER_LENGTH);
 
-        let read = unsafe { canonicalize_address(send_ptr, canon as u32) };
-        if read < 0 {
-            return Err(StdError::generic_err("canonicalize_address returned error"));
+        let result = unsafe { canonicalize_address(send_ptr, canon as u32) };
+        if result != 0 {
+            let error = unsafe { consume_string_region_written_by_vm(result as *mut Region) };
+            return Err(StdError::generic_err(format!(
+                "canonicalize_address errored: {}",
+                error
+            )));
         }
 
         let out = unsafe { consume_region(canon) };
@@ -171,18 +175,28 @@ impl Api for ExternalApi {
         let send_ptr = &*send as *const Region as u32;
         let human = alloc(HUMAN_ADDRESS_BUFFER_LENGTH);
 
-        let read = unsafe { humanize_address(send_ptr, human as u32) };
-        if read < 0 {
-            return Err(StdError::generic_err("humanize_address returned error"));
+        let result = unsafe { humanize_address(send_ptr, human as u32) };
+        if result != 0 {
+            let error = unsafe { consume_string_region_written_by_vm(result as *mut Region) };
+            return Err(StdError::generic_err(format!(
+                "humanize_address errored: {}",
+                error
+            )));
         }
 
-        let out = unsafe { consume_region(human) };
-        // we know input was correct when created, so let's save some bytes
-        let result = unsafe { String::from_utf8_unchecked(out) };
-        Ok(HumanAddr(result))
+        let address = unsafe { consume_string_region_written_by_vm(human) };
+        Ok(address.into())
     }
 }
 
+/// Takes a pointer to a Region and reads the data into a String.
+/// This is for trusted string sources only.
+unsafe fn consume_string_region_written_by_vm(from: *mut Region) -> String {
+    let data = consume_region(from);
+    // We trust the VM/chain to return correct UTF-8, so let's save some gas
+    String::from_utf8_unchecked(data)
+}
+
 /// A stateless convenience wrapper around imports provided by the VM
 pub struct ExternalQuerier {}
 

packages/vm/src/compatability.rs

@@ -25,7 +25,7 @@ static SUPPORTED_IMPORTS: &[&str] = &[
 /// Basically, anything that is used in calls.rs
 /// This is unlikely to change much, must be frozen at 1.0 to avoid breaking existing contracts
 static REQUIRED_EXPORTS: &[&str] = &[
-    "cosmwasm_vm_version_1",
+    "cosmwasm_vm_version_2",
     "query",
     "init",
     "handle",
@@ -176,15 +176,15 @@ mod test {
     fn test_check_wasm_old_contract() {
         match check_wasm(CONTRACT_0_7, &default_features()) {
             Err(VmError::StaticValidationErr { msg, .. }) => assert!(msg.starts_with(
-                "Wasm contract doesn't have required export: \"cosmwasm_vm_version_1\""
+                "Wasm contract doesn't have required export: \"cosmwasm_vm_version_2\""
             )),
             Err(e) => panic!("Unexpected error {:?}", e),
             Ok(_) => panic!("This must not succeeed"),
         };
 
         match check_wasm(CONTRACT_0_6, &default_features()) {
             Err(VmError::StaticValidationErr { msg, .. }) => assert!(msg.starts_with(
-                "Wasm contract doesn't have required export: \"cosmwasm_vm_version_1\""
+                "Wasm contract doesn't have required export: \"cosmwasm_vm_version_2\""
             )),
             Err(e) => panic!("Unexpected error {:?}", e),
             Ok(_) => panic!("This must not succeeed"),
@@ -309,7 +309,7 @@ mod test {
         match check_wasm_exports(&module) {
             Err(VmError::StaticValidationErr { msg, .. }) => {
                 assert!(msg.starts_with(
-                    "Wasm contract doesn't have required export: \"cosmwasm_vm_version_1\""
+                    "Wasm contract doesn't have required export: \"cosmwasm_vm_version_2\""
                 ));
             }
             Err(e) => panic!("Unexpected error {:?}", e),
@@ -323,7 +323,7 @@ mod test {
         match check_wasm_exports(&module) {
             Err(VmError::StaticValidationErr { msg, .. }) => {
                 assert!(msg.starts_with(
-                    "Wasm contract doesn't have required export: \"cosmwasm_vm_version_1\""
+                    "Wasm contract doesn't have required export: \"cosmwasm_vm_version_2\""
                 ));
             }
             Err(e) => panic!("Unexpected error {:?}", e),

packages/vm/src/context.rs

@@ -380,8 +380,8 @@ mod test {
                 "db_scan" => Func::new(|_a: u32, _b: u32, _c: i32| -> u32 { 0 }),
                 "db_next" => Func::new(|_a: u32| -> u32 { 0 }),
                 "query_chain" => Func::new(|_a: u32| -> u32 { 0 }),
-                "canonicalize_address" => Func::new(|_a: u32, _b: u32| -> i32 { 0 }),
-                "humanize_address" => Func::new(|_a: u32, _b: u32| -> i32 { 0 }),
+                "canonicalize_address" => Func::new(|_a: u32, _b: u32| -> u32 { 0 }),
+                "humanize_address" => Func::new(|_a: u32, _b: u32| -> u32 { 0 }),
             },
         };
         let mut instance = Box::from(module.instantiate(&import_obj).unwrap());

packages/vm/src/errors.rs

@@ -269,6 +269,7 @@ impl CommunicationError {
         InvalidOrder { value }.build()
     }
 
+    #[allow(dead_code)]
     pub(crate) fn invalid_utf8<S: ToString>(msg: S) -> Self {
         InvalidUtf8 {
             msg: msg.to_string(),