solana-ibc: map IBC client ids into u32 and refactor client state sto…

…rage

IBC client ids have ‘<client-type>-<counter>’ format where counter is
a global sequential number.  Take advantage of that by converting
client ids into u32 including just the counter.

Since we’re effectively ignoring the client-type part of the id, keep
counter → client id map in private storage and use it to verify that
id we were given is the one we know about.

Furthermore, rather than storing client information in a map keep it
in a vector which is more compact and faster to index.  At the same
time, keep just a single vector for all per-client data rather than
having separate maps for each piece of information.

Issue: #35

common/lib/src/hash.rs

@@ -49,6 +49,15 @@ impl CryptoHash {
         builder.build()
     }
 
+    /// Returns hash of a Borsh-serialised object.
+    #[inline]
+    #[cfg(feature = "borsh")]
+    pub fn borsh(obj: &impl borsh::BorshSerialize) -> io::Result<Self> {
+        let mut builder = Self::builder();
+        obj.serialize(&mut builder)?;
+        Ok(builder.build())
+    }
+
     /// Decodes a base64 string representation of the hash.
     pub fn from_base64(base64: &str) -> Option<Self> {
         // base64 API is kind of garbage.  In certain situations the output
@@ -233,9 +242,7 @@ impl io::Write for Builder {
 }
 
 #[test]
-fn test_new_hash() {
-    assert_eq!(CryptoHash::from([0; 32]), CryptoHash::default());
-
+fn test_empty_digest() {
     // https://www.di-mgt.com.au/sha_testvectors.html
     let want = CryptoHash::from([
         0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8,
@@ -250,19 +257,31 @@ fn test_new_hash() {
         builder.build()
     };
     assert_eq!(want, got);
-    assert_eq!(want, CryptoHash::builder().build());
 
+    #[cfg(feature = "borsh")]
+    assert_eq!(want, CryptoHash::borsh(&()).unwrap());
+}
+
+#[test]
+fn test_abc_digest() {
+    // https://www.di-mgt.com.au/sha_testvectors.html
     let want = CryptoHash::from([
         0xba, 0x78, 0x16, 0xbf, 0x8f, 0x01, 0xcf, 0xea, 0x41, 0x41, 0x40, 0xde,
         0x5d, 0xae, 0x22, 0x23, 0xb0, 0x03, 0x61, 0xa3, 0x96, 0x17, 0x7a, 0x9c,
         0xb4, 0x10, 0xff, 0x61, 0xf2, 0x00, 0x15, 0xad,
     ]);
     assert_eq!(want, CryptoHash::digest(b"abc"));
+
     let got = {
         let mut builder = CryptoHash::builder();
         builder.update(b"a");
         builder.update(b"bc");
         builder.build()
     };
     assert_eq!(want, got);
+
+    #[cfg(feature = "borsh")]
+    assert_eq!(want, CryptoHash::borsh(b"abc").unwrap());
+    #[cfg(feature = "borsh")]
+    assert_eq!(want, CryptoHash::borsh(&(b'a', 25442u16)).unwrap());
 }

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

@@ -458,14 +458,13 @@ impl ibc::clients::ics07_tendermint::CommonContext for IbcStorage<'_, '_> {
         &self,
         client_id: &ClientId,
     ) -> Result<Vec<Height>, ContextError> {
-        // TODO(mina86): use BTreeMap::range here so that we don’t iterate over
-        // the entire map.
         self.borrow()
             .private
+            .client(client_id)?
+            .1
             .consensus_states
             .keys()
-            .filter(|(client, _)| client == client_id.as_str())
-            .map(|(_, height)| ibc::Height::new(height.0, height.1))
+            .map(|height| ibc::Height::new(height.0, height.1))
             .collect::<Result<Vec<_>, _>>()
             .map_err(ContextError::from)
     }
@@ -532,21 +531,21 @@ impl IbcStorage<'_, '_> {
         dir: Direction,
     ) -> Result<Option<AnyConsensusState>, ContextError> {
         let height = (height.revision_number(), height.revision_height());
-        let pivot = core::ops::Bound::Excluded((client_id.to_string(), height));
+        let pivot = core::ops::Bound::Excluded(height);
         let range = if dir == Direction::Next {
             (pivot, core::ops::Bound::Unbounded)
         } else {
             (core::ops::Bound::Unbounded, pivot)
         };
 
         let store = self.borrow();
-        let mut range = store.private.consensus_states.range(range);
-        if dir == Direction::Next { range.next() } else { range.next_back() }
-            .map(|(_, data)| borsh::BorshDeserialize::try_from_slice(data))
-            .transpose()
-            .map_err(|err| err.to_string())
-            .map_err(|description| {
-                ContextError::from(ClientError::ClientSpecific { description })
-            })
+        let mut range =
+            store.private.client(client_id)?.1.consensus_states.range(range);
+        Ok(if dir == Direction::Next {
+            range.next()
+        } else {
+            range.next_back()
+        }
+        .map(|(_, data)| data.clone()))
     }
 }

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

@@ -37,44 +37,30 @@ impl ClientExecutionContext for IbcStorage<'_, '_> {
     fn store_client_state(
         &mut self,
         path: ClientStatePath,
-        client_state: Self::AnyClientState,
+        state: Self::AnyClientState,
     ) -> Result {
-        msg!("store_client_state({}, {:?})", path, client_state);
+        msg!("store_client_state({}, {:?})", path, state);
+        let hash = CryptoHash::borsh(&state).map_err(error)?;
         let mut store = self.borrow_mut();
-        let serialized = store_serialised_proof(
-            &mut store.provable,
-            &TrieKey::from(&path),
-            &client_state,
-        )?;
-        let key = path.0.to_string();
-        store.private.clients.insert(key.clone(), serialized);
-        store.private.client_id_set.push(key);
-        Ok(())
+        let (client_idx, _) = store.private.set_client(path.0, state)?;
+        let key = TrieKey::for_client_state(client_idx);
+        store.provable.set(&key, &hash).map_err(error)
     }
 
     fn store_consensus_state(
         &mut self,
-        consensus_state_path: ClientConsensusStatePath,
-        consensus_state: Self::AnyConsensusState,
+        path: ClientConsensusStatePath,
+        state: Self::AnyConsensusState,
     ) -> Result {
-        msg!(
-            "store_consensus_state - path: {}, consensus_state: {:?}",
-            consensus_state_path,
-            consensus_state
-        );
+        msg!("store_consensus_state({}, {:?})", path, state);
+        let hash = CryptoHash::borsh(&state).map_err(error)?;
         let mut store = self.borrow_mut();
-        let serialized = store_serialised_proof(
-            &mut store.provable,
-            &TrieKey::from(&consensus_state_path),
-            &consensus_state,
-        )?;
-        let key = (
-            consensus_state_path.client_id.to_string(),
-            (consensus_state_path.epoch, consensus_state_path.height),
-        );
-        store.private.consensus_states.insert(key, serialized);
-        store.private.height =
-            (consensus_state_path.epoch, consensus_state_path.height);
+        let (client_idx, client) = store.private.client_mut(&path.client_id)?;
+        client.consensus_states.insert((path.epoch, path.height), state);
+        let key =
+            TrieKey::for_consensus_state(client_idx, path.epoch, path.height);
+        store.provable.set(&key, &hash).map_err(error)?;
+        store.private.height = (path.epoch, path.height);
         Ok(())
     }
 
@@ -83,10 +69,12 @@ impl ClientExecutionContext for IbcStorage<'_, '_> {
         path: ClientConsensusStatePath,
     ) -> Result<(), ContextError> {
         msg!("delete_consensus_state({})", path);
-        let key = (path.client_id.to_string(), (path.epoch, path.height));
         let mut store = self.borrow_mut();
-        store.private.consensus_states.remove(&key);
-        store.provable.del(&TrieKey::from(&path)).unwrap();
+        let (client_idx, client) = store.private.client_mut(&path.client_id)?;
+        client.consensus_states.remove(&(path.epoch, path.height));
+        let key =
+            TrieKey::for_consensus_state(client_idx, path.epoch, path.height);
+        store.provable.del(&key).map_err(error)?;
         Ok(())
     }
 
@@ -98,14 +86,10 @@ impl ClientExecutionContext for IbcStorage<'_, '_> {
     ) -> Result<(), ContextError> {
         self.borrow_mut()
             .private
-            .client_processed_heights
-            .get_mut(client_id.as_str())
-            .and_then(|processed_times| {
-                processed_times.remove(&(
-                    height.revision_number(),
-                    height.revision_height(),
-                ))
-            });
+            .client_mut(&client_id)?
+            .1
+            .processed_heights
+            .remove(&height);
         Ok(())
     }
 
@@ -116,14 +100,10 @@ impl ClientExecutionContext for IbcStorage<'_, '_> {
     ) -> Result<(), ContextError> {
         self.borrow_mut()
             .private
-            .client_processed_times
-            .get_mut(client_id.as_str())
-            .and_then(|processed_times| {
-                processed_times.remove(&(
-                    height.revision_number(),
-                    height.revision_height(),
-                ))
-            });
+            .client_mut(&client_id)?
+            .1
+            .processed_times
+            .remove(&height);
         Ok(())
     }
 
@@ -136,13 +116,10 @@ impl ClientExecutionContext for IbcStorage<'_, '_> {
         msg!("store_update_time({}, {}, {})", client_id, height, timestamp);
         self.borrow_mut()
             .private
-            .client_processed_times
-            .entry(client_id.to_string())
-            .or_default()
-            .insert(
-                (height.revision_number(), height.revision_height()),
-                timestamp.nanoseconds(),
-            );
+            .client_mut(&client_id)?
+            .1
+            .processed_times
+            .insert(height, timestamp.nanoseconds());
         Ok(())
     }
 
@@ -155,28 +132,16 @@ impl ClientExecutionContext for IbcStorage<'_, '_> {
         msg!("store_update_height({}, {}, {})", client_id, height, host_height);
         self.borrow_mut()
             .private
-            .client_processed_heights
-            .entry(client_id.to_string())
-            .or_default()
-            .insert(
-                (height.revision_number(), height.revision_height()),
-                (host_height.revision_number(), host_height.revision_height()),
-            );
+            .client_mut(&client_id)?
+            .1
+            .processed_heights
+            .insert(height, host_height);
         Ok(())
     }
 }
 
 impl ExecutionContext for IbcStorage<'_, '_> {
-    fn increase_client_counter(&mut self) -> Result {
-        let mut store = self.borrow_mut();
-        store.private.client_counter =
-            store.private.client_counter.checked_add(1).unwrap();
-        msg!(
-            "client_counter has increased to: {}",
-            store.private.client_counter
-        );
-        Ok(())
-    }
+    fn increase_client_counter(&mut self) -> Result { Ok(()) }
 
     fn store_connection(
         &mut self,
@@ -196,19 +161,12 @@ impl ExecutionContext for IbcStorage<'_, '_> {
 
     fn store_connection_to_client(
         &mut self,
-        client_connection_path: &ClientConnectionPath,
+        path: &ClientConnectionPath,
         conn_id: ConnectionId,
     ) -> Result {
-        msg!(
-            "store_connection_to_client: path: {}, connection_id: {:?}",
-            client_connection_path,
-            conn_id
-        );
-        let mut store = self.borrow_mut();
-        store
-            .private
-            .client_to_connection
-            .insert(client_connection_path.0.to_string(), conn_id.to_string());
+        msg!("store_connection_to_client({}, {:?})", path, conn_id);
+        self.borrow_mut().private.client_mut(&path.0)?.1.connection_id =
+            Some(conn_id);
         Ok(())
     }
 
@@ -371,8 +329,7 @@ impl ExecutionContext for IbcStorage<'_, '_> {
     }
 
     fn emit_ibc_event(&mut self, event: IbcEvent) -> Result {
-        crate::events::emit(event)
-            .map_err(|description| ClientError::Other { description }.into())
+        crate::events::emit(event).map_err(error)
     }
 
     fn log_message(&mut self, message: String) -> Result {
@@ -423,8 +380,7 @@ fn store_serialised_proof(
                     .map(|()| value)
                     .map_err(|err| err.to_string())
             })
-            .map_err(|description| ClientError::Other { description })
-            .map_err(ContextError::ClientError)
+            .map_err(error)
     }
     store_impl(trie, key, borsh::to_vec(value))
 }
@@ -463,3 +419,7 @@ fn delete_packet_sequence(
         }
     }
 }
+
+fn error(description: impl ToString) -> ContextError {
+    ClientError::Other { description: description.to_string() }.into()
+}