From 02f5f4371340d2a464c276ddfb0a317d75f4ddda Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Mon, 20 Nov 2023 21:33:48 +0100 Subject: [PATCH 1/3] solana-ibc: restrict port identifier and use more compact representation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restrict port identifier to be 12 alphanumeric characters. With that restriction it’s now possible to decode the port using base64 encoding to get 9 bytes. Those bytes can then serve as an internal port identifier used with the trie and maps in private storage. Issue: https://github.com/ComposableFi/emulated-light-client/issues/35 --- .../programs/solana-ibc/src/storage/ids.rs | 111 ++++++++++++++++-- .../solana-ibc/src/storage/trie_key.rs | 49 +++++--- 2 files changed, 130 insertions(+), 30 deletions(-) diff --git a/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs b/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs index 7cffa1b9..5cffaf10 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs @@ -1,4 +1,5 @@ use anchor_lang::prelude::borsh; +use base64::engine::{general_purpose, Engine}; use super::ibc; @@ -125,7 +126,7 @@ impl TryFrom<&ibc::ConnectionId> for ConnectionIdx { borsh::BorshDeserialize, )] pub struct PortChannelPK { - pub(super) port_id: ibc::PortId, + pub(super) port_key: PortKey, pub(super) channel_idx: u32, } @@ -141,16 +142,20 @@ impl PortChannelPK { port_id: impl MaybeOwned, channel_id: impl MaybeOwned, ) -> Result { - let channel_str = channel_id.as_ref().as_str(); - match parse_sans_prefix(Self::CHANNEL_IBC_PREFIX, channel_str) { - Some(channel_idx) => { - Ok(Self { port_id: port_id.into_owned(), channel_idx }) - } - None => Err(ibc::ChannelError::ChannelNotFound { - port_id: port_id.into_owned(), - channel_id: channel_id.into_owned(), - }), - } + (|| { + let channel = channel_id.as_ref().as_str(); + Some(Self { + port_key: PortKey::try_from(port_id.as_ref()).ok()?, + channel_idx: parse_sans_prefix( + Self::CHANNEL_IBC_PREFIX, + channel, + )?, + }) + })() + .ok_or_else(|| ibc::ChannelError::ChannelNotFound { + port_id: port_id.into_owned(), + channel_id: channel_id.into_owned(), + }) } } @@ -170,6 +175,90 @@ impl MaybeOwned for T { } +/// An internal port identifier. +/// +/// We’re restricting valid port identifiers to be at most 12 alphanumeric +/// characters. +/// +/// We pad the id with slash characters (which are invalid in IBC identifiers) +/// and then parse them using base64 to get a 9-byte buffer which represents the +/// identifier. +#[derive( + Clone, + PartialEq, + Eq, + PartialOrd, + Ord, + Hash, + borsh::BorshSerialize, + // TODO(mina86): Verify value is valid when deserialising. There are bit + // patterns which don’t correspond to valid port keys. + borsh::BorshDeserialize, +)] +pub struct PortKey([u8; 9]); + +impl PortKey { + #[inline] + pub fn as_bytes(&self) -> &[u8; 9] { &self.0 } +} + +impl TryFrom<&ibc::PortId> for PortKey { + type Error = (); + + fn try_from(port_id: &ibc::PortId) -> Result { + let port_id = port_id.as_bytes(); + // We allow alphanumeric characters only in the port id. We need to + // filter out pluses and slashes since those are valid base64 characters + // and base64 decoder won’t error out on those. + // + // We technically shouldn’t need to check for slashes since IBC should + // guarantee that the identifier has no slash. However, just to make + // sure also filter slashes out. + if port_id.iter().any(|byte| *byte == b'+' || *byte == b'/') { + return Err(()); + } + + // Pad the identifier with slashes. Observe that slash is a valid + // base64 character so we can treat the entire 12-character long string + // as base64-encoded value. + let mut buf = [b'/'; 12]; + buf.get_mut(..port_id.len()).ok_or(())?.copy_from_slice(port_id); + + // Decode into 9-byte buffer. + let mut this = Self([0; 9]); + let len = general_purpose::STANDARD + .decode_slice_unchecked(&buf[..], &mut this.0[..]) + .map_err(|_| ())?; + debug_assert_eq!(this.0.len(), len); + + Ok(this) + } +} + +impl core::fmt::Display for PortKey { + fn fmt(&self, fmtr: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + let mut buf = [0; 12]; + let mut len = general_purpose::STANDARD + .encode_slice(self.as_bytes(), &mut buf[..]) + .unwrap(); + debug_assert_eq!(buf.len(), len); + + while len > 0 && buf[len - 1] == b'/' { + len -= 1; + } + + // SAFETY: base64 outputs ASCII characters. + fmtr.write_str(unsafe { core::str::from_utf8_unchecked(&buf[..len]) }) + } +} + +impl core::fmt::Debug for PortKey { + fn fmt(&self, fmtr: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + core::fmt::Display::fmt(self, fmtr) + } +} + + /// 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 { diff --git a/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs b/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs index 72b0a395..c437f8de 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs @@ -15,13 +15,13 @@ use crate::storage::{ibc, ids}; /// 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 }, -/// NextSequenceRecv { port_id: String, channel_id: u32 }, -/// NextSequenceAck { port_id: String, channel_id: u32 }, -/// Commitment { port_id: String, channel_id: u32, sequence: u64 }, -/// Receipts { port_id: String, channel_id: u32, sequence: u64 }, -/// Acks { port_id: String, channel_id: u32, sequence: u64 }, +/// ChannelEnd { port_id: [u8; 9], channel_id: u32 }, +/// NextSequenceSend { port_id: [u8; 9], channel_id: u32 }, +/// NextSequenceRecv { port_id: [u8; 9], channel_id: u32 }, +/// NextSequenceAck { port_id: [u8; 9], channel_id: u32 }, +/// Commitment { port_id: [u8; 9], channel_id: u32, sequence: u64 }, +/// Receipts { port_id: [u8; 9], channel_id: u32, sequence: u64 }, +/// Acks { port_id: [u8; 9], channel_id: u32, sequence: u64 }, /// } /// ``` /// @@ -83,24 +83,34 @@ impl TrieKey { } /// Constructs a new key for a channel end path. + /// + /// The hash stored under the key is `hash(borsh(channel_end))`. pub fn for_channel_end(port_channel: &ids::PortChannelPK) -> Self { Self::for_channel_path(Tag::ChannelEnd, port_channel) } + /// Constructs a new key for next sequence counters. + /// + /// The hash stored under the key is built by `SequenceTriple::hash` method + /// and directly encodes next send, receive and ack sequence numbers. pub fn for_next_sequence(port_channel: &ids::PortChannelPK) -> Self { Self::for_channel_path(Tag::NextSequence, port_channel) } /// Constructs a new key for a `(port_id, channel_id)` path. /// - /// Panics if `channel_id` is invalid. + /// This is internal method used by other public-facing methods which use + /// only (port, channel) tuple as the key component. fn for_channel_path(tag: Tag, port_channel: &ids::PortChannelPK) -> Self { new_key_impl!(tag, port_channel) } /// Constructs a new key for a `(port_id, channel_id, sequence)` path. /// - /// Panics if `channel_id` is invalid. + /// Returns an error if `port_id` or `channel_id` is invalid. + /// + /// This is internal method used by other public-facing interfaces which use + /// only (port, channel, sequence) tuple as the key component. fn try_for_sequence_path( tag: Tag, port_id: &ibc::PortId, @@ -222,18 +232,12 @@ cast_component!(ids::ClientIdx as u32); cast_component!(ids::ConnectionIdx as u32); cast_component!(ibc::Sequence as u64); -// TODO(#35): Investigate more compact ways of representing port identifier or -// enforcing restrictions on it impl AsComponent for ids::PortChannelPK { fn key_len(&self) -> usize { - let port_id_len = self.port_id.as_bytes().len(); - assert!(port_id_len <= usize::from(u8::MAX)); - 1 + port_id_len + self.channel_idx.key_len() + self.port_key.as_bytes().len() + self.channel_idx.key_len() } fn append_into(&self, dest: &mut Vec) { - let port_id = self.port_id.as_bytes(); - dest.push(port_id.len() as u8); - dest.extend(port_id); + self.port_key.as_bytes().append_into(dest); self.channel_idx.append_into(dest); } } @@ -249,13 +253,20 @@ impl AsComponent for ibc::Height { impl AsComponent for u32 { fn key_len(&self) -> usize { core::mem::size_of_val(self) } fn append_into(&self, dest: &mut Vec) { - dest.extend(&self.to_be_bytes()[..]); + self.to_be_bytes().append_into(dest) } } impl AsComponent for u64 { fn key_len(&self) -> usize { core::mem::size_of_val(self) } fn append_into(&self, dest: &mut Vec) { - dest.extend(&self.to_be_bytes()[..]); + self.to_be_bytes().append_into(dest) + } +} + +impl AsComponent for [u8; N] { + fn key_len(&self) -> usize { N } + fn append_into(&self, dest: &mut Vec) { + dest.extend_from_slice(self) } } From dc1329e69654e25c117f3eea6df6e1c2ab6fc047 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Tue, 21 Nov 2023 16:54:12 +0100 Subject: [PATCH 2/3] fmt --- solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs b/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs index c437f8de..16c5bb8a 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/storage/trie_key.rs @@ -266,7 +266,5 @@ impl AsComponent for u64 { impl AsComponent for [u8; N] { fn key_len(&self) -> usize { N } - fn append_into(&self, dest: &mut Vec) { - dest.extend_from_slice(self) - } + fn append_into(&self, dest: &mut Vec) { dest.extend_from_slice(self) } } From 81bb4c2abfb4cdc69fedb5f0e0ea1c78af7c3e51 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 24 Nov 2023 15:18:50 +0100 Subject: [PATCH 3/3] add getters --- .../programs/solana-ibc/src/storage/ids.rs | 55 +++++++++++++++---- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs b/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs index 5cffaf10..3e0310d5 100644 --- a/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs +++ b/solana/solana-ibc/programs/solana-ibc/src/storage/ids.rs @@ -1,3 +1,5 @@ +use core::str::FromStr; + use anchor_lang::prelude::borsh; use base64::engine::{general_purpose, Engine}; @@ -157,6 +159,14 @@ impl PortChannelPK { channel_id: channel_id.into_owned(), }) } + + #[allow(dead_code)] + pub fn port_id(&self) -> ibc::PortId { ibc::PortId::from(&self.port_key) } + + #[allow(dead_code)] + pub fn channel_id(&self) -> ibc::ChannelId { + ibc::ChannelId::new(self.channel_idx.into()) + } } pub trait MaybeOwned { @@ -200,6 +210,27 @@ pub struct PortKey([u8; 9]); impl PortKey { #[inline] pub fn as_bytes(&self) -> &[u8; 9] { &self.0 } + + fn write_into<'a>(&self, buf: &'a mut [u8; 12]) -> &'a str { + let mut len = general_purpose::STANDARD + .encode_slice(self.as_bytes(), &mut buf[..]) + .unwrap(); + debug_assert_eq!(buf.len(), len); + + while len > 0 && buf[len - 1] == b'/' { + len -= 1; + } + + // SAFETY: base64 outputs ASCII characters. + unsafe { core::str::from_utf8_unchecked(&buf[..len]) } + } +} + +impl TryFrom for PortKey { + type Error = (); + fn try_from(port_id: ibc::PortId) -> Result { + Self::try_from(&port_id) + } } impl TryFrom<&ibc::PortId> for PortKey { @@ -235,20 +266,22 @@ impl TryFrom<&ibc::PortId> for PortKey { } } +impl From for ibc::PortId { + fn from(port_key: PortKey) -> Self { Self::from(&port_key) } +} + +impl From<&PortKey> for ibc::PortId { + fn from(port_key: &PortKey) -> Self { + let mut buf = [0; 12]; + Self::from_str(port_key.write_into(&mut buf)).unwrap() + } +} + impl core::fmt::Display for PortKey { + #[inline] fn fmt(&self, fmtr: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { let mut buf = [0; 12]; - let mut len = general_purpose::STANDARD - .encode_slice(self.as_bytes(), &mut buf[..]) - .unwrap(); - debug_assert_eq!(buf.len(), len); - - while len > 0 && buf[len - 1] == b'/' { - len -= 1; - } - - // SAFETY: base64 outputs ASCII characters. - fmtr.write_str(unsafe { core::str::from_utf8_unchecked(&buf[..len]) }) + fmtr.write_str(self.write_into(&mut buf)) } }