clean up code to follow best solidity practices:

- add docs to core contracts
- fix contract naming
- organize contract files

README.md

@@ -1,9 +1,39 @@
 # vIBC Core Smart Contracts
 
-This project includes
+This project includes the core smart contracts for the vIBC protocol, and a few demo contracts that simulate testing and serve as a template for integrating dapp devs.
 
-- vIBC Core Smart Contracts (CoreSC)
-- a few demo contracts that simulate dev users protocol contracts, eg. Mars, etc.
+![](./diagrams/vibcContractsOverview.jpg)
+
+
+## Repo Structure
+
+All contracts internal to this project are in the `contracts`. This directory contains the following subdirectories:
+- `core/`: The contracts that are core to the vIBC protocol. These are the contracts that will be deployed and used in production. 
+- `interfaces/`: Interfaces for core and testing contracts 
+- `libs/`: Libraries used by the core vIbc protocol.
+- The `utils/`, `base/`, and `example/` directories all contain contracts that are not core to the vIBC protocol. These contracts are used only for testing or as templates for dapp contracts that will integrate with the vIBC protocol. 
+
+
+# Core Contracts
+## Dispatcher
+The Dispatcher contract routes packets to dapps by mapping channels to dapp addresses. The dispatcher also calls packet and channel callbacks on sending/receiving dapps. This contract uses a UUPS proxy compliant with ERC1967 for upgrades. The proxy is intended to be managed by a single owner address, which will be replaced by a multisig in production. 
+
+Since the optimistic light client contract is used to prove that events happened on the Polymer chain, all methods in the dispatcher are permissionless, aside from the methods to set the connection to client mapping, upgrading the implementation, and setting the dispatcher's port prefix. 
+
+Due to the nature of ibc callbacks, the dispatcher should safely be able to integrate its handler methods with any arbitrary (i.e. potentially malicious) contracts. 
+
+The dispatcher can integrate with multiple light clients of the peptide chain to fit differing needs of security and ux. Currently, the dispatcher integrates with the DummyLightClient (used for testing to reduce the need for generating proofs), and the OptimisticLightClient - an EVM implementation of the Optimistic proof verification currently used by Optimism. Support for future clients is also possible. 
+
+Any dapps which send/receive Ibc packets can do so directly through the dispatcher, or through a middleware contract like the `UniversalChannelHandler`. Dapps that directly integrate with the dispatcher are expected to implement the `IbcReceiver` interface, and those which use the `UniversalChannelHandler` are assumed to implement the `IbcUniversalPacketReceiver` interface.
+
+## OptimisticLightClient
+The OptimisticLightClient contract abstracts away proof verification from the dispatcher contract. This light client represents a view of the Polymer chain, and is used to prove that channel handshake and packet sending events happened. 
+
+## OptimisticProofVerifier
+The optimisticProofVerifier verifies proofs for the optimistic light client. 
+
+## UniversalChannelHandler
+The UniversalChannelHandler is a middleware contract that can be used to save dapps from having to go through the 4-step channel handshake to send or receive Ibc packets. 
 
 ## Quick Start with Forge/Foundry
 
@@ -36,3 +66,4 @@ forge test
 ```sh
 forge clean
 ```
+

contracts/base/GeneralMiddleware.sol

@@ -2,7 +2,8 @@
 
 pragma solidity ^0.8.9;
 
