solana-ibc: refactor module structure (#103)

Move trie_key inside of storage module and add storage::ids module with
ClientIndex type and other identifier related code.  The new module will
house other code related to converting IBC identifiers into compact
internal ones.

solana/solana-ibc/programs/solana-ibc/src/execution_context.rs

@@ -24,8 +24,8 @@ use lib::hash::CryptoHash;
 
 use crate::client_state::AnyClientState;
 use crate::consensus_state::AnyConsensusState;
+use crate::storage::trie_key::TrieKey;
 use crate::storage::{self, IbcStorage};
-use crate::trie_key::TrieKey;
 
 type Result<T = (), E = ibc::core::ContextError> = core::result::Result<T, E>;
 
@@ -339,7 +339,7 @@ impl ExecutionContext for IbcStorage<'_, '_> {
 impl storage::IbcStorageInner<'_, '_> {
     fn store_next_sequence(
         &mut self,
-        path: crate::trie_key::SequencePath<'_>,
+        path: storage::trie_key::SequencePath<'_>,
         index: storage::SequenceTripleIdx,
         seq: Sequence,
     ) -> Result {

solana/solana-ibc/programs/solana-ibc/src/lib.rs

@@ -16,9 +16,6 @@ const PACKET_SEED: &[u8] = b"packet";
 const SOLANA_IBC_STORAGE_SEED: &[u8] = b"private";
 const TRIE_SEED: &[u8] = b"trie";
 
-const CONNECTION_ID_PREFIX: &str = "connection-";
-const CHANNEL_ID_PREFIX: &str = "channel-";
-
 declare_id!("EnfDJsAK7BGgetnmKzBx86CsgC5kfSPcsktFCQ4YLC81");
 
 mod chain;
@@ -33,7 +30,6 @@ mod storage;
 #[cfg(test)]
 mod tests;
 mod transfer;
-mod trie_key;
 mod validation_context;
 // mod client_context;
 

solana/solana-ibc/programs/solana-ibc/src/storage.rs

@@ -21,6 +21,9 @@ mod ibc {
     pub use ibc::Height;
 }
 
+pub(crate) mod ids;
+pub(crate) mod trie_key;
+
 pub(crate) type SolanaTimestamp = u64;
 pub(crate) type InnerConnectionId = String;
 pub(crate) type InnerPortId = String;
@@ -79,47 +82,6 @@ impl SequenceTriple {
     }
 }
 
-/// An index used as unique identifier for a client.
-///
-/// IBC client id uses `<client-type>-<counter>` format.  This index is
-/// constructed from a client id by stripping the client type.  Since counter is
-/// unique within an IBC module, the index is enough to identify a known client.
-///
-/// To avoid confusing identifiers with the same counter but different client
-/// type (which may be crafted by an attacker), we always check that client type
-/// matches one we know.  Because of this check, to get `ClientIdx`
-/// [`PrivateStorage::client`] needs to be used.
-///
-/// The index is guaranteed to fit `u32` and `usize`.
-#[derive(Clone, Copy, PartialEq, Eq, derive_more::From, derive_more::Into)]
-pub struct ClientIdx(u32);
-
-impl From<ClientIdx> for usize {
-    #[inline]
-    fn from(index: ClientIdx) -> usize { index.0 as usize }
-}
-
-impl core::str::FromStr for ClientIdx {
-    type Err = core::num::ParseIntError;
-
-    #[inline]
-    fn from_str(value: &str) -> Result<Self, Self::Err> {
-        if core::mem::size_of::<usize>() < 4 {
-            usize::from_str(value).map(|index| Self(index as u32))
-        } else {
-            u32::from_str(value).map(Self)
-        }
-    }
-}
-
-impl PartialEq<usize> for ClientIdx {
-    #[inline]
-    fn eq(&self, rhs: &usize) -> bool {
-        u32::try_from(*rhs).ok().filter(|rhs| self.0 == *rhs).is_some()
-    }
-}
-
-
 /// Per-client private storage.
 #[derive(Clone, Debug, borsh::BorshSerialize, borsh::BorshDeserialize)]
 pub(crate) struct ClientStore {
@@ -192,7 +154,7 @@ impl PrivateStorage {
     pub fn client(
         &self,
         client_id: &ibc::ClientId,
-    ) -> Result<(ClientIdx, &ClientStore), ibc::ClientError> {
+    ) -> Result<(ids::ClientIdx, &ClientStore), ibc::ClientError> {
         self.client_index(client_id)
             .and_then(|idx| {
                 self.clients
@@ -218,7 +180,7 @@ impl PrivateStorage {
         &mut self,
         client_id: &ibc::ClientId,
         create: bool,
-    ) -> Result<(ClientIdx, &mut ClientStore), ibc::ClientError> {
+    ) -> Result<(ids::ClientIdx, &mut ClientStore), ibc::ClientError> {
         self.client_mut_impl(client_id, create).ok_or_else(|| {
             ibc::ClientError::ClientStateNotFound {
                 client_id: client_id.clone(),
@@ -230,7 +192,7 @@ impl PrivateStorage {
         &mut self,
         client_id: &ibc::ClientId,
         create: bool,
-    ) -> Option<(ClientIdx, &mut ClientStore)> {
+    ) -> Option<(ids::ClientIdx, &mut ClientStore)> {
         use core::cmp::Ordering;
 
         let idx = self.client_index(client_id)?;
@@ -245,7 +207,10 @@ impl PrivateStorage {
         }
     }
 
-    fn client_index(&self, client_id: &ibc::ClientId) -> Option<ClientIdx> {
+    fn client_index(
+        &self,
+        client_id: &ibc::ClientId,
+    ) -> Option<ids::ClientIdx> {
         client_id
             .as_str()
             .rsplit_once('-')

solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs

@@ -0,0 +1,53 @@
+/// Prefix of IBC connection ids.
+///
+/// Note: We’re not using ConnectionId::prefix() because it returns the prefix
+/// without trailing `-` which we want included to simplify stripping of the
+/// prefix.
+pub(super) const CONNECTION_ID_PREFIX: &str = "connection-";
+
+/// Prefix of IBC channel ids.
+///
+/// Note: We’re not using ChannelId::prefix() because it returns the prefix
+/// without trailing `-` which we want included to simplify stripping of the
+/// prefix.
+pub(super) const CHANNEL_ID_PREFIX: &str = "channel-";
+
+/// An index used as unique identifier for a client.
+///
+/// IBC client id uses `<client-type>-<counter>` format.  This index is
+/// constructed from a client id by stripping the client type.  Since counter is
+/// unique within an IBC module, the index is enough to identify a known client.
+///
+/// To avoid confusing identifiers with the same counter but different client
+/// type (which may be crafted by an attacker), we always check that client type
+/// matches one we know.  Because of this check, to get `ClientIdx`
+/// [`PrivateStorage::client`] needs to be used.
+///
+/// The index is guaranteed to fit `u32` and `usize`.
+#[derive(Clone, Copy, PartialEq, Eq, derive_more::From, derive_more::Into)]
+pub struct ClientIdx(u32);
+
+impl From<ClientIdx> for usize {
+    #[inline]
+    fn from(index: ClientIdx) -> usize { index.0 as usize }
+}
+
+impl core::str::FromStr for ClientIdx {
+    type Err = core::num::ParseIntError;
+
+    #[inline]
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        if core::mem::size_of::<usize>() < 4 {
+            usize::from_str(value).map(|index| Self(index as u32))
+        } else {
+            u32::from_str(value).map(Self)
+        }
+    }
+}
+
+impl PartialEq<usize> for ClientIdx {
+    #[inline]
+    fn eq(&self, rhs: &usize) -> bool {
+        u32::try_from(*rhs).ok().filter(|rhs| self.0 == *rhs).is_some()
+    }
+}

solana/solana-ibc/programs/solana-ibc/src/trie_key.rs → solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs

@@ -5,11 +5,7 @@ use ibc::core::ics24_host::path::{
     SeqAckPath, SeqRecvPath, SeqSendPath,
 };
 
-// Note: We’re not using ChannelId::prefix() and ConnectionId::prefix() because
-// those return the prefix without trailing `-` and we want constants which also
-// include that hyphen.
-use super::{CHANNEL_ID_PREFIX, CONNECTION_ID_PREFIX};
-use crate::storage::ClientIdx;
+use crate::storage::ids::{ClientIdx, CHANNEL_ID_PREFIX, CONNECTION_ID_PREFIX};
 
 /// A key used for indexing entries in the provable storage.
 ///

solana/solana-ibc/programs/solana-ibc/src/validation_context.rs

@@ -24,8 +24,8 @@ use lib::hash::CryptoHash;
 
 use crate::client_state::AnyClientState;
 use crate::consensus_state::AnyConsensusState;
-use crate::storage::IbcStorage;
-use crate::trie_key::TrieKey;
+use crate::storage::trie_key::TrieKey;
+use crate::storage::{self, IbcStorage};
 
 type Result<T = (), E = ContextError> = core::result::Result<T, E>;
 
@@ -145,42 +145,33 @@ impl ValidationContext for IbcStorage<'_, '_> {
     }
 
     fn get_next_sequence_send(&self, path: &SeqSendPath) -> Result<Sequence> {
-        self.get_next_sequence(
-            path.into(),
-            crate::storage::SequenceTripleIdx::Send,
-        )
-        .map_err(|(port_id, channel_id)| {
-            ContextError::PacketError(PacketError::MissingNextSendSeq {
-                port_id,
-                channel_id,
+        self.get_next_sequence(path.into(), storage::SequenceTripleIdx::Send)
+            .map_err(|(port_id, channel_id)| {
+                ContextError::PacketError(PacketError::MissingNextSendSeq {
+                    port_id,
+                    channel_id,
+                })
             })
-        })
     }
 
     fn get_next_sequence_recv(&self, path: &SeqRecvPath) -> Result<Sequence> {
-        self.get_next_sequence(
-            path.into(),
-            crate::storage::SequenceTripleIdx::Recv,
-        )
-        .map_err(|(port_id, channel_id)| {
-            ContextError::PacketError(PacketError::MissingNextRecvSeq {
-                port_id,
-                channel_id,
+        self.get_next_sequence(path.into(), storage::SequenceTripleIdx::Recv)
+            .map_err(|(port_id, channel_id)| {
+                ContextError::PacketError(PacketError::MissingNextRecvSeq {
+                    port_id,
+                    channel_id,
+                })
             })
-        })
     }
 
     fn get_next_sequence_ack(&self, path: &SeqAckPath) -> Result<Sequence> {
-        self.get_next_sequence(
-            path.into(),
-            crate::storage::SequenceTripleIdx::Ack,
-        )
-        .map_err(|(port_id, channel_id)| {
-            ContextError::PacketError(PacketError::MissingNextAckSeq {
-                port_id,
-                channel_id,
+        self.get_next_sequence(path.into(), storage::SequenceTripleIdx::Ack)
+            .map_err(|(port_id, channel_id)| {
+                ContextError::PacketError(PacketError::MissingNextAckSeq {
+                    port_id,
+                    channel_id,
+                })
             })
-        })
     }
 
     fn get_packet_commitment(
@@ -321,8 +312,8 @@ impl ibc::core::ics02_client::ClientValidationContext for IbcStorage<'_, '_> {
 impl IbcStorage<'_, '_> {
     fn get_next_sequence(
         &self,
-        path: crate::trie_key::SequencePath<'_>,
-        index: crate::storage::SequenceTripleIdx,
+        path: crate::storage::trie_key::SequencePath<'_>,
+        index: storage::SequenceTripleIdx,
     ) -> core::result::Result<
         Sequence,
         (