From 21707771fe9cea520cffef9a5b06c543d8543ef6 Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz <mina86@mina86.com>
Date: Tue, 31 Oct 2023 14:27:04 +0100
Subject: [PATCH] solana-ibc: accept just one message in deliver (#67)

Change deliver function to take a single message rather than e vector
of messages.  If caller needs to handle multiple messages, which is
probably uncommon, they can batch Solana transactions.

With that change, handle errors better by actually returning them
on failure.
---
 .../solana-ibc/programs/solana-ibc/Cargo.toml |  1 +
 .../solana-ibc/programs/solana-ibc/src/lib.rs | 41 ++++++++++++++-----
 .../programs/solana-ibc/src/tests.rs          | 12 +++---
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/solana/solana-ibc/programs/solana-ibc/Cargo.toml b/solana/solana-ibc/programs/solana-ibc/Cargo.toml
index 004c30fa..c81238b7 100644
--- a/solana/solana-ibc/programs/solana-ibc/Cargo.toml
+++ b/solana/solana-ibc/programs/solana-ibc/Cargo.toml
@@ -21,6 +21,7 @@ ibc.workspace = true
 ibc-proto.workspace = true
 serde.workspace = true
 serde_json.workspace = true
+strum.workspace = true
 
 lib.workspace = true
 memory.workspace = true
diff --git a/solana/solana-ibc/programs/solana-ibc/src/lib.rs b/solana/solana-ibc/programs/solana-ibc/src/lib.rs
index bd7418c9..1080352f 100644
--- a/solana/solana-ibc/programs/solana-ibc/src/lib.rs
+++ b/solana/solana-ibc/programs/solana-ibc/src/lib.rs
@@ -37,13 +37,11 @@ pub mod solana_ibc {
 
     pub fn deliver(
         ctx: Context<Deliver>,
-        messages: Vec<ibc::core::MsgEnvelope>,
+        message: ibc::core::MsgEnvelope,
     ) -> Result<()> {
-        msg!("Called deliver method");
+        msg!("Called deliver method: {message}");
         let _sender = ctx.accounts.sender.to_account_info();
 
-        msg!("These are messages {:?}", messages);
-
         let private: &mut PrivateStorage = &mut ctx.accounts.storage;
         msg!("This is private_store {:?}", private);
 
@@ -56,11 +54,9 @@ pub mod solana_ibc {
         let mut store = IbcStorage(Rc::new(RefCell::new(inner)));
         let mut router = store.clone();
 
-        let errors = messages
-            .into_iter()
-            .map(|msg| ibc::core::dispatch(&mut store, &mut router, msg))
-            .filter_map(core::result::Result::err)
-            .collect::<Vec<_>>();
+        if let Err(e) = ibc::core::dispatch(&mut store, &mut router, message) {
+            return err!(Error::RouterError(&e));
+        }
 
         // Drop refcount on store so we can unwrap the Rc object below.
         core::mem::drop(router);
@@ -70,7 +66,6 @@ pub mod solana_ibc {
         // so using the inner function instead.
         let inner = Rc::try_unwrap(store.0).unwrap().into_inner();
 
-        msg!("These are errors {:?}", errors);
         msg!("This is final structure {:?}", inner.private);
 
         // msg!("this is length {}", TrieKey::ClientState{ client_id: String::from("hello")}.into());
@@ -91,6 +86,32 @@ pub struct Deliver<'info> {
     pub system_program: Program<'info, System>,
 }
 
+/// Error returned when handling a request.
+#[derive(Clone, strum::AsRefStr, strum::EnumDiscriminants)]
+#[strum_discriminants(repr(u32))]
+pub enum Error<'a> {
+    RouterError(&'a ibc::core::RouterError),
+}
+
+impl Error<'_> {
+    pub fn name(&self) -> String { self.as_ref().into() }
+}
+
+impl core::fmt::Display for Error<'_> {
+    fn fmt(&self, fmtr: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Self::RouterError(err) => write!(fmtr, "{err}"),
+        }
+    }
+}
+
+impl From<Error<'_>> for u32 {
+    fn from(err: Error<'_>) -> u32 {
+        let code = ErrorDiscriminants::from(err) as u32;
+        anchor_lang::error::ERROR_CODE_OFFSET + code
+    }
+}
+
 #[event]
 pub struct EmitIBCEvent {
     pub ibc_event: Vec<u8>,
diff --git a/solana/solana-ibc/programs/solana-ibc/src/tests.rs b/solana/solana-ibc/programs/solana-ibc/src/tests.rs
index 0c3b55c5..b674c48d 100644
--- a/solana/solana-ibc/programs/solana-ibc/src/tests.rs
+++ b/solana/solana-ibc/programs/solana-ibc/src/tests.rs
@@ -82,7 +82,7 @@ fn anchor_test_deliver() -> Result<()> {
 
     let (mock_client_state, mock_cs_state) = create_mock_client_and_cs_state();
     let _client_id = ClientId::new(mock_client_state.client_type(), 0).unwrap();
-    let messages = vec![make_message!(
+    let message = make_message!(
         MsgCreateClient::new(
             Any::from(mock_client_state),
             Any::from(mock_cs_state),
@@ -90,7 +90,7 @@ fn anchor_test_deliver() -> Result<()> {
         ),
         ibc::core::ics02_client::msgs::ClientMsg::CreateClient,
         ibc::core::MsgEnvelope::Client,
-    )];
+    );
 
     let sig = program
         .request()
@@ -100,7 +100,7 @@ fn anchor_test_deliver() -> Result<()> {
             trie,
             system_program: system_program::ID,
         })
-        .args(instruction::Deliver { messages })
+        .args(instruction::Deliver { message })
         .payer(authority.clone())
         .signer(&*authority)
         .send_with_spinner_and_config(RpcSendTransactionConfig {
@@ -122,7 +122,7 @@ fn anchor_test_deliver() -> Result<()> {
     let commitment_prefix: CommitmentPrefix =
         IBC_TRIE_PREFIX.to_vec().try_into().unwrap();
 
-    let messages = vec![make_message!(
+    let message = make_message!(
         MsgConnectionOpenInit {
             client_id_on_a: ClientId::new(mock_client_state.client_type(), 0)
                 .unwrap(),
@@ -137,7 +137,7 @@ fn anchor_test_deliver() -> Result<()> {
         },
         ibc::core::ics03_connection::msgs::ConnectionMsg::OpenInit,
         ibc::core::MsgEnvelope::Connection,
-    )];
+    );
 
     let sig = program
         .request()
@@ -147,7 +147,7 @@ fn anchor_test_deliver() -> Result<()> {
             trie,
             system_program: system_program::ID,
         })
-        .args(instruction::Deliver { messages })
+        .args(instruction::Deliver { message })
         .payer(authority.clone())
         .signer(&*authority)
         .send_with_spinner_and_config(RpcSendTransactionConfig {