AccountTrace Serialization was done in the wrong order

framework/contracts/native/ibc-client/Cargo.toml

@@ -37,7 +37,7 @@ thiserror       = { workspace = true }
 workspace-hack = { version = "0.1", path = "../../../workspace-hack" }
 
 [dev-dependencies]
-abstract-interface = { workspace = true, features = ["interchain"] }
+abstract-interface = { workspace = true, features = ["interchain", "testing"] }
 abstract-testing   = { workspace = true }
 cosmwasm-schema    = { workspace = true }
 cw-orch            = { workspace = true }

framework/contracts/native/ibc-client/src/ibc.rs

@@ -8,6 +8,7 @@ use abstract_std::{
         IbcClientCallback,
     },
     objects::TruncatedChainId,
+    ABSTRACT_EVENT_TYPE,
 };
 use cosmwasm_std::{from_json, Attribute, DepsMut, Env, MessageInfo};
 
@@ -64,7 +65,7 @@ pub fn receive_action_callback(
                 let wasm_abstract_attributes: Vec<Attribute> = account_creation_result
                     .events
                     .into_iter()
-                    .filter(|e| e.ty == "wasm-abstract")
+                    .filter(|e| e.ty == ABSTRACT_EVENT_TYPE)
                     .flat_map(|e| e.attributes)
                     .collect();
 

framework/contracts/native/ibc-client/src/queries.rs

@@ -44,7 +44,6 @@ pub fn list_accounts(
                 Ok::<_, StdError>((AccountId::new(seq, trace).unwrap(), chain, address))
             },
         )?;
-
     Ok(ListAccountsResponse { accounts })
 }
 

framework/contracts/native/ibc-client/tests/multihop_accounts.rs

@@ -0,0 +1,106 @@
+use abstract_interface::Abstract;
+use abstract_std::{
+    ibc::polytone_callbacks::CallbackMessage,
+    ibc_client::{ExecuteMsgFns as _, IbcClientCallback, QueryMsgFns},
+    objects::{AccountId, TruncatedChainId},
+    ABSTRACT_EVENT_TYPE,
+};
+use cosmwasm_std::{to_json_binary, StdResult, SubMsgResponse};
+use cw_orch::{core::serde_json, mock::MockBech32, prelude::*, take_storage_snapshot};
+
+type AResult = cw_orch::anyhow::Result<()>; // alias for Result<(), anyhow::Error>
+
+#[test]
+fn multihop_account_snapshot() -> AResult {
+    let chain = MockBech32::new("mock");
+    // Mock note, so it can take execute calls
+    let note_code_id = chain
+        .upload_custom(
+            "note",
+            Box::new(ContractWrapper::new(
+                |_, _, _, _: serde_json::Value| StdResult::Ok(cosmwasm_std::Response::new()),
+                |_, _, _, _: Empty| StdResult::Ok(cosmwasm_std::Response::new()),
+                |_,
+                 _,
+                 _: cosmwasm_std::Empty|
+                 -> Result<cosmwasm_std::Binary, cosmwasm_std::Never> {
+                    unreachable!()
+                },
+            )),
+        )?
+        .uploaded_code_id()?;
+    let note = chain
+        .instantiate(note_code_id, &Empty {}, Some("note"), None, &[])?
+        .instantiated_contract_address()?;
+
+    // Make ibc-client trust our mock note for registering accounts
+    let deployment = Abstract::new(chain.clone());
+    deployment.ibc.client.upload()?;
+    deployment
+        .ibc
+        .client
+        .instantiate(&abstract_std::ibc_client::InstantiateMsg {}, None, &[])?;
+    deployment.ibc.client.register_infrastructure(
+        TruncatedChainId::from_chain_id("remoteone-1"),
+        "host",
+        note.clone(),
+    )?;
+    deployment
+        .ibc
+        .client
+        .call_as(&note)
+        .callback(CallbackMessage {
+            initiator: deployment.ibc.client.address()?,
+            initiator_msg: to_json_binary(&IbcClientCallback::WhoAmI {})?,
+            result: abstract_std::ibc::polytone_callbacks::Callback::Execute(Ok(
+                abstract_std::ibc::polytone_callbacks::ExecutionResponse {
+                    executed_by: "host".to_owned(),
+                    result: vec![],
+                },
+            )),
+        })?;
+
+    let multihop_account_id = AccountId::new(
+        42,
+        abstract_std::objects::AccountTrace::Remote(vec![
+            TruncatedChainId::from_chain_id("remoteone-1"),
+            TruncatedChainId::from_chain_id("remotetwo-1"),
+        ]),
+    )?;
+    // register account
+    deployment
+        .ibc
+        .client
+        .call_as(&note)
+        .callback(CallbackMessage {
+            initiator: deployment.ibc.client.address()?,
+            initiator_msg: to_json_binary(&IbcClientCallback::CreateAccount {
+                account_id: multihop_account_id.clone(),
+            })?,
+            result: abstract_std::ibc::polytone_callbacks::Callback::Execute(Ok(
+                abstract_std::ibc::polytone_callbacks::ExecutionResponse {
+                    executed_by: "host".to_owned(),
+                    #[allow(deprecated)]
+                    result: vec![SubMsgResponse {
+                        events: vec![cosmwasm_std::Event::new(ABSTRACT_EVENT_TYPE)
+                            .add_attribute("account_address", "remote_account")],
+                        data: None,
+                        msg_responses: vec![],
+                    }],
+                },
+            )),
+        })?;
+    let accounts = deployment.ibc.client.list_accounts(None, None)?;
+    // Make sure we have in state exactly what we did put
+    assert_eq!(
+        accounts.accounts,
+        vec![(
+            multihop_account_id,
+            TruncatedChainId::from_chain_id("remoteone-1"),
+            "remote_account".to_owned()
+        )]
+    );
+
+    take_storage_snapshot!(chain, "multihop_account");
+    Ok(())
+}

