📝 Ensure Skip Package Is Well Commented (#52)

* 📝 Ensure all package types and methods are commented for purpose

* 📝 Add changelog entry

CHANGELOG.md

@@ -32,6 +32,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 # Changelog
 
+## Unreleased
+
+### Minor Improvements
+- [#52](https://github.com/skip-mev/skip-api-contracts/pull/52) Ensure Skip package types, methods, and functions are well documented.
+
 ## [v0.2.0](https://github.com/skip-mev/skip-api-contracts/releases/tag/v0.2.0) - 2023-08-03
 
 [Tracking](https://github.com/skip-mev/skip-api-contracts/issues/28)

packages/skip/README.md

@@ -0,0 +1,4 @@
+# Skip: Common Types and Functions
+
+This Skip packages folder contains common types and functions that are used across multiple Skip API contracts.
+

packages/skip/src/entry_point.rs

@@ -1,15 +1,16 @@
-use cosmwasm_schema::{cw_serde, QueryResponses};
-use cosmwasm_std::{Binary, Coin, Uint128};
-
 use crate::{
     ibc::IbcInfo,
     swap::{Swap, SwapExactCoinOut, SwapVenue},
 };
 
+use cosmwasm_schema::{cw_serde, QueryResponses};
+use cosmwasm_std::{Binary, Coin, Uint128};
+
 ///////////////////
 /// INSTANTIATE ///
 ///////////////////
 
+// The InstantiateMsg struct defines the initialization parameters for the entry point contract.
 #[cw_serde]
 pub struct InstantiateMsg {
     pub swap_venues: Vec<SwapVenue>,
@@ -20,6 +21,8 @@ pub struct InstantiateMsg {
 /// EXECUTE ///
 ///////////////
 
+// The ExecuteMsg enum defines the execution messages that the entry point contract can handle.
+// Only the SwapAndAction message is callable by external users.
 #[cw_serde]
 #[allow(clippy::large_enum_variant)]
 pub enum ExecuteMsg {
@@ -49,6 +52,7 @@ pub enum ExecuteMsg {
 /// QUERY ///
 /////////////
 
+// The QueryMsg enum defines the queries the entry point contract provides.
 #[cw_serde]
 #[derive(QueryResponses)]
 pub enum QueryMsg {
@@ -67,6 +71,7 @@ pub enum QueryMsg {
 /// COMMON TYPES ///
 ////////////////////
 
+// The Action enum is used to specify what action to take after a swap.
 #[cw_serde]
 pub enum Action {
     BankSend {
@@ -81,6 +86,8 @@ pub enum Action {
     },
 }
 
+// The Affiliate struct is used to specify an affiliate address and BPS fee taken
+// from the min_coin to send to that address.
 #[cw_serde]
 pub struct Affiliate {
     pub basis_points_fee: Uint128,

packages/skip/src/error.rs

@@ -3,14 +3,20 @@ use thiserror::Error;
 
 #[derive(Error, Debug, PartialEq)]
 pub enum SkipError {
-    // GENERAL
+    ///////////////
+    /// GENERAL ///
+    ///////////////
+
     #[error(transparent)]
     Std(#[from] StdError),
 
     #[error("Unauthorized")]
     Unauthorized,
 
-    // SWAP OPERATIONS
+    ////////////
+    /// SWAP ///
+    ////////////
+
     #[error("Swap Operations Empty")]
     SwapOperationsEmpty,
 
@@ -20,7 +26,10 @@ pub enum SkipError {
     #[error("Last Swap Operations' Denom Out Differs From Swap Coin Out Denom")]
     SwapOperationsCoinOutDenomMismatch,
 
-    // IBC FEES
+    ///////////
+    /// IBC ///
+    ///////////
+
     #[error("Ibc Fees Are Not A Single Coin, Either Multiple Denoms Or No Coin Specified")]
     IbcFeesNotOneCoin,
 }

packages/skip/src/ibc.rs

@@ -1,13 +1,16 @@
 use crate::{error::SkipError, proto_coin::ProtoCoin};
+
+use std::convert::From;
+
 use cosmwasm_schema::{cw_serde, QueryResponses};
 use cosmwasm_std::{Coin, Coins, StdError};
 use neutron_proto::neutron::feerefunder::Fee as NeutronFee;
-use std::convert::From;
 
 ///////////////////
 /// INSTANTIATE ///
 ///////////////////
 
+// The InstantiateMsg struct defines the initialization parameters for the IBC Transfer Adapter contracts.
 #[cw_serde]
 pub struct InstantiateMsg {
     pub entry_point_contract_address: String,
@@ -17,6 +20,7 @@ pub struct InstantiateMsg {
 /// EXECUTE ///
 ///////////////
 
+// The ExecuteMsg enum defines the execution message that the IBC Transfer Adapter contracts can handle.
 #[cw_serde]
 pub enum ExecuteMsg {
     IbcTransfer {
@@ -30,6 +34,9 @@ pub enum ExecuteMsg {
 /// QUERY ///
 /////////////
 
+// TODO: Consolidate into one enum now that the return types are the same
+
+// The NeutronQueryMsg enum defines the queries the Neutron IBC Transfer Adapter Contract provides.
 #[cw_serde]
 #[derive(QueryResponses)]
 pub enum NeutronQueryMsg {
@@ -40,6 +47,7 @@ pub enum NeutronQueryMsg {
     },
 }
 
+// The OsmosisQueryMsg enum defines the queries the Osmosis IBC Transfer Adapter Contract provides.
 #[cw_serde]
 #[derive(QueryResponses)]
 pub enum OsmosisQueryMsg {
@@ -54,6 +62,7 @@ pub enum OsmosisQueryMsg {
 /// COMMON TYPES ///
 ////////////////////
 
+// The IbcFee struct defines the fees for an IBC transfer standardized across all IBC Transfer Adapter contracts.
 #[cw_serde]
 #[derive(Default)]
 pub struct IbcFee {
@@ -62,7 +71,7 @@ pub struct IbcFee {
     pub timeout_fee: Vec<Coin>,
 }
 
-// Converts an IbcFee struct to a neutron_proto Fee
+// Converts an IbcFee struct to a neutron_proto::neutron::feerefunder Fee
 impl From<IbcFee> for NeutronFee {
     fn from(ibc_fee: IbcFee) -> Self {
         NeutronFee {
@@ -85,7 +94,8 @@ impl From<IbcFee> for NeutronFee {
     }
 }
 
-// Converts an IbcFee struct to a Coins struct
+// Converts an IbcFee struct to a cosmwasm_std::Coins struct
+// Must be TryFrom since adding the ibc_fees can overflow.
 impl TryFrom<IbcFee> for Coins {
     type Error = StdError;
 
@@ -102,6 +112,12 @@ impl TryFrom<IbcFee> for Coins {
 }
 
 impl IbcFee {
+    // one_coin aims to mimic the behavior of cw_utls::one_coin,
+    // returing the single coin in the IbcFee struct if it exists,
+    // erroring if 0 or more than 1 coins exist.
+    //
+    // one_coin is used because the entry_point contract only supports
+    // the handling of a single denomination for IBC fees.
     pub fn one_coin(&self) -> Result<Coin, SkipError> {
         let ibc_fees_map: Coins = self.clone().try_into()?;
 
@@ -113,6 +129,7 @@ impl IbcFee {
     }
 }
 
+// The IbcInfo struct defines the information for an IBC transfer standardized across all IBC Transfer Adapter contracts.
 #[cw_serde]
 pub struct IbcInfo {
     pub source_channel: String,
@@ -122,13 +139,15 @@ pub struct IbcInfo {
     pub recover_address: String,
 }
 
+// The IbcTransfer struct defines the parameters for an IBC transfer standardized across all IBC Transfer Adapter contracts.
 #[cw_serde]
 pub struct IbcTransfer {
     pub info: IbcInfo,
     pub coin: Coin,
     pub timeout_timestamp: u64,
 }
 
+// Converts an IbcTransfer struct to an ExecuteMsg::IbcTransfer enum
 impl From<IbcTransfer> for ExecuteMsg {
     fn from(ibc_transfer: IbcTransfer) -> Self {
         ExecuteMsg::IbcTransfer {
@@ -144,6 +163,9 @@ impl From<IbcTransfer> for ExecuteMsg {
 // ibc transfer upon receiving a successful sub msg reply.
 pub type AckID<'a> = (&'a str, u64);
 
+// The IbcLifecycleComplete enum defines the possible sudo messages that the
+// ibc transfer adapter contract on ibc-hook enabled chains can expect to received
+// from the ibc-hooks module.
 #[cw_serde]
 pub enum IbcLifecycleComplete {
     IbcAck {

packages/skip/src/proto_coin.rs

@@ -1,6 +1,5 @@
-use cosmwasm_schema::cw_serde;
-
 use cosmos_sdk_proto::cosmos::base::v1beta1::Coin as CosmosSdkCoin;
+use cosmwasm_schema::cw_serde;
 use ibc_proto::cosmos::base::v1beta1::Coin as IbcCoin;
 use osmosis_std::types::cosmos::base::v1beta1::Coin as OsmosisStdCoin;
 

packages/skip/src/sudo.rs

@@ -1,4 +1,5 @@
 use crate::ibc::IbcLifecycleComplete;
+
 use cosmwasm_schema::cw_serde;
 
 // SudoType used to give info in response attributes when the sudo function is called

packages/skip/src/swap.rs

@@ -1,24 +1,32 @@
 use crate::error::SkipError;
+
+use std::{
+    convert::{From, TryFrom},
+    num::ParseIntError,
+};
+
 use astroport::{asset::AssetInfo, router::SwapOperation as AstroportSwapOperation};
 use cosmwasm_schema::{cw_serde, QueryResponses};
 use cosmwasm_std::{Addr, BankMsg, Coin, DepsMut, Env, MessageInfo, Response};
 use osmosis_std::types::osmosis::poolmanager::v1beta1::{
     SwapAmountInRoute as OsmosisSwapAmountInRoute, SwapAmountOutRoute as OsmosisSwapAmountOutRoute,
 };
-use std::{
-    convert::{From, TryFrom},
-    num::ParseIntError,
-};
 
 ///////////////////
 /// INSTANTIATE ///
 ///////////////////
 
+// The OsmosisInstantiateMsg struct defines the initialization parameters for the
+// Osmosis Poolmanager swap adapter contract.
 #[cw_serde]
 pub struct OsmosisInstantiateMsg {
     pub entry_point_contract_address: String,
 }
 
+// TODO: Change to AstroportInstantiateMsg as part of restructuring
+
+// The NeutronInstantiateMsg struct defines the initialization parameters for the
+// Neutron Astroport swap adapter contract.
 #[cw_serde]
 pub struct NeutronInstantiateMsg {
     pub entry_point_contract_address: String,
@@ -29,12 +37,16 @@ pub struct NeutronInstantiateMsg {
 ///      EXECUTE      ///
 /////////////////////////
 
+// The ExecuteMsg enum defines the execution message that the swap adapter contracts can handle.
+// Only the Swap message is callable by external users.
 #[cw_serde]
 pub enum ExecuteMsg {
     Swap { operations: Vec<SwapOperation> },
     TransferFundsBack { swapper: Addr },
 }
 
+// Converts a SwapExactCoinIn used in the entry point contract
+// to a swap adapter Swap execute message
 impl From<SwapExactCoinIn> for ExecuteMsg {
     fn from(swap: SwapExactCoinIn) -> Self {
         ExecuteMsg::Swap {
@@ -43,6 +55,8 @@ impl From<SwapExactCoinIn> for ExecuteMsg {
     }
 }
 
+// Converts a SwapExactCoinOut used in the entry point contract
+// to a swap adapter Swap execute message
 impl From<SwapExactCoinOut> for ExecuteMsg {
     fn from(swap: SwapExactCoinOut) -> Self {
         ExecuteMsg::Swap {
@@ -55,6 +69,8 @@ impl From<SwapExactCoinOut> for ExecuteMsg {
 ///       QUERY       ///
 /////////////////////////
 
+// The QueryMsg enum defines the queries the swap adapter contracts provide.
+// RouterContractAddress is only implemented for Astroport swap adapter contracts.
 #[cw_serde]
 #[derive(QueryResponses)]
 pub enum QueryMsg {