doc: review comments

contracts/interchain-token-service/src/contract/execute.rs

@@ -270,9 +270,9 @@ pub fn set_chain_config(
 /// The amount is calculated based on the token decimals on the source and destination chains.
 /// The calculation is done as following:
 /// 1) `destination_amount` = `source_amount` * 10 ^ (`destination_chain_decimals` - `source_chain_decimals`)
-/// 3) If new_amount is greater than the destination chain's `max_uint`, the translation
+/// 3) If `destination_amount` is greater than the destination chain's `max_uint`, the translation
 /// fails.
-/// 4) If new_amount is zero, the translation fails.
+/// 4) If `destination_amount` is zero, the translation fails.
 fn destination_amount(
     storage: &dyn Storage,
     source_chain: &ChainNameRaw,
@@ -312,6 +312,8 @@ fn destination_amount(
             amount: source_amount,
         })?;
     let destination_amount = if source_decimals > destination_decimals {
+        // Note: We should track the truncated dust in the global token config to allow recovery in the future
+        // The token's decimals on the origin chain will always be greater than or equal to `source_decimals`, so we can scale and add to the total dust amount
         source_amount
             .checked_div(scaling_factor)
             .expect("scaling_factor must be non-zero")
@@ -325,7 +327,9 @@ fn destination_amount(
             })?
     };
 
+    // Note: Use ensure! instead of bail!?
     if destination_amount.gt(&destination_max_uint) {
+        // Note: shall we different error enums to make debugging easier? Maybe add scaling factor in the error as well?
         bail!(Error::InvalidTransferAmount {
             source_chain: source_chain.to_owned(),
             destination_chain: destination_chain.to_owned(),
@@ -366,7 +370,9 @@ fn destination_token_decimals(
     {
         source_chain_decimals
     } else {
-        source_chain_config
+        // Note: Since destination_chain's `max_uint` is less than source_chain's, we need to use `max_target_decimals` of the destination chain.
+        // E.g. Deploying token from Ethereum to Sui
+        destination_chain_config
             .max_target_decimals
             .min(source_chain_decimals)
     }
@@ -379,6 +385,8 @@ fn apply_transfer(
     destination_chain: ChainNameRaw,
     transfer: &InterchainTransfer,
 ) -> Result<InterchainTransfer, Error> {
+    // Note: We're loading the token instances multiple times, are we refactoring it to load first and save at the end?
+    // Same for chain config (checking frozen status and reading max_uint)
     let destination_amount = destination_amount(
         storage,
         &source_chain,

contracts/interchain-token-service/src/state.rs

@@ -73,6 +73,8 @@ impl TokenSupply {
 #[cw_serde]
 pub struct TokenInstance {
     pub supply: TokenSupply,
+    // Note: Since we have decided to drop deploy token manager msg type, we can save `u8` instead of `Option<u8>`
+    // When we add custom token linking, we'll include decimals in the msg type
     pub decimals: Option<u8>,
 }
 

contracts/interchain-token-service/tests/execute.rs

@@ -260,6 +260,7 @@ fn execute_message_interchain_transfer_should_scale_the_amount_when_source_decim
     );
 }
 
+// Note: we should add a test with 3 chains with distinct max uint values and try a round robin deploy + A -> B -> C transfer test
 #[test]
 fn execute_message_deploy_interchain_token_should_translate_decimals_when_max_uints_are_different()
 {