solana-ibc: introduce ConnectionIdx type for connection identification (

#105)

IBC connections identifiers are in the form `connection-<number>` and
use sequential numbering.  Take advantage of that by introducing
ConnectionIdx type which only holds the number and storing connection
information in a vector rather than a map.  This is analogous to
change already made for clients.

Issue: #35

common/sealable-trie/src/trie/tests.rs

@@ -13,7 +13,7 @@ fn make_trie_impl<'a>(
     want: Option<(&str, usize)>,
 ) -> TestTrie {
     let keys = keys.into_iter();
-    let count = keys.size_hint().1.unwrap_or(1000).saturating_mul(4);
+    let count = keys.size_hint().1.unwrap_or(1000).saturating_mul(4).max(100);
     let mut trie = TestTrie::new(count);
     for key in keys {
         set(&mut trie, key)

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

@@ -6,6 +6,7 @@ use ibc::core::events::IbcEvent;
 use ibc::core::ics02_client::error::ClientError;
 use ibc::core::ics02_client::ClientExecutionContext;
 use ibc::core::ics03_connection::connection::ConnectionEnd;
+use ibc::core::ics03_connection::error::ConnectionError;
 use ibc::core::ics04_channel::channel::ChannelEnd;
 use ibc::core::ics04_channel::commitment::{
     AcknowledgementCommitment, PacketCommitment,
@@ -25,7 +26,7 @@ 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::storage::{self, ids, IbcStorage};
 
 type Result<T = (), E = ibc::core::ContextError> = core::result::Result<T, E>;
 
@@ -144,13 +145,34 @@ impl ExecutionContext for IbcStorage<'_, '_> {
         path: &ConnectionPath,
         connection_end: ConnectionEnd,
     ) -> Result {
+        use core::cmp::Ordering;
+
         msg!("store_connection({}, {:?})", path, connection_end);
-        self.borrow_mut().store_serialised_proof(
-            |private| &mut private.connections,
-            path.0.to_string(),
-            &TrieKey::from(path),
-            &connection_end,
-        )
+        let connection = ids::ConnectionIdx::try_from(&path.0)?;
+        let serialised = storage::Serialised::new(&connection_end)?;
+        let hash = serialised.digest();
+
+        let mut store = self.borrow_mut();
+
+        let connections = &mut store.private.connections;
+        let index = usize::from(connection);
+        match index.cmp(&connections.len()) {
+            Ordering::Less => connections[index] = serialised,
+            Ordering::Equal => connections.push(serialised),
+            Ordering::Greater => {
+                return Err(ConnectionError::ConnectionNotFound {
+                    connection_id: path.0.clone(),
+                }
+                .into())
+            }
+        }
+
+        store
+            .provable
+            .set(&TrieKey::for_connection(connection), &hash)
+            .map_err(error)?;
+
+        Ok(())
     }
 
     fn store_connection_to_client(
@@ -159,21 +181,13 @@ impl ExecutionContext for IbcStorage<'_, '_> {
         conn_id: ConnectionId,
     ) -> Result {
         msg!("store_connection_to_client({}, {:?})", path, conn_id);
+        let conn_id = ids::ConnectionIdx::try_from(&conn_id)?;
         self.borrow_mut().private.client_mut(&path.0, false)?.connection_id =
             Some(conn_id);
         Ok(())
     }
 
-    fn increase_connection_counter(&mut self) -> Result {
-        let mut store = self.borrow_mut();
-        store.private.connection_counter =
-            store.private.connection_counter.checked_add(1).unwrap();
-        msg!(
-            "connection_counter has increased to: {}",
-            store.private.connection_counter
-        );
-        Ok(())
-    }
+    fn increase_connection_counter(&mut self) -> Result { Ok(()) }
 
     fn store_packet_commitment(
         &mut self,

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

@@ -14,6 +14,7 @@ use crate::consensus_state::AnyConsensusState;
 mod ibc {
     pub use ibc::core::ics02_client::error::ClientError;
     pub use ibc::core::ics03_connection::connection::ConnectionEnd;
+    pub use ibc::core::ics03_connection::error::ConnectionError;
     pub use ibc::core::ics04_channel::channel::ChannelEnd;
     pub use ibc::core::ics04_channel::msgs::PacketMsg;
     pub use ibc::core::ics04_channel::packet::Sequence;
@@ -25,7 +26,6 @@ pub(crate) mod ids;
 pub(crate) mod trie_key;
 
 pub(crate) type SolanaTimestamp = u64;
-pub(crate) type InnerConnectionId = String;
 pub(crate) type InnerPortId = String;
 pub(crate) type InnerChannelId = String;
 
@@ -86,7 +86,7 @@ impl SequenceTriple {
 #[derive(Clone, Debug, borsh::BorshSerialize, borsh::BorshDeserialize)]
 pub(crate) struct ClientStore {
     pub client_id: ibc::ClientId,
-    pub connection_id: Option<ibc::ConnectionId>,
+    pub connection_id: Option<ids::ConnectionIdx>,
 
     pub client_state: Serialised<AnyClientState>,
     pub consensus_states: BTreeMap<ibc::Height, Serialised<AnyConsensusState>>,
@@ -141,14 +141,20 @@ pub struct IbcPackets(pub Vec<ibc::PacketMsg>);
 
 #[account]
 #[derive(Debug)]
-/// All the structs from IBC are stored as String since they dont implement
-/// AnchorSerialize and AnchorDeserialize
+/// The private IBC storage, i.e. data which doesn’t require proofs.
 pub(crate) struct PrivateStorage {
+    /// Per-client information.
+    ///
+    /// Entry at index `N` corresponds to the client with IBC identifier
+    /// `client-<N>`.
     clients: Vec<ClientStore>,
 
-    pub connection_counter: u64,
-    pub connections:
-        BTreeMap<InnerConnectionId, Serialised<ibc::ConnectionEnd>>,
+    /// Information about the counterparty on given connection.
+    ///
+    /// Entry at index `N` corresponds to the connection with IBC identifier
+    /// `connection-<N>`.
+    pub connections: Vec<Serialised<ibc::ConnectionEnd>>,
+
     pub channel_ends:
         BTreeMap<(InnerPortId, InnerChannelId), Serialised<ibc::ChannelEnd>>,
     pub channel_counter: u64,

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

@@ -1,9 +1,9 @@
-/// 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-";
+use anchor_lang::prelude::borsh;
+
+use super::ibc;
+
+type Result<T, E = ibc::ClientError> = core::result::Result<T, E>;
+
 
 /// Prefix of IBC channel ids.
 ///
@@ -12,6 +12,7 @@ pub(super) const CONNECTION_ID_PREFIX: &str = "connection-";
 /// 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
@@ -51,3 +52,71 @@ impl PartialEq<usize> for ClientIdx {
         u32::try_from(*rhs).ok().filter(|rhs| self.0 == *rhs).is_some()
     }
 }
+
+
+/// An internal connection identifier.
+///
+/// The identifier is build from IBC identifiers which are of the form
+/// `connection-<number>`.  Rather than treating the identifier as a string,
+/// we’re parsing the number out and keep only that.
+#[derive(
+    Clone,
+    Copy,
+    Debug,
+    PartialEq,
+    Eq,
+    borsh::BorshSerialize,
+    borsh::BorshDeserialize,
+    derive_more::From,
+    derive_more::Into,
+)]
+pub struct ConnectionIdx(u32);
+
+impl ConnectionIdx {
+    /// 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.
+    const IBC_PREFIX: &'static str = "connection-";
+}
+
+impl From<ConnectionIdx> for usize {
+    #[inline]
+    fn from(index: ConnectionIdx) -> usize { index.0 as usize }
+}
+
+impl TryFrom<ibc::ConnectionId> for ConnectionIdx {
+    type Error = ibc::ConnectionError;
+
+    fn try_from(id: ibc::ConnectionId) -> Result<Self, Self::Error> {
+        match parse_sans_prefix(Self::IBC_PREFIX, id.as_str()) {
+            Some(num) => Ok(Self(num)),
+            None => Err(ibc::ConnectionError::ConnectionNotFound {
+                connection_id: id,
+            }),
+        }
+    }
+}
+
+impl TryFrom<&ibc::ConnectionId> for ConnectionIdx {
+    type Error = ibc::ConnectionError;
+
+    fn try_from(id: &ibc::ConnectionId) -> Result<Self, Self::Error> {
+        match parse_sans_prefix(Self::IBC_PREFIX, id.as_str()) {
+            Some(num) => Ok(Self(num)),
+            None => Err(ibc::ConnectionError::ConnectionNotFound {
+                connection_id: id.clone(),
+            }),
+        }
+    }
+}
+
+
+/// Strips `prefix` from `data` and parses it to get `u32`.  Panics if data
+/// doesn’t start with the prefix or parsing fails.
+fn parse_sans_prefix(prefix: &'static str, data: &str) -> Option<u32> {
+    data.strip_prefix(prefix)
+        .and_then(|index| index.parse().ok())
+        .filter(|index| usize::try_from(*index).is_ok())
+}

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

@@ -1,11 +1,11 @@
 use ibc::core::ics04_channel::packet::Sequence;
 use ibc::core::ics24_host::identifier::{ChannelId, PortId};
 use ibc::core::ics24_host::path::{
-    AckPath, ChannelEndPath, CommitmentPath, ConnectionPath, ReceiptPath,
-    SeqAckPath, SeqRecvPath, SeqSendPath,
+    AckPath, ChannelEndPath, CommitmentPath, ReceiptPath, SeqAckPath,
+    SeqRecvPath, SeqSendPath,
 };
 
-use crate::storage::ids::{ClientIdx, CHANNEL_ID_PREFIX, CONNECTION_ID_PREFIX};
+use crate::storage::ids;
 
 /// A key used for indexing entries in the provable storage.
 ///
@@ -60,16 +60,24 @@ macro_rules! new_key_impl {
 impl TrieKey {
     /// Constructs a new key for a client state path for client with given
     /// counter.
-    pub fn for_client_state(client: ClientIdx) -> Self {
+    pub fn for_client_state(client: ids::ClientIdx) -> Self {
         new_key_impl!(Tag::ClientState, client)
     }
 
     /// Constructs a new key for a consensus state path for client with given
     /// counter and specified height.
-    pub fn for_consensus_state(client: ClientIdx, height: ibc::Height) -> Self {
+    pub fn for_consensus_state(
+        client: ids::ClientIdx,
+        height: ibc::Height,
+    ) -> Self {
         new_key_impl!(Tag::ConsensusState, client, height)
     }
 
+    /// Constructs a new key for a connection end path.
+    pub fn for_connection(connection: ids::ConnectionIdx) -> Self {
+        new_key_impl!(Tag::Connection, connection)
+    }
+
     /// Constructs a new key for a `(port_id, channel_id)` path.
     ///
     /// Panics if `channel_id` is invalid.
@@ -99,12 +107,6 @@ impl core::ops::Deref for TrieKey {
     fn deref(&self) -> &[u8] { self.0.as_slice() }
 }
 
-impl From<&ConnectionPath> for TrieKey {
-    fn from(path: &ConnectionPath) -> Self {
-        new_key_impl!(Tag::Connection, path.0)
-    }
-}
-
 impl From<&ChannelEndPath> for TrieKey {
     fn from(path: &ChannelEndPath) -> Self {
         Self::from_channel_path(Tag::ChannelEnd, &path.0, &path.1)
@@ -197,17 +199,17 @@ trait AsComponent {
     fn append_into(&self, dest: &mut Vec<u8>);
 }
 
-impl AsComponent for ClientIdx {
+impl AsComponent for ids::ClientIdx {
     fn key_len(&self) -> usize { 0_u32.key_len() }
     fn append_into(&self, dest: &mut Vec<u8>) {
         u32::from(*self).append_into(dest)
     }
 }
 
-impl AsComponent for ibc::core::ics24_host::identifier::ConnectionId {
+impl AsComponent for ids::ConnectionIdx {
     fn key_len(&self) -> usize { 0_u32.key_len() }
     fn append_into(&self, dest: &mut Vec<u8>) {
-        parse_sans_prefix(CONNECTION_ID_PREFIX, self.as_str()).append_into(dest)
+        u32::from(*self).append_into(dest)
     }
 }
 
@@ -223,7 +225,8 @@ impl AsComponent for ibc::core::ics24_host::identifier::PortId {
 impl AsComponent for ibc::core::ics24_host::identifier::ChannelId {
     fn key_len(&self) -> usize { 0_u32.key_len() }
     fn append_into(&self, dest: &mut Vec<u8>) {
-        parse_sans_prefix(CHANNEL_ID_PREFIX, self.as_str()).append_into(dest)
+        parse_sans_prefix(ids::CHANNEL_ID_PREFIX, self.as_str())
+            .append_into(dest)
     }
 }
 

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

@@ -25,7 +25,7 @@ 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::storage::{self, ids, IbcStorage};
 
 type Result<T = (), E = ContextError> = core::result::Result<T, E>;
 
@@ -92,10 +92,11 @@ impl ValidationContext for IbcStorage<'_, '_> {
     }
 
     fn connection_end(&self, conn_id: &ConnectionId) -> Result<ConnectionEnd> {
+        let idx = ids::ConnectionIdx::try_from(conn_id)?;
         self.borrow()
             .private
             .connections
-            .get(conn_id.as_str())
+            .get(usize::from(idx))
             .ok_or_else(|| ConnectionError::ConnectionNotFound {
                 connection_id: conn_id.clone(),
             })?
@@ -121,8 +122,9 @@ impl ValidationContext for IbcStorage<'_, '_> {
     }
 
     fn connection_counter(&self) -> Result<u64> {
-        let store = self.borrow();
-        Ok(store.private.connection_counter)
+        u64::try_from(self.borrow().private.connections.len()).map_err(|err| {
+            ConnectionError::Other { description: err.to_string() }.into()
+        })
     }
 
     fn channel_end(