From 0a7f5635412b22b2e30120f6474cee46ec3dd0f5 Mon Sep 17 00:00:00 2001
From: Ethan Frey <ethanfrey@users.noreply.github.com>
Date: Wed, 28 Jun 2023 10:02:37 +0200
Subject: [PATCH] Addressed first round PR comments

---
 contracts/consumer/converter/src/contract.rs        |  2 +-
 contracts/provider/external-staking/src/contract.rs | 12 +++++-------
 contracts/provider/external-staking/src/ibc.rs      | 12 +-----------
 docs/ibc/Rewards.md                                 |  2 +-
 packages/sync/src/txs.rs                            |  4 +---
 5 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/contracts/consumer/converter/src/contract.rs b/contracts/consumer/converter/src/contract.rs
index 20ee713e..56f2c564 100644
--- a/contracts/consumer/converter/src/contract.rs
+++ b/contracts/consumer/converter/src/contract.rs
@@ -58,7 +58,7 @@ impl ConverterContract<'_> {
         admin: Option<String>,
     ) -> Result<Response, ContractError> {
         nonpayable(&ctx.info)?;
-        // valdiate args
+        // validate args
         if discount > Decimal::one() {
             return Err(ContractError::InvalidDiscount);
         }
diff --git a/contracts/provider/external-staking/src/contract.rs b/contracts/provider/external-staking/src/contract.rs
index 41656800..521286c0 100644
--- a/contracts/provider/external-staking/src/contract.rs
+++ b/contracts/provider/external-staking/src/contract.rs
@@ -355,7 +355,8 @@ impl ExternalStakingContract<'_> {
             data: to_binary(&packet)?,
             timeout: packet_timeout(&ctx.env),
         };
-        // send packet if we are ibc enabled (skip in tests)
+        // send packet if we are ibc enabled
+        // TODO: send in test code when we can handle it
         #[cfg(not(test))]
         {
             resp = resp.add_message(msg);
@@ -750,13 +751,10 @@ impl ExternalStakingContract<'_> {
         self.pending_txs.remove(deps.storage, tx_id);
 
         // Verify tx is of the right type and get data
-        let (_amount, staker, validator) = match tx {
+        let (staker, validator) = match tx {
             Tx::InFlightTransferFunds {
-                amount,
-                staker,
-                validator,
-                ..
-            } => (amount, staker, validator),
+                staker, validator, ..
+            } => (staker, validator),
             _ => {
                 return Err(ContractError::WrongTypeTx(tx_id, tx));
             }
diff --git a/contracts/provider/external-staking/src/ibc.rs b/contracts/provider/external-staking/src/ibc.rs
index ef56c1f3..30e00f6e 100644
--- a/contracts/provider/external-staking/src/ibc.rs
+++ b/contracts/provider/external-staking/src/ibc.rs
@@ -203,7 +203,7 @@ pub fn ibc_packet_ack(
                 .add_attribute("tx_id", tx_id.to_string());
         }
         (ProviderPacket::TransferRewards { tx_id, .. }, AckWrapper::Result(_)) => {
-            // Any events to add?
+            // TODO: Any events to add?
             contract.commit_withdraw_rewards(deps, tx_id)?;
         }
         (ProviderPacket::TransferRewards { tx_id, .. }, AckWrapper::Error(e)) => {
@@ -213,16 +213,6 @@ pub fn ibc_packet_ack(
                 .add_attribute("packet", msg.original_packet.sequence.to_string());
         }
     }
-
-    // Question: do we need a special event with all this info on error?
-
-    //         // Provide info to find the actual packet.
-    //         let event = Event::new("mesh_ibc_error")
-    //             .add_attribute("error", e)
-    //             .add_attribute("channel", msg.original_packet.src.channel_id)
-    //             .add_attribute("sequence", msg.original_packet.sequence.to_string());
-    //         resp = resp.add_event(event);
-    //     }
     Ok(resp)
 }
 
diff --git a/docs/ibc/Rewards.md b/docs/ibc/Rewards.md
index 85988b48..5a75dfc2 100644
--- a/docs/ibc/Rewards.md
+++ b/docs/ibc/Rewards.md
@@ -147,7 +147,7 @@ that should be distributed among all stakers on the given validators. It is up t
 must guarantee to hold `rewards` tokens in reserve to be released by future IBC packets
 on this channel.
 
-The provider side has a new variant `ProviderPacket::TransferFunds {}` which specifies the
+The provider side has a new variant `ProviderPacket::TransferRewards {}` which specifies the
 among and the recipient address on the consumer chain. If the converter contract hold those
 tokens in reserve (always in the case of a correct provider chain), it must release
 them to the given address. The consumer chain must be designed to handle rollbacks here
diff --git a/packages/sync/src/txs.rs b/packages/sync/src/txs.rs
index ca3410ee..0af4be47 100644
--- a/packages/sync/src/txs.rs
+++ b/packages/sync/src/txs.rs
@@ -16,7 +16,6 @@ pub enum Tx {
         /// Remote staking contract
         lienholder: Addr,
     },
-    // IBC flight
     InFlightRemoteStaking {
         /// Transaction id
         id: u64,
@@ -27,7 +26,6 @@ pub enum Tx {
         /// Remote validator
         validator: String,
     },
-    // IBC flight
     InFlightRemoteUnstaking {
         /// Transaction id
         id: u64,
@@ -47,7 +45,7 @@ pub enum Tx {
         staker: Addr,
         /// The validator whose rewards they come from (to revert)
         validator: String,
-    }, // InFlightSlashing
+    },
 }
 
 impl Tx {