Repository: stellar/rs-soroban-env
Pull request #1094 (2023-10-05 15:54:20)

Replace address bytes conversions with strkey conversions.
### What

Replace address bytes conversions with strkey conversions.

### Why

The existing conversions are pretty confusing due to account vs contract address dichotomy. Strkey is more natural format for cross-chain interaction, that is much less error-prone (at a minor performance cost).

More motivation in https://github.com/stellar/rs-soroban-sdk/issues/996

### Known limitations

I'm not 100% sure about metering here - I do charge the allocations, but there is also a couple for-loops (base32 encoding/decoding and CRC computation). These should be very cheap though.



Patch:
diff --git a/soroban-env-common/env.json b/soroban-env-common/env.json
index b69d417e..4f7f189d 100644
--- a/soroban-env-common/env.json
+++ b/soroban-env-common/env.json
@@ -1996,54 +1996,30 @@
                 },
                 {
                     "export": "1",
-                    "name": "account_public_key_to_address",
+                    "name": "strkey_to_address",
                     "args": [
                         {
-                            "name": "pk_bytes",
-                            "type": "BytesObject"
+                            "name": "strkey",
+                            "type": "Val"
                         }
                     ],
                     "return": "AddressObject",
-                    "docs": "Converts a provided 32-byte Stellar account public key to the corresponding address. This is only useful in the context of cross-chain interoperability. Prefer directly using the Address objects whenever possible."
+                    "docs": "Converts a provided Stellar strkey address of an account or a contract ('G...' or 'C...' respectively) to an address object. `strkey` can be either `BytesObject` or `StringObject` (the contents should represent the `G.../C...` string in both cases). Any other valid or invalid strkey (e.g. 'S...') will trigger an error. Prefer directly using the Address objects whenever possible. This is only useful in the context of custom messaging protocols (e.g. cross-chain)."
                 },
                 {
                     "export": "2",
-                    "name": "contract_id_to_address",
-                    "args": [
-                        {
-                            "name": "contract_id_bytes",
-                            "type": "BytesObject"
-                        }
-                    ],
-                    "return": "AddressObject",
-                    "docs": "Converts a provided 32-byte contract identifier to a corresponding Address object."
-                },
-                {
-                    "export": "3",
-                    "name": "address_to_account_public_key",
+                    "name": "address_to_strkey",
                     "args": [
                         {
                             "name": "address",
                             "type": "AddressObject"
                         }
                     ],
-                    "return": "Val",
-                    "docs": "Returns the 32-byte public key of the Stellar account corresponding to the provided Address object. If the Address doesn't belong to an account, returns Val corresponding to the unit type (`()`)."
-                },
-                {
-                    "export": "4",
-                    "name": "address_to_contract_id",
-                    "args": [
-                        {
-                            "name": "address",
-                            "type": "AddressObject"
-                        }
-                    ],
-                    "return": "Val",
-                    "docs": "Returns the 32-byte contract identifier corresponding to the provided Address object. If the Address doesn't belong to an account, returns Val corresponding to the unit type (`()`)."
+                    "return": "StringObject",
+                    "docs": "Converts a provided address to Stellar strkey format ('G...' for account or 'C...' for contract). Prefer directly using the Address objects whenever possible. This is only useful in the context of custom messaging protocols (e.g. cross-chain)."
                 },
                 {
-                    "export": "5",
+                    "export": "3",
                     "name": "authorize_as_curr_contract",
                     "args": [
                         {
diff --git a/soroban-env-common/src/meta.rs b/soroban-env-common/src/meta.rs
index 010734cd..ebb86657 100644
--- a/soroban-env-common/src/meta.rs
+++ b/soroban-env-common/src/meta.rs
@@ -58,7 +58,7 @@ soroban_env_macros::generate_env_meta_consts!(
 #[cfg(not(feature = "next"))]
 soroban_env_macros::generate_env_meta_consts!(
     ledger_protocol_version: 20,
-    pre_release_version: 64,
+    pre_release_version: 65,
 );
 
 pub const fn get_ledger_protocol_version(interface_version: u64) -> u32 {
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index 9ac7abad..b0747e02 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -44,7 +44,9 @@ mod prng;
 pub use prng::{Seed, SEED_BYTES};
 mod validity;
 pub use error::HostError;
-use soroban_env_common::xdr::{ContractIdPreimage, ContractIdPreimageFromAddress, ScErrorCode};
+use soroban_env_common::xdr::{
+    ContractIdPreimage, ContractIdPreimageFromAddress, ScErrorCode, Uint256,
+};
 
 use self::{
     frame::{Context, ContractReentryMode},
@@ -2451,50 +2453,102 @@ impl VmCallerEnv for Host {
             .into())
     }
 
-    fn account_public_key_to_address(
+    fn address_to_strkey(
         &self,
         _vmcaller: &mut VmCaller<Self::VmUserState>,
-        pk_bytes: BytesObject,
-    ) -> Result<AddressObject, Self::Error> {
-        let account_id = self.account_id_from_bytesobj(pk_bytes)?;
-        self.add_host_object(ScAddress::Account(account_id))
+        address: AddressObject,
+    ) -> Result<StringObject, Self::Error> {
+        let strkey = self.visit_obj(address, |addr: &ScAddress| {
+            // Approximate the strkey encoding cost with two vector allocations:
+            // one for the payload size (32-byte key/hash + 3 bytes for
+            // version/checksum) and  another one for the base32 encoding of
+            // the payload.
+            const PAYLOAD_LEN: u64 = 32 + 3;
+            Vec::<u8>::charge_bulk_init_cpy(PAYLOAD_LEN + (PAYLOAD_LEN * 8 + 4) / 5, self)?;
+            let strkey = match addr {
+                ScAddress::Account(acc_id) => {
+                    let AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(ed25519))) = acc_id;
+                    let strkey = stellar_strkey::Strkey::PublicKeyEd25519(
+                        stellar_strkey::ed25519::PublicKey(ed25519.metered_clone(self)?),
+                    );
+                    strkey
+                }
+                ScAddress::Contract(Hash(h)) => stellar_strkey::Strkey::Contract(
+                    stellar_strkey::Contract(h.metered_clone(self)?),
+                ),
+            };
+            Ok(strkey.to_string())
+        })?;
+        self.add_host_object(ScString(strkey.try_into()?))
     }
 
-    fn contract_id_to_address(
+    fn strkey_to_address(
         &self,
         _vmcaller: &mut VmCaller<Self::VmUserState>,
-        contract_id_bytes: BytesObject,
+        strkey_obj: Val,
     ) -> Result<AddressObject, Self::Error> {
-        let contract_id = self.hash_from_bytesobj_input("contract_id", contract_id_bytes)?;
-        self.add_host_object(ScAddress::Contract(contract_id))
-    }
-
-    fn address_to_account_public_key(
-        &self,
-        _vmcaller: &mut VmCaller<Self::VmUserState>,
-        address: AddressObject,
-    ) -> Result<Val, Self::Error> {
-        let addr = self.visit_obj(address, |addr: &ScAddress| addr.metered_clone(self))?;
-        match addr {
-            ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(pk))) => Ok(self
-                .add_host_object(ScBytes(self.metered_slice_to_vec(&pk.0)?.try_into()?))?
-                .into()),
-            ScAddress::Contract(_) => Ok(().into()),
-        }
-    }
+        let strkey_obj = Object::try_from(strkey_obj).map_err(|_| {
+            self.err(
+                ScErrorType::Value,
+                ScErrorCode::UnexpectedType,
+                "strkey is not an object",
+                &[strkey_obj],
+            )
+        })?;
+        let sc_addr = self.visit_obj_untyped(strkey_obj, |key_obj: &HostObject| {
+            let key = match key_obj {
+                HostObject::Bytes(b) => b.as_slice(),
+                HostObject::String(s) => s.as_slice(),
+                _ => {
+                    return Err(self.err(
+                        ScErrorType::Value,
+                        ScErrorCode::UnexpectedType,
+                        "strkey is not a string or bytes object",
+                        &[strkey_obj.to_val()],
+                    ));
+                }
+            };
+            const PAYLOAD_LEN: u64 = 32 + 3;
+            let expected_key_len = (PAYLOAD_LEN * 8 + 4) / 5;
+            if expected_key_len != key.len() as u64 {
+                return Err(self.err(
+                    ScErrorType::Value,
+                    ScErrorCode::InvalidInput,
+                    "unexpected strkey length",
+                    &[strkey_obj.to_val()],
+                ));
+            }
+            // Charge for the key copy to string.
+            Vec::<u8>::charge_bulk_init_cpy(key.len() as u64, self)?;
+            let key_str = String::from_utf8_lossy(key);
+            // Approximate the decoding cost as two vector allocations for the
+            // expected payload length (the strkey library does one extra copy).
+            Vec::<u8>::charge_bulk_init_cpy(PAYLOAD_LEN + PAYLOAD_LEN, self)?;
+            let strkey = stellar_strkey::Strkey::from_string(&key_str).map_err(|_| {
+                self.err(
+                    ScErrorType::Value,
+                    ScErrorCode::InvalidInput,
+                    "couldn't process the string as strkey",
+                    &[strkey_obj.to_val()],
+                )
+            })?;
+            match strkey {
+                stellar_strkey::Strkey::PublicKeyEd25519(pk) => Ok(ScAddress::Account(AccountId(
+                    PublicKey::PublicKeyTypeEd25519(Uint256(pk.0)),
+                ))),
 
-    fn address_to_contract_id(
-        &self,
-        _vmcaller: &mut VmCaller<Self::VmUserState>,
-        address: AddressObject,
-    ) -> Result<Val, Self::Error> {
-        let addr = self.visit_obj(address, |addr: &ScAddress| addr.metered_clone(self))?;
-        match addr {
-            ScAddress::Account(_) => Ok(().into()),
-            ScAddress::Contract(Hash(h)) => Ok(self
-                .add_host_object(ScBytes(self.metered_slice_to_vec(&h)?.try_into()?))?
-                .into()),
-        }
+                stellar_strkey::Strkey::Contract(c) => Ok(ScAddress::Contract(Hash(c.0))),
+                _ => {
+                    return Err(self.err(
+                        ScErrorType::Value,
+                        ScErrorCode::InvalidInput,
+                        "incorrect strkey type",
+                        &[strkey_obj.to_val()],
+                    ));
+                }
+            }
+        })?;
+        self.add_host_object(sc_addr)
     }
 
     // endregion "address" module functions


Test patch:
diff --git a/soroban-env-host/src/test/address.rs b/soroban-env-host/src/test/address.rs
index cd2f9cb2..14b03821 100644
--- a/soroban-env-host/src/test/address.rs
+++ b/soroban-env-host/src/test/address.rs
@@ -1,73 +1,170 @@
 use crate::Host;
 use soroban_env_common::{
-    xdr::{AccountId, Hash, PublicKey, ScAddress, ScBytes, Uint256},
-    Env, TryIntoVal,
+    xdr::{AccountId, Hash, PublicKey, ScAddress, ScBytes, ScString, Uint256},
+    Compare, Env, StringObject, Val,
 };
 
+fn extract_string(host: &Host, s: StringObject) -> String {
+    host.visit_obj(s, |st: &ScString| Ok(st.0.to_string_lossy()))
+        .unwrap()
+}
+
+fn string_to_object(host: &Host, s: &str) -> Val {
+    host.add_host_object(ScString(s.try_into().unwrap()))
+        .unwrap()
+        .to_val()
+}
+
+fn string_to_bytes_object(host: &Host, s: &str) -> Val {
+    host.add_host_object(ScBytes(s.try_into().unwrap()))
+        .unwrap()
+        .to_val()
+}
+
+// Example values are taken from
+// https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0023.md
+
 #[test]
 fn test_account_address_conversions() {
     let host = Host::default();
-    let account_pk = [5_u8; 32];
-    let account_pk_obj = host
-        .add_host_object(ScBytes(account_pk.try_into().unwrap()))
+    let account_pk = [
+        0x3f, 0x0c, 0x34, 0xbf, 0x93, 0xad, 0x0d, 0x99, 0x71, 0xd0, 0x4c, 0xcc, 0x90, 0xf7, 0x05,
+        0x51, 0x1c, 0x83, 0x8a, 0xad, 0x97, 0x34, 0xa4, 0xa2, 0xfb, 0x0d, 0x7a, 0x03, 0xfc, 0x7f,
+        0xe8, 0x9a,
+    ];
+    let address_obj = host
+        .add_host_object(ScAddress::Account(AccountId(
+            PublicKey::PublicKeyTypeEd25519(Uint256(account_pk)),
+        )))
         .unwrap();
-    let address_obj = host.account_public_key_to_address(account_pk_obj).unwrap();
+
+    let strkey = host.address_to_strkey(address_obj).unwrap();
     assert_eq!(
-        host.visit_obj(address_obj, |addr: &ScAddress| { Ok(addr.clone()) })
-            .unwrap(),
-        ScAddress::Account(AccountId(PublicKey::PublicKeyTypeEd25519(Uint256(
-            account_pk
-        ))))
+        extract_string(&host, strkey),
+        "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ"
     );
-    let restored_pk_obj = host
-        .address_to_account_public_key(address_obj)
+
+    let converted_address_object = host.strkey_to_address(strkey.to_val()).unwrap();
+    assert!(host
+        .compare(&address_obj, &converted_address_object)
         .unwrap()
-        .try_into_val(&host)
+        .is_eq());
+    let converted_from_bytes_obj = host
+        .strkey_to_address(string_to_bytes_object(
+            &host,
+            "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZ",
+        ))
         .unwrap();
-    assert_eq!(
-        host.visit_obj(restored_pk_obj, |pk: &ScBytes| Ok(pk.to_vec()))
-            .unwrap(),
-        account_pk.to_vec()
-    );
-    // Verify that the trying to get the contract id returns the unit type.
-    // As Vals aren't comparable, we use `try_into_val` to do the verification
-    // instead.
-    let _: () = host
-        .address_to_contract_id(address_obj)
+    assert!(host
+        .compare(&address_obj, &converted_from_bytes_obj)
         .unwrap()
-        .try_into_val(&host)
-        .unwrap();
+        .is_eq());
 }
 
 #[test]
 fn test_contract_address_conversions() {
     let host = Host::default();
-    let contract_id = [222_u8; 32];
-    let contract_id_obj = host
-        .add_host_object(ScBytes(contract_id.try_into().unwrap()))
+    let contract_id = [
+        0x3f, 0x0c, 0x34, 0xbf, 0x93, 0xad, 0x0d, 0x99, 0x71, 0xd0, 0x4c, 0xcc, 0x90, 0xf7, 0x05,
+        0x51, 0x1c, 0x83, 0x8a, 0xad, 0x97, 0x34, 0xa4, 0xa2, 0xfb, 0x0d, 0x7a, 0x03, 0xfc, 0x7f,
+        0xe8, 0x9a,
+    ];
+    let address_obj = host
+        .add_host_object(ScAddress::Contract(Hash(contract_id)))
         .unwrap();
-    let address_obj = host.contract_id_to_address(contract_id_obj).unwrap();
+
+    let strkey = host.address_to_strkey(address_obj).unwrap();
     assert_eq!(
-        host.visit_obj(address_obj, |addr: &ScAddress| { Ok(addr.clone()) })
-            .unwrap(),
-        ScAddress::Contract(Hash(contract_id))
+        extract_string(&host, strkey),
+        "CA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUWDA"
     );
-    let restored_contract_id_obj = host
-        .address_to_contract_id(address_obj)
+
+    let converted_address_object = host.strkey_to_address(strkey.to_val()).unwrap();
+    assert!(host
+        .compare(&address_obj, &converted_address_object)
         .unwrap()
-        .try_into_val(&host)
+        .is_eq());
+
+    let converted_from_bytes_obj = host
+        .strkey_to_address(string_to_bytes_object(
+            &host,
+            "CA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUWDA",
+        ))
         .unwrap();
-    assert_eq!(
-        host.visit_obj(restored_contract_id_obj, |pk: &ScBytes| Ok(pk.to_vec()))
-            .unwrap(),
-        contract_id.to_vec()
-    );
-    // Verify that the trying to get the account key returns the unit type.
-    // As Vals aren't comparable, we use `try_into_val` to do the verification
-    // instead.
-    let _: () = host
-        .address_to_account_public_key(address_obj)
+    assert!(host
+        .compare(&address_obj, &converted_from_bytes_obj)
         .unwrap()
-        .try_into_val(&host)
-        .unwrap();
+        .is_eq());
+}
+
+#[test]
+fn invalid_strkey_to_address_conversion() {
+    let host = Host::default();
+    assert!(host.strkey_to_address(string_to_object(&host, "")).is_err());
+    assert!(host
+        .strkey_to_address(string_to_object(&host, "GAAAAAAAACGC6"))
+        .is_err());
+    // one symbol short
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSG"
+        ))
+        .is_err());
+    // one symbol long
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZZ"
+        ))
+        .is_err());
+    // Invalid bit length (congruent to 1 mod 8)
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZA"
+        ))
+        .is_err());
+    // Incorrect version
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "DA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUWDA"
+        ))
+        .is_err());
+    // Valid multiplexed account
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAAAAAAAACJUQ"
+        ))
+        .is_err());
+    // Valid private key
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "SBU2RRGLXH3E5CQHTD3ODLDF2BWDCYUSSBLLZ5GNW7JXHDIYKXZWHOKR"
+        ))
+        .is_err());
+    // Valid pre auth tx
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "TBU2RRGLXH3E5CQHTD3ODLDF2BWDCYUSSBLLZ5GNW7JXHDIYKXZWHXL7"
+        ))
+        .is_err());
+    // Valid hash x
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "XBU2RRGLXH3E5CQHTD3ODLDF2BWDCYUSSBLLZ5GNW7JXHDIYKXZWGTOG"
+        ))
+        .is_err());
+    // Valid signed payload
+    assert!(host
+        .strkey_to_address(string_to_object(
+            &host,
+            "PA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAQACAQDAQCQMBYIBEFAWDANBYHRAEISCMKBKFQXDAMRUGY4DUPB6IBZGM"
+        ))
+        .is_err());
 }