refine code comments (#14)

contracts/CelerLedger.sol

@@ -13,6 +13,12 @@ import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
 /**
  * @title CelerLedger wrapper contract
  * @notice A wrapper contract using libraries to provide CelerLedger's APIs.
+ * @notice Ownable contract and LedgerBalanceLimit library should only be used
+ *   in the initial stage of the mainnet operation for a very short period of
+ *   time to limit the balance amount that can be deposit into each channel,
+ *   so that any losses due to unknown bugs (if any) will be limited. The balance
+ *   limits should be disabled and the owner account of CelerLedger should renounce
+ *   its ownership after the system is stable and comprehensively audited.
  */
 contract CelerLedger is ICelerLedger, Ownable {
     using LedgerOperation for LedgerStruct.Ledger;
@@ -31,14 +37,14 @@ contract CelerLedger is ICelerLedger, Ownable {
         ledger.ethPool = IEthPool(_ethPool);
         ledger.payRegistry = IPayRegistry(_payRegistry);
         ledger.celerWallet = ICelerWallet(_celerWallet);
-        // enable deposit limits in default
+        // enable balance limits in default
         ledger.balanceLimitsEnabled = true;
     }
 
     /**
-     * @notice Set the deposit limits of given tokens
+     * @notice Set the per-channel balance limits of given tokens
      * @param _tokenAddrs addresses of the tokens (address(0) is for ETH)
-     * @param _limits deposit limits of the tokens
+     * @param _limits balance limits of the tokens
      */
     function setBalanceLimits(
         address[] calldata _tokenAddrs,
@@ -51,14 +57,14 @@ contract CelerLedger is ICelerLedger, Ownable {
     }
 
     /**
-     * @notice Disable deposit limits of all tokens
+     * @notice Disable balance limits of all tokens
      */
     function disableBalanceLimits() external onlyOwner {
         ledger.disableBalanceLimits();
     }
 
     /**
-     * @notice Enable deposit limits of all tokens
+     * @notice Enable balance limits of all tokens
      */
     function enableBalanceLimits() external onlyOwner {
         ledger.enableBalanceLimits();
@@ -161,7 +167,7 @@ contract CelerLedger is ICelerLedger, Ownable {
     }
 
     /**
-     * @notice Cooperatively withdraw specific amount of deposit
+     * @notice Cooperatively withdraw specific amount of balance
      * @param _cooperativeWithdrawRequest bytes of cooperative withdraw request message
      */
     function cooperativeWithdraw(bytes calldata _cooperativeWithdrawRequest) external {
@@ -199,7 +205,7 @@ contract CelerLedger is ICelerLedger, Ownable {
 
     /**
      * @notice Confirm channel settlement
-     * @dev This must be alled after settleFinalizedTime
+     * @dev This must be called after settleFinalizedTime
      * @param _channelId ID of the channel
      */
     function confirmSettle(bytes32 _channelId) external {
@@ -326,7 +332,7 @@ contract CelerLedger is ICelerLedger, Ownable {
      * @param _channelId ID of the channel to be viewed
      * @return peers' addresses
      * @return peers' deposits
-     * @return peers' owedDeposits
+     * @return peers' withdrawals
      * @return peers' state sequence numbers
      * @return peers' transferOut map
      * @return peers' pendingPayOut map
@@ -480,9 +486,9 @@ contract CelerLedger is ICelerLedger, Ownable {
     }
 
     /**
-     * @notice Return deposit limit of given token
+     * @notice Return balance limit of given token
      * @param _tokenAddr query token address
-     * @return token deposit limit
+     * @return token balance limit
      */
     function getBalanceLimit(address _tokenAddr) external view returns(uint) {
         return ledger.getBalanceLimit(_tokenAddr);

contracts/CelerWallet.sol

@@ -11,6 +11,13 @@ import "openzeppelin-solidity/contracts/lifecycle/Pausable.sol";
  * @notice A multi-owner, multi-token, operator-centric wallet designed for CelerChannel.
  *   This wallet can run independetly and doesn't rely on trust of any external contracts
  *   even CelerLedger to maximize its security.
+ * @notice Pausable contract and drainToken() function should only be used for handling
+ *   unexpected emergencies in the initial stage of the mainnet operation for a very short
+ *   period of time. The pauser accounts should only call pause() and drainToken() functions
+ *   when some fatal bugs or crucial errors happen in order to ensure the safety of the
+ *   funds stored in CelerWallet. After the system is stable and comprehensively audited,
+ *   all pauser accounts should renounce their pauser roles so that no one will ever be able
+ *   to pause() or drainToken() anymore.
  */
 contract CelerWallet is ICelerWallet, Pausable {
     using SafeMath for uint;
@@ -217,7 +224,7 @@ contract CelerWallet is ICelerWallet, Pausable {
 
     /**
      * @notice Pauser drains one type of tokens when paused
-     * @dev This is for emergency situations.
+     * @notice This is only for emergent situations.
      * @param _tokenAddress address of token to drain
      * @param _receiver token receiver
      * @param _amount drained token amount

contracts/lib/ledgerlib/LedgerBalanceLimit.sol

@@ -8,7 +8,7 @@ import "./LedgerStruct.sol";
  */
 library LedgerBalanceLimit {
     /**
-     * @notice Set the balance limits of given tokens
+     * @notice Set the per-channel balance limits of given tokens
      * @param _self storage data of CelerLedger contract
      * @param _tokenAddrs addresses of the tokens (address(0) is for ETH)
      * @param _limits balance limits of the tokens

contracts/lib/ledgerlib/LedgerOperation.sol

@@ -293,7 +293,13 @@ library LedgerOperation {
         bytes32 recipientChannelId = c.withdrawIntent.recipientChannelId;
         delete c.withdrawIntent;
 
-        // don't need to check balance because intendWithdraw() has already checked withdraw limit
+        // NOTE: for safety reasons, from offchain point of view, only one pending withdraw (including
+        //   both cooperative ones and noncooperative ones) should be allowed at any given time.
+        //   Also note that snapshotStates between an intendWithdraw and a confirmWithdraw won't update
+        //   the withdraw limit calculated in the intendWithdraw.
+        // TODO: move withdrawLimit check from intendWithdraw() to here to check withdraw limit
+        //   with latest states. Yet there are no security issues because CelerWallet will check
+        //   the total balance anyways.
         // this implicitly require receiver be a peer
         c._addWithdrawal(receiver, amount, false);
 
@@ -322,7 +328,7 @@ library LedgerOperation {
     }
 
     /**
-     * @notice Cooperatively withdraw specific amount of deposit
+     * @notice Cooperatively withdraw specific amount of balance
      * @param _self storage data of CelerLedger contract
      * @param _cooperativeWithdrawRequest bytes of cooperative withdraw request message
      */
@@ -491,7 +497,7 @@ library LedgerOperation {
 
     /**
      * @notice Confirm channel settlement
-     * @dev This must be alled after settleFinalizedTime
+     * @dev This must be called after settleFinalizedTime
      * @param _self storage data of CelerLedger contract
      * @param _channelId ID of the channel
      */
@@ -510,10 +516,13 @@ library LedgerOperation {
 
         // require channel status of current intendSettle has been finalized,
         // namely all payments have already been either cleared or expired
-        // TODO: here we should use (lastPayResolveDeadline + clear safe margin)
-        //   instead of lastPayResolveDeadline to avoid race condition between clearPays
-        //   and confirmSettle, which may lead to different settle balance. Add this safe
-        //   margin to the value of lastPayResolveDeadline for now as a temporary solution.
+        // Note: this lastPayResolveDeadline should use
+        //   (the actual last resolve deadline of all pays + clearPays safe margin)
+        //   to ensure that peers have enough time to clearPays before confirmSettle.
+        //   However this only matters if there are multiple blocks of pending pay list
+        //   i.e. the nextPayIdListHash after intendSettle is not bytes32(0).
+        // TODO: add an additional clearSafeMargin param or change the semantics of
+        //   lastPayResolveDeadline to also include clearPays safe margin and rename it.
         require(
             (peerProfiles[0].state.nextPayIdListHash == bytes32(0) ||
                 blockNumber > peerProfiles[0].state.lastPayResolveDeadline) &&

contracts/lib/ledgerlib/LedgerStruct.sol

@@ -63,9 +63,9 @@ library LedgerStruct {
         IEthPool ethPool;
         IPayRegistry payRegistry;
         ICelerWallet celerWallet;
-        // per channel deposit limits for different tokens
+        // per-channel balance limits for different tokens
         mapping(address => uint) balanceLimits;
-        // whether deposit limits of all tokens have been enabled
+        // whether balance limits of all tokens have been enabled
         bool balanceLimitsEnabled;
         mapping(bytes32 => Channel) channelMap;
     }