framework/contracts/native/registry/src/queries.rs

@@ -608,7 +608,7 @@ mod test {
         let yank_msg = ExecuteMsg::YankModule {
             module: module_info,
         };
-        let res = dbg!(execute_as(deps, &abstr.owner, yank_msg));
+        let res = execute_as(deps, &abstr.owner, yank_msg);
         assert!(res.is_ok());
     }
 

framework/packages/abstract-std/src/objects/account/account_id.rs

@@ -362,8 +362,8 @@ mod test {
             assert_eq!(
                 account_id.trace,
                 AccountTrace::Remote(vec![
-                    TruncatedChainId::_from_str("ethereum"),
                     TruncatedChainId::_from_str("bitcoin"),
+                    TruncatedChainId::_from_str("ethereum"),
                 ])
             );
         }

framework/packages/abstract-std/src/objects/account/account_trace.rs

@@ -22,6 +22,7 @@ impl KeyDeserialize for &AccountTrace {
 
     #[inline(always)]
     fn from_vec(value: Vec<u8>) -> StdResult<Self::Output> {
+        let value = value.into_iter().filter(|b| *b > 32).collect();
         Ok(AccountTrace::from_string(String::from_vec(value)?))
     }
 }
@@ -39,6 +40,7 @@ impl<'a> PrimaryKey<'a> for AccountTrace {
                 let len = chain_name.len();
                 chain_name
                     .iter()
+                    .rev()
                     .enumerate()
                     .flat_map(|(s, c)| {
                         if s == len - 1 {
@@ -153,34 +155,11 @@ impl AccountTrace {
     ///
     /// **only use this for deserialization**
     pub(crate) fn from_string(trace: String) -> Self {
-        let acc = if trace == LOCAL {
-            Self::Local
-        } else {
-            Self::Remote(
-                trace
-                    .split(CHAIN_DELIMITER)
-                    .map(TruncatedChainId::_from_str)
-                    .collect(),
-            )
-        };
-        acc
+        account_trace_from_str(&trace)
     }
 
-    /// **No verification is done here**
-    ///
-    /// **only use this for deserialization**
-    #[allow(unused)]
     pub(crate) fn from_str(trace: &str) -> Result<Self, AbstractError> {
-        let acc = if trace == LOCAL {
-            Self::Local
-        } else {
-            Self::Remote(
-                trace
-                    .split(CHAIN_DELIMITER)
-                    .map(TruncatedChainId::_from_str)
-                    .collect(),
-            )
-        };
+        let acc = account_trace_from_str(trace);
         acc.verify()?;
         Ok(acc)
     }
@@ -190,15 +169,21 @@ impl TryFrom<&str> for AccountTrace {
     type Error = AbstractError;
 
     fn try_from(trace: &str) -> Result<Self, Self::Error> {
-        if trace == LOCAL {
-            Ok(Self::Local)
-        } else {
-            let chain_trace: Vec<TruncatedChainId> = trace
-                .split(CHAIN_DELIMITER)
-                .map(|t| TruncatedChainId::from_string(t.to_string()))
-                .collect::<Result<Vec<_>, _>>()?;
-            Ok(Self::Remote(chain_trace))
-        }
+        AccountTrace::from_str(trace)
+    }
+}
+
+fn account_trace_from_str(trace: &str) -> AccountTrace {
+    if trace == LOCAL {
+        AccountTrace::Local
+    } else {
+        let rev_trace: Vec<_> = trace
+            // DoubleEndedSearcher implemented for char, but not for "str"
+            .split(CHAIN_DELIMITER.chars().next().unwrap())
+            .map(TruncatedChainId::_from_str)
+            .rev()
+            .collect();
+        AccountTrace::Remote(rev_trace)
     }
 }
 
@@ -212,6 +197,7 @@ impl Display for AccountTrace {
                 // "juno>terra>osmosis"
                 chain_name
                     .iter()
+                    .rev()
                     .map(|name| name.as_str())
                     .collect::<Vec<&str>>()
                     .join(CHAIN_DELIMITER)
@@ -255,25 +241,29 @@ mod test {
 
         #[coverage_helper::test]
         fn remote_multi_works() {
+            // Here the account originates from ethereum and was then bridged to bitcoin
             let trace = AccountTrace::from_str("bitcoin>ethereum").unwrap();
             assert_eq!(
                 trace,
+                // The trace vector pushes the last chains last
                 AccountTrace::Remote(vec![
+                    TruncatedChainId::from_str("ethereum").unwrap(),
                     TruncatedChainId::from_str("bitcoin").unwrap(),
-                    TruncatedChainId::from_str("ethereum").unwrap()
                 ])
             );
         }
 
         #[coverage_helper::test]
         fn remote_multi_multi_works() {
+            // Here the account originates from cosmos, and was then bridged to ethereum and was then bridged to bitcoin
             let trace = AccountTrace::from_str("bitcoin>ethereum>cosmos").unwrap();
             assert_eq!(
                 trace,
+                // The trace vector pushes the last chains last
                 AccountTrace::Remote(vec![
-                    TruncatedChainId::from_str("bitcoin").unwrap(),
-                    TruncatedChainId::from_str("ethereum").unwrap(),
                     TruncatedChainId::from_str("cosmos").unwrap(),
+                    TruncatedChainId::from_str("ethereum").unwrap(),
+                    TruncatedChainId::from_str("bitcoin").unwrap(),
                 ])
             );
         }
@@ -312,29 +302,46 @@ mod test {
             AccountTrace::Remote(vec![TruncatedChainId::from_str("bitcoin").unwrap()])
         }
 
+        fn mock_multi_hop_key() -> AccountTrace {
+            AccountTrace::Remote(vec![
+                TruncatedChainId::from_str("bitcoin").unwrap(),
+                TruncatedChainId::from_str("atom").unwrap(),
+                TruncatedChainId::from_str("foo").unwrap(),
+            ])
+        }
+
         #[coverage_helper::test]
         fn storage_key_works() {
             let mut deps = mock_dependencies();
             let key = mock_key();
+            let multihop_key = mock_multi_hop_key();
             let map: Map<&AccountTrace, u64> = Map::new("map");
 
             map.save(deps.as_mut().storage, &key, &42069).unwrap();
+            map.save(deps.as_mut().storage, &multihop_key, &69420)
+                .unwrap();
 
             assert_eq!(map.load(deps.as_ref().storage, &key).unwrap(), 42069);
+            assert_eq!(
+                map.load(deps.as_ref().storage, &multihop_key).unwrap(),
+                69420
+            );
 
             let items = map
                 .range(deps.as_ref().storage, None, None, Order::Ascending)
                 .map(|item| item.unwrap())
                 .collect::<Vec<_>>();
 
-            assert_eq!(items.len(), 1);
-            assert_eq!(items[0], (key, 42069));
+            assert_eq!(items.len(), 2);
+            assert_eq!(items[0], (multihop_key, 69420));
+            assert_eq!(items[1], (key, 42069));
         }
 
         #[coverage_helper::test]
         fn composite_key_works() {
             let mut deps = mock_dependencies();
             let key = mock_key();
+            let multihop_key = mock_multi_hop_key();
             let map: Map<(&AccountTrace, Addr), u64> = Map::new("map");
 
             map.save(
@@ -343,6 +350,12 @@ mod test {
                 &42069,
             )
             .unwrap();
+            map.save(
+                deps.as_mut().storage,
+                (&multihop_key, Addr::unchecked("larry")),
+                &42069,
+            )
+            .unwrap();
 
             map.save(
                 deps.as_mut().storage,
@@ -357,7 +370,7 @@ mod test {
                 .map(|item| item.unwrap())
                 .collect::<Vec<_>>();
 
-            assert_eq!(items.len(), 2);
+            assert_eq!(items.len(), 3);
             assert_eq!(items[0], (Addr::unchecked("jake"), 69420));
             assert_eq!(items[1], (Addr::unchecked("larry"), 42069));
         }