From 3686bb4fa7f814ce85b99c98c5330e13ba160d82 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 17 Nov 2023 02:12:22 +0100 Subject: [PATCH] solana-ibc: map IBC client ids into u32 and refactor client state storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IBC client ids have ‘-’ 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: https://github.com/ComposableFi/emulated-light-client/issues/35 --- common/lib/src/hash.rs | 27 ++- .../programs/solana-ibc/src/client_state.rs | 25 +-- .../solana-ibc/src/execution_context.rs | 136 ++++-------- .../programs/solana-ibc/src/storage.rs | 207 +++++++++++++++--- .../programs/solana-ibc/src/trie_key.rs | 52 ++--- .../solana-ibc/src/validation_context.rs | 75 +++---- 6 files changed, 307 insertions(+), 215 deletions(-) diff --git a/common/lib/src/hash.rs b/common/lib/src/hash.rs index 269ae12c..8b84bec2 100644 --- a/common/lib/src/hash.rs +++ b/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 { + 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 { // 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,14 +257,21 @@ 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"); @@ -265,4 +279,9 @@ fn test_new_hash() { 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()); } diff --git a/solana/solana-ibc/programs/solana-ibc/src/client_state.rs b/solana/solana-ibc/programs/solana-ibc/src/client_state.rs index 314942e5..44110114 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/client_state.rs +++ b/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, 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::, _>>() .map_err(ContextError::from) } @@ -532,7 +531,7 @@ impl IbcStorage<'_, '_> { dir: Direction, ) -> Result, 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 { @@ -540,13 +539,13 @@ impl IbcStorage<'_, '_> { }; 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())) } } diff --git a/solana/solana-ibc/programs/solana-ibc/src/execution_context.rs b/solana/solana-ibc/programs/solana-ibc/src/execution_context.rs index 17747f68..0ed29bfe 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/execution_context.rs +++ b/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() +} diff --git a/solana/solana-ibc/programs/solana-ibc/src/storage.rs b/solana/solana-ibc/programs/solana-ibc/src/storage.rs index 9ed523e8..4c181486 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/storage.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/storage.rs @@ -3,20 +3,27 @@ use alloc::rc::Rc; use core::cell::RefCell; use anchor_lang::prelude::*; -use ibc::core::ics04_channel::msgs::PacketMsg; -use ibc::core::ics04_channel::packet::Sequence; +use lib::hash::CryptoHash; + +type Result = core::result::Result; + +mod ibc { + pub(super) use ibc::core::ics02_client::error::ClientError; + pub(super) use ibc::core::ics04_channel::msgs::PacketMsg; + pub(super) use ibc::core::ics04_channel::packet::Sequence; + pub(super) use ibc::core::ics24_host::identifier::{ + ClientId, ConnectionId, + }; + pub(super) use ibc::Height; +} pub(crate) type InnerHeight = (u64, u64); -pub(crate) type HostHeight = InnerHeight; pub(crate) type SolanaTimestamp = u64; -pub(crate) type InnerClientId = String; pub(crate) type InnerConnectionId = String; pub(crate) type InnerPortId = String; pub(crate) type InnerChannelId = String; -pub(crate) type InnerClient = Vec; // Serialized pub(crate) type InnerConnectionEnd = Vec; // Serialized pub(crate) type InnerChannelEnd = Vec; // Serialized -pub(crate) type InnerConsensusState = Vec; // Serialized /// A triple of send, receive and acknowledge sequences. #[derive( @@ -42,24 +49,24 @@ pub(crate) enum SequenceTripleIdx { impl SequenceTriple { /// Returns sequence at given index or `None` if it wasn’t set yet. - pub(crate) fn get(&self, idx: SequenceTripleIdx) -> Option { + pub(crate) fn get(&self, idx: SequenceTripleIdx) -> Option { if self.mask & (1 << (idx as u32)) == 1 { - Some(Sequence::from(self.sequences[idx as usize])) + Some(ibc::Sequence::from(self.sequences[idx as usize])) } else { None } } /// Sets sequence at given index. - pub(crate) fn set(&mut self, idx: SequenceTripleIdx, seq: Sequence) { + pub(crate) fn set(&mut self, idx: SequenceTripleIdx, seq: ibc::Sequence) { self.sequences[idx as usize] = u64::from(seq); self.mask |= 1 << (idx as u32) } /// Encodes the object as a `CryptoHash` so it can be stored in the trie /// directly. - pub(crate) fn to_hash(&self) -> lib::hash::CryptoHash { - let mut hash = lib::hash::CryptoHash::default(); + pub(crate) fn to_hash(&self) -> CryptoHash { + let mut hash = CryptoHash::default(); let (first, tail) = stdx::split_array_mut::<8, 24, 32>(&mut hash.0); let (second, tail) = stdx::split_array_mut::<8, 16, 24>(tail); let (third, tail) = stdx::split_array_mut::<8, 8, 16>(tail); @@ -71,9 +78,80 @@ impl SequenceTriple { } } +/// An index used as unique identifier for a client. +/// +/// IBC client id uses `-` 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 `ClientIndex` +/// [`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 ClientIndex(u32); + +impl From for usize { + #[inline] + fn from(index: ClientIndex) -> usize { index.0 as usize } +} + +impl core::str::FromStr for ClientIndex { + type Err = core::num::ParseIntError; + + #[inline] + fn from_str(value: &str) -> Result { + if core::mem::size_of::() < 4 { + usize::from_str(value).map(|index| Self(index as u32)) + } else { + u32::from_str(value).map(Self) + } + } +} + +impl PartialEq for ClientIndex { + #[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 { + pub client_id: ibc::ClientId, + pub connection_id: Option, + + pub client_state: crate::client_state::AnyClientState, + pub consensus_states: + BTreeMap, + + pub processed_times: BTreeMap, + pub processed_heights: BTreeMap, +} + +impl ClientStore { + fn new( + client_id: ibc::ClientId, + client_state: crate::client_state::AnyClientState, + ) -> Self { + Self { + client_id, + connection_id: Default::default(), + client_state, + consensus_states: Default::default(), + processed_times: Default::default(), + processed_heights: Default::default(), + } + } +} + #[account] #[derive(Debug)] -pub struct IBCPackets(pub Vec); +pub struct IBCPackets(pub Vec); #[account] #[derive(Debug)] @@ -81,37 +159,24 @@ pub struct IBCPackets(pub Vec); /// AnchorSerialize and AnchorDeserialize pub(crate) struct PrivateStorage { pub height: InnerHeight, - pub clients: BTreeMap, - /// The client ids of the clients. - pub client_id_set: Vec, - pub client_counter: u64, - pub client_processed_times: - BTreeMap>, - pub client_processed_heights: - BTreeMap>, - pub consensus_states: - BTreeMap<(InnerClientId, InnerHeight), InnerConsensusState>, - /// This collection contains the heights corresponding to all consensus states of - /// all clients stored in the contract. - pub client_consensus_state_height_sets: - BTreeMap>, + + clients: Vec, + /// The connection ids of the connections. pub connection_id_set: Vec, pub connection_counter: u64, pub connections: BTreeMap, pub channel_ends: BTreeMap<(InnerPortId, InnerChannelId), InnerChannelEnd>, - // Contains the client id corresponding to the connectionId - pub client_to_connection: BTreeMap, /// The port and channel id tuples of the channels. pub port_channel_id_set: Vec<(InnerPortId, InnerChannelId)>, pub channel_counter: u64, /// The sequence numbers of the packet commitments. pub packet_commitment_sequence_sets: - BTreeMap<(InnerPortId, InnerChannelId), Vec>, + BTreeMap<(InnerPortId, InnerChannelId), Vec>, /// The sequence numbers of the packet acknowledgements. pub packet_acknowledgement_sequence_sets: - BTreeMap<(InnerPortId, InnerChannelId), Vec>, + BTreeMap<(InnerPortId, InnerChannelId), Vec>, /// Next send, receive and ack sequence for given (port, channel). /// @@ -121,6 +186,88 @@ pub(crate) struct PrivateStorage { pub next_sequence: BTreeMap<(InnerPortId, InnerChannelId), SequenceTriple>, } +impl PrivateStorage { + /// Returns number of known clients; or counter for the next client. + pub fn client_counter(&self) -> u64 { + u64::try_from(self.clients.len()).unwrap() + } + + /// Returns state for an existing client. + /// + /// Client ids use `-` format where is + /// sequential. We take advantage of that by extracting the and + /// using it as index in client states. + pub fn client( + &self, + client_id: &ibc::ClientId, + ) -> Result<(ClientIndex, &ClientStore), ibc::ClientError> { + self.client_index(client_id) + .and_then(|idx| { + self.clients + .get(usize::from(idx)) + .filter(|state| state.client_id == *client_id) + .map(|state| (idx, state)) + }) + .ok_or_else(|| ibc::ClientError::ClientStateNotFound { + client_id: client_id.clone(), + }) + } + + /// Returns state for an existing client. + /// + /// Client ids use `-` format where is + /// sequential. We take advantage of that by extracting the and + /// using it as index in client states. + pub fn client_mut( + &mut self, + client_id: &ibc::ClientId, + ) -> Result<(ClientIndex, &mut ClientStore), ibc::ClientError> { + self.client_index(client_id) + .and_then(|idx| { + self.clients + .get_mut(usize::from(idx)) + .filter(|state| state.client_id == *client_id) + .map(|state| (idx, state)) + }) + .ok_or_else(|| ibc::ClientError::ClientStateNotFound { + client_id: client_id.clone(), + }) + } + + /// Sets client’s status potentially inserting a new client. + /// + /// If client’s index is exactly one past the last existing client, inserts + /// a new client. Otherwise, expects the client id to correspond to an + /// existing client. + pub fn set_client( + &mut self, + client_id: ibc::ClientId, + client_state: crate::client_state::AnyClientState, + ) -> Result<(ClientIndex, &mut ClientStore), ibc::ClientError> { + if let Some(index) = self.client_index(&client_id) { + if index == self.clients.len() { + let store = ClientStore::new(client_id, client_state); + self.clients.push(store); + return Ok((index, self.clients.last_mut().unwrap())); + } + if let Some(store) = self.clients.get_mut(usize::from(index)) { + if store.client_id == client_id { + store.client_state = client_state; + return Ok((index, store)); + } + } + } + Err(ibc::ClientError::ClientStateNotFound { client_id }) + } + + fn client_index(&self, client_id: &ibc::ClientId) -> Option { + client_id + .as_str() + .rsplit_once('-') + .and_then(|(_, index)| core::str::FromStr::from_str(index).ok()) + } +} + /// Provable storage, i.e. the trie, held in an account. pub type AccountTrie<'a, 'b> = solana_trie::AccountTrie>; diff --git a/solana/solana-ibc/programs/solana-ibc/src/trie_key.rs b/solana/solana-ibc/programs/solana-ibc/src/trie_key.rs index 45132014..34150f08 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/trie_key.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/trie_key.rs @@ -1,15 +1,15 @@ use ibc::core::ics04_channel::packet::Sequence; use ibc::core::ics24_host::identifier::{ChannelId, PortId}; use ibc::core::ics24_host::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, ClientStatePath, - CommitmentPath, ConnectionPath, ReceiptPath, SeqAckPath, SeqRecvPath, - SeqSendPath, + AckPath, ChannelEndPath, CommitmentPath, ConnectionPath, ReceiptPath, + 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::ClientIndex; /// A key used for indexing entries in the provable storage. /// @@ -19,8 +19,8 @@ use super::{CHANNEL_ID_PREFIX, CONNECTION_ID_PREFIX}; /// /// ```ignore /// enum TrieKey { -/// ClientState { client_id: String }, -/// ConsensusState { client_id: String, epoch: u64, height: u64 }, +/// ClientState { client_id: u32 }, +/// ConsensusState { client_id: u32, epoch: u64, height: u64 }, /// Connection { connection_id: u32 }, /// ChannelEnd { port_id: String, channel_id: u32 }, /// NextSequenceSend { port_id: String, channel_id: u32 }, @@ -62,6 +62,22 @@ 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(counter: ClientIndex) -> Self { + new_key_impl!(Tag::ClientState, u32::from(counter)) + } + + /// Constructs a new key for a consensus state path for client with given + /// counter and specified height. + pub fn for_consensus_state( + counter: ClientIndex, + epoch: u64, + height: u64, + ) -> Self { + new_key_impl!(Tag::ConsensusState, u32::from(counter), epoch, height) + } + /// Constructs a new key for a `(port_id, channel_id)` path. /// /// Panics if `channel_id` is invalid. @@ -91,23 +107,6 @@ impl core::ops::Deref for TrieKey { fn deref(&self) -> &[u8] { self.0.as_slice() } } -impl From<&ClientStatePath> for TrieKey { - fn from(path: &ClientStatePath) -> Self { - new_key_impl!(Tag::ClientState, path.0) - } -} - -impl From<&ClientConsensusStatePath> for TrieKey { - fn from(path: &ClientConsensusStatePath) -> Self { - new_key_impl!( - Tag::ConsensusState, - path.client_id, - path.epoch, - path.height - ) - } -} - impl From<&ConnectionPath> for TrieKey { fn from(path: &ConnectionPath) -> Self { new_key_impl!(Tag::Connection, path.0) @@ -206,15 +205,6 @@ trait AsComponent { fn append_into(&self, dest: &mut Vec); } -// TODO(#35): Investigate weather we can impose restrictions on client -// identifiers, e.g. `client-`. -impl AsComponent for ibc::core::ics24_host::identifier::ClientId { - fn key_len(&self) -> usize { self.as_str().key_len() } - fn append_into(&self, dest: &mut Vec) { - self.as_str().append_into(dest) - } -} - impl AsComponent for ibc::core::ics24_host::identifier::ConnectionId { fn key_len(&self) -> usize { 0_u32.key_len() } fn append_into(&self, dest: &mut Vec) { diff --git a/solana/solana-ibc/programs/solana-ibc/src/validation_context.rs b/solana/solana-ibc/programs/solana-ibc/src/validation_context.rs index d29ef905..902bd35a 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/validation_context.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/validation_context.rs @@ -39,13 +39,7 @@ impl ValidationContext for IbcStorage<'_, '_> { &self, client_id: &ClientId, ) -> Result { - deserialise( - self.borrow().private.clients.get(client_id.as_str()), - || ClientError::ClientStateNotFound { - client_id: client_id.clone(), - }, - |description| ClientError::Other { description }, - ) + Ok(self.borrow().private.client(client_id)?.1.client_state.clone()) } fn decode_client_state( @@ -57,32 +51,23 @@ impl ValidationContext for IbcStorage<'_, '_> { fn consensus_state( &self, - client_cons_state_path: &ClientConsensusStatePath, + path: &ClientConsensusStatePath, ) -> Result { - let key = &( - client_cons_state_path.client_id.to_string(), - (client_cons_state_path.epoch, client_cons_state_path.height), - ); - let state = self - .borrow() + self.borrow() .private + .client(&path.client_id)? + .1 .consensus_states - .get(key) - .map(|data| borsh::BorshDeserialize::try_from_slice(data)); - match state { - Some(Ok(value)) => Ok(value), - Some(Err(err)) => Err(ClientError::ClientSpecific { - description: err.to_string(), - }), - None => Err(ClientError::ConsensusStateNotFound { - client_id: client_cons_state_path.client_id.clone(), - height: ibc::Height::new( - client_cons_state_path.epoch, - client_cons_state_path.height, - )?, - }), - } - .map_err(ibc::core::ContextError::from) + .get(&(path.epoch, path.height)) + .cloned() + .ok_or_else(|| match ibc::Height::new(path.epoch, path.height) { + Ok(height) => ClientError::ConsensusStateNotFound { + client_id: path.client_id.clone(), + height, + }, + Err(err) => err, + }) + .map_err(ibc::core::ContextError::from) } fn host_height(&self) -> Result { @@ -109,8 +94,7 @@ impl ValidationContext for IbcStorage<'_, '_> { } fn client_counter(&self) -> Result { - let store = self.borrow(); - Ok(store.private.client_counter) + Ok(self.borrow().private.client_counter()) } fn connection_end(&self, conn_id: &ConnectionId) -> Result { @@ -295,12 +279,10 @@ impl ibc::core::ics02_client::ClientValidationContext for IbcStorage<'_, '_> { let store = self.borrow(); store .private - .client_processed_times - .get(client_id.as_str()) - .and_then(|processed_times| { - processed_times - .get(&(height.revision_number(), height.revision_height())) - }) + .client(client_id)? + .1 + .processed_times + .get(height) .map(|ts| Timestamp::from_nanoseconds(*ts).unwrap()) .ok_or_else(|| { ContextError::ClientError(ClientError::Other { @@ -318,18 +300,13 @@ impl ibc::core::ics02_client::ClientValidationContext for IbcStorage<'_, '_> { client_id: &ClientId, height: &Height, ) -> Result { - let store = self.borrow(); - store + self.borrow() .private - .client_processed_heights - .get(client_id.as_str()) - .and_then(|processed_heights| { - processed_heights - .get(&(height.revision_number(), height.revision_height())) - }) - .map(|client_height| { - Height::new(client_height.0, client_height.1).unwrap() - }) + .client(client_id)? + .1 + .processed_heights + .get(height) + .copied() .ok_or_else(|| { ContextError::ClientError(ClientError::Other { description: format!(