-import {Ibc, IbcUtils, UniversalPacket, AckPacket} from "../libs/Ibc.sol";
+import {Ibc, UniversalPacket, AckPacket} from "../libs/Ibc.sol";
+import {IbcUtils} from "../libs/IbcUtils.sol";
 import {
     IbcUniversalPacketReceiver,
     IbcMwUser,
@@ -12,6 +13,12 @@ import {
     IbcMwPacketSender
 } from "../interfaces/IbcMiddleware.sol";
 
+/**
+ * @title GeneralMiddleware
+ *  @author Polymer Labs
+ *  @notice GeneralMiddleware is a starting point for developers to implement their own middleware logic. It is not
+ * intended to be directly deployed, but rather only used for testing and development
+ */
 contract GeneralMiddleware is IbcMwUser, IbcMiddleware, IbcMwEventsEmitter {
     /**
      * @dev MW_ID is the ID of MW contract on all supported virtual chains.

contracts/core/OpLightClient.sol → contracts/core/OptimisticLightClient.sol

@@ -1,35 +1,37 @@
 // SPDX-License-Identifier: UNLICENSED
-pragma solidity ^0.8.0;
+pragma solidity 0.8.15;
 
-import {L1Header, ProofVerifier, OpL2StateProof, Ics23Proof} from "../interfaces/ProofVerifier.sol";
-import {LightClient} from "../interfaces/LightClient.sol";
+import {L1Header, IProofVerifier, OpL2StateProof, Ics23Proof} from "../interfaces/IProofVerifier.sol";
+import {ILightClient} from "../interfaces/ILightClient.sol";
 import {L1Block} from "optimism/L2/L1Block.sol";
 
-// OptimisticLightClient manages the appHash at different
-// heights and track the fraud proof end time for them.
-contract OptimisticLightClient is LightClient {
+/**
+ * @title OptimisticLightClient
+ * @author Polymer Labs
+ * @dev This specific light client implementation uses the same client that is used in the op-stack
+ */
+contract OptimisticLightClient is ILightClient {
     // consensusStates maps from the height to the appHash.
     mapping(uint256 => uint256) public consensusStates;
 
     // fraudProofEndtime maps from the appHash to the fraud proof end time.
     mapping(uint256 => uint256) public fraudProofEndtime;
     uint256 public fraudProofWindowSeconds;
-    ProofVerifier public verifier;
+    IProofVerifier public verifier;
     L1Block public l1BlockProvider;
 
     error CannotUpdatePendingOptimisticConsensusState();
     error AppHashHasNotPassedFraudProofWindow();
 
-    constructor(uint32 fraudProofWindowSeconds_, ProofVerifier verifier_, L1Block _l1BlockProvider) {
+    constructor(uint32 fraudProofWindowSeconds_, IProofVerifier verifier_, L1Block _l1BlockProvider) {
         fraudProofWindowSeconds = fraudProofWindowSeconds_;
         verifier = verifier_;
         l1BlockProvider = _l1BlockProvider;
     }
 
-    // addOpConsensusState adds an appHash to internal store and
-    // returns the fraud proof end time, and a bool flag indicating if
-    // the fraud proof window has passed according to the block's
-    // timestamp.
+    /**
+     * @inheritdoc ILightClient
+     */
     function addOpConsensusState(
         L1Header calldata l1header,
         OpL2StateProof calldata proof,
@@ -60,17 +62,13 @@ contract OptimisticLightClient is LightClient {
     }
 
     /**
-     * getState returns the appHash at the given height, and the fraud
-     * proof end time.
-     * 0 is returned if there isn't an appHash with the given height.
+     * @inheritdoc ILightClient
      */
     function getState(uint256 height) external view returns (uint256 appHash, uint256 fraudProofEndTime, bool ended) {
         return getInternalState(height);
     }
     /**
-     * verifyMembership checks if the current trustedOptimisticConsensusState state
-     * can be used to perform the membership test and if so, it uses
-     * the verifier to perform membership check.
+     * @inheritdoc ILightClient
      */
 
     function verifyMembership(Ics23Proof calldata proof, bytes calldata key, bytes calldata expectedValue)
@@ -85,17 +83,26 @@ contract OptimisticLightClient is LightClient {
         verifier.verifyMembership(bytes32(appHash), key, expectedValue, proof);
     }
 
+    /**
+     * @inheritdoc ILightClient
+     */
     function verifyNonMembership(Ics23Proof calldata proof, bytes calldata key) external view {
         (uint256 appHash,, bool ended) = getInternalState(proof.height - 1);
         if (!ended) revert AppHashHasNotPassedFraudProofWindow();
         verifier.verifyNonMembership(bytes32(appHash), key, proof);
     }
 
+    /**
+     * @inheritdoc ILightClient
+     */
     function getFraudProofEndtime(uint256 height) external view returns (uint256 fraudProofEndTime) {
         uint256 hash = consensusStates[height];
         return fraudProofEndtime[hash];
     }
 
+    /**
+     * @dev Returns the internal state of the light client at a given height.
+     */
     function getInternalState(uint256 height)
         public
         view

contracts/core/OpProofVerifier.sol → contracts/core/OptimisticProofVerifier.sol

@@ -1,12 +1,17 @@
 // SPDX-License-Identifier: UNLICENSED
-pragma solidity ^0.8.0;
+pragma solidity 0.8.15;
 
 import {SecureMerkleTrie} from "optimism/libraries/trie/SecureMerkleTrie.sol";
 import {RLPReader} from "optimism/libraries/rlp/RLPReader.sol";
 import {RLPWriter} from "optimism/libraries/rlp/RLPWriter.sol";
-import {ProofVerifier, L1Header, OpL2StateProof, Ics23Proof, OpIcs23Proof} from "../interfaces/ProofVerifier.sol";
-
-contract OpProofVerifier is ProofVerifier {
+import {IProofVerifier, L1Header, OpL2StateProof, Ics23Proof, OpIcs23Proof} from "../interfaces/IProofVerifier.sol";
+
+/**
+ * @title OptimisticProofVerifier
+ * @notice Verifies proofs related to Optimistic Rollup state updates
+ * @author Polymer Labs
+ */
+contract OptimisticProofVerifier is IProofVerifier {
     using RLPReader for RLPReader.RLPItem;
     using RLPReader for bytes;
 
@@ -62,12 +67,12 @@ contract OpProofVerifier is ProofVerifier {
             revert InvalidL1BlockNumber();
         }
 
-        // this computes the L1 header hash
+        // This computes the L1 header hash
         if (trustedL1BlockHash != keccak256(RLPWriter.writeList(l1header.header))) {
             revert InvalidL1BlockHash();
         }
 
-        // these two checks are here to verify that the "plain" (i.e. not RLP encoded) values in the l1header are
+        // These two checks are here to verify that the "plain" (i.e. not RLP encoded) values in the l1header are
         // the same ones found in l1header.header (i.e. RLP encoded). This is because it is cheaper to RLP
         // encode that decode
         if (keccak256(RLPWriter.writeUint(l1header.number)) != keccak256(l1header.header[_L1_NUMBER_INDEX])) {
@@ -95,7 +100,7 @@ contract OpProofVerifier is ProofVerifier {
             abi.encode(proof.l2OutputProposalKey), proof.outputRootProof, bytes32(bytes(stateAccount[2].readBytes()))
         );
 
-        // now that the output root is verified, we need to verify the app hash. To do so we try to derive the
+        // Now that the output root is verified, we need to verify the app hash. To do so we try to derive the
         // the output root the same way the proposer did.
         // See https://github.com/polymerdao/optimism/blob/polymer/v1.2.0/op-service/eth/output.go#L44
         if (
@@ -112,11 +117,16 @@ contract OpProofVerifier is ProofVerifier {
         }
     }
 
+    /**
+     * @dev Prove that a given state is not part of a proof
+     * @dev this method is mainly used for packet timeouts, which is currently not implemented
+     */
     function verifyNonMembership(bytes32, bytes calldata, Ics23Proof calldata) external pure {
         revert MethodNotImplemented();
     }
 
     /**
+     * @inheritdoc IProofVerifier
      * @dev verifies a chain of ICS23 proofs
      * Each computed subroot starting from index 0 must match the value of the next proof (hence chained proofs).
      * The cosmos SDK and ics23 support chained proofs to switch between different proof specs.
@@ -138,8 +148,13 @@ contract OpProofVerifier is ProofVerifier {
         if (appHash != _verify(proofs.proof[1])) revert InvalidIbcStateProof();
     }
 
-    // this code was adapted from the ICS23 membership verification found here:
-    // https://github.com/cosmos/ics23/blob/go/v0.10.0/go/ics23.go#L36
+    /**
+     * @dev Verifies an ICS23 proof through the root hash based on the provided proof.
+     * @dev This code was adapted from the ICS23 membership verification found here:
+     * https://github.com/cosmos/ics23/blob/go/v0.10.0/go/ics23.go#L36
+     * @param proof The ICS23 proof to be verified.
+     * @return computed The computed root hash.
+     */
     function _verify(OpIcs23Proof calldata proof) internal pure returns (bytes32 computed) {
         bytes32 hashedData = sha256(proof.value);
         computed = sha256(
@@ -153,6 +168,11 @@ contract OpProofVerifier is ProofVerifier {
         }
     }
 
+    /**
+     * @dev Encodes an integer value into a variable-length integer format.
+     * @param value The integer value to be encoded.
+     * @return encoded The encoded bytes array.
+     */
     function _encodeVarint(uint256 value) internal pure returns (bytes memory encoded) {
         bytes memory result;
         while (value >= 0x80) {

contracts/core/UniversalChannelHandler.sol

@@ -1,6 +1,6 @@
 //SPDX-License-Identifier: UNLICENSED
 
-pragma solidity ^0.8.9;
+pragma solidity 0.8.15;
 
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 import {IbcDispatcher} from "../interfaces/IbcDispatcher.sol";
@@ -13,9 +13,17 @@ import {
 } from "../interfaces/IbcMiddleware.sol";
 import {IbcReceiver} from "../interfaces/IbcReceiver.sol";
 import {IbcReceiverBaseUpgradeable} from "../interfaces/IbcReceiverUpgradeable.sol";
-import {ChannelOrder, ChannelEnd, IbcPacket, AckPacket, UniversalPacket, IbcUtils} from "../libs/Ibc.sol";
+import {ChannelOrder, ChannelEnd, IbcPacket, AckPacket, UniversalPacket} from "../libs/Ibc.sol";
+import {IbcUtils} from "../libs/IbcUtils.sol";
 import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
 
+/**
+ * @title Universal Channel Handler
+ * @author Polymer Labs
+ * @notice Implements universal channels for virtual IBC. Universal channels prevent dapps from needing to do a 4-step
+ * channel handshake to establish a channel.
+ * @dev This contract can integrate directly with dapps, or a middleware stack for packet routing.
+ */
 contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable, IbcUniversalChannelMW {
     uint256[49] private __gap;
 
@@ -60,6 +68,13 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
         dispatcher.channelOpenInit(version, ordering, feeEnabled, connectionHops, counterpartyPortIdentifier);
     }
 
+    /**
+     * @notice Sends a universal packet over an IBC channel
+     * @param channelId The channel ID through which the packet is sent on the dispatcher
+     * @param destPortAddr The destination port address
+     * @param appData The packet data to be sent
+     * @param timeoutTimestamp of when the packet can timeout
+     */
     function sendUniversalPacket(
         bytes32 channelId,
         bytes32 destPortAddr,
@@ -73,7 +88,15 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
         dispatcher.sendPacket(channelId, packetData, timeoutTimestamp);
     }
 
-    // called by another IBC middleware; pack packet and send over to Dispatcher
+    /**
+     * @notice Sends a middleware packet over an IBC channel. This is intended to be called by another middleware
+     * contract, rather than an end Dapp itself.
+     * @param channelId The channel ID through which the packet is sent on the dispatcher
+     * @param destPortAddr The destination port address
+     * @param srcMwIds The mwId bitmap of the middleware stack
+     * @param appData The packet data to be sent
+     * @param timeoutTimestamp of when the packet can timeout
+     */
     function sendMWPacket(
         bytes32 channelId,
         // original source address of the packet
@@ -89,6 +112,11 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
         dispatcher.sendPacket(channelId, packetData, timeoutTimestamp);
     }
 
+    /**
+     * @notice Handles the reception of an IBC packet from the counterparty
+     * @param packet The received IBC packet
+     * @return ackPacket The packet acknowledgement
+     */
     function onRecvPacket(IbcPacket calldata packet)
         external
         override
@@ -108,6 +136,11 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
         }
     }
 
+    /**
+     * @notice Handles acknowledging the reception of an acknowledgement packet by the counterparty
+     * @param packet The IBC packet to be acknowledged
+     * @param ack The packet acknowledgement
+     */
     function onAcknowledgementPacket(IbcPacket calldata packet, AckPacket calldata ack)
         external
         override
@@ -126,6 +159,10 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
         }
     }
 
+    /**
+     * @notice Handles the timeout event for an IBC packet
+     * @param packet The IBC packet that has timed out
+     */
     function onTimeoutPacket(IbcPacket calldata packet) external override onlyIbcDispatcher {
         UniversalPacket memory ucPacketData = IbcUtils.fromUniversalPacketBytes(packet.data);
         address[] storage mwAddrs = mwStackAddrs[ucPacketData.mwBitmap];
@@ -185,6 +222,12 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
     }
 
     // IBC callback functions
+    /**
+     * @notice Handles the acknowledgment of channel opening.
+     * This function is accessible only by the IBC dispatcher.
+     * @param channelId The channel ID of the opened channel
+     * @param counterpartyVersion The version string provided by the counterparty
+     */
     function onChanOpenAck(bytes32 channelId, bytes32, string calldata counterpartyVersion)
         external
         view
@@ -206,6 +249,11 @@ contract UniversalChannelHandler is IbcReceiverBaseUpgradeable, UUPSUpgradeable,
         return version;
     }
 
+    /**
+     * @dev Internal function to open a channel only if the version matches what is expected.
+     * @param version The version string provided by the counterparty
+     * @return selectedVersion The selected version string
+     */
     function _openChannel(string calldata version) internal pure returns (string memory selectedVersion) {
         if (keccak256(abi.encodePacked(version)) != keccak256(abi.encodePacked(VERSION))) {
             revert UnsupportedVersion();

contracts/examples/Earth.sol

@@ -2,9 +2,16 @@
 
 pragma solidity ^0.8.9;
 
-import {IbcUtils, UniversalPacket, AckPacket} from "../libs/Ibc.sol";
+import {UniversalPacket, AckPacket} from "../libs/Ibc.sol";
+import {IbcUtils} from "../libs/IbcUtils.sol";
 import {IbcMwUser, IbcUniversalPacketReceiver, IbcUniversalPacketSender} from "../interfaces/IbcMiddleware.sol";
 
+/**
+ * @title Earth
+ * @notice Earth is a simple IBC receiver contract that receives packets and sends acks.
+ * @dev This contract is used for only testing IBC functionality and as an example for dapp developers on how to
+ * integrate with the vibc protocol.
+ */
 contract Earth is IbcMwUser, IbcUniversalPacketReceiver {
     struct UcPacketWithChannel {
         bytes32 channelId;