Improve the coding style according to OPStack's guide

README.md

@@ -1,5 +1,8 @@
 # EthStorage Decentralized Storage Contracts V1
 
+## Style Guide
+Smart contracts should be written according to this [STYLE_GUIDE.md](https://github.com/ethstorage/optimism/blob/develop/packages/contracts-bedrock/STYLE_GUIDE.md)
+
 ## How to verify the contract
 
 - Implementation: npx hardhat verify --network sepolia <contract address>

contracts/DecentralizedKV.sol

@@ -2,94 +2,125 @@
 pragma solidity ^0.8.0;
 
 import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
-import "./MerkleLib.sol";
-import "./BinaryRelated.sol";
+import "./libraries/MerkleLib.sol";
+import "./libraries/BinaryRelated.sol";
 
+/// @custom:upgradeable
+/// @title DecentralizedKV
+/// @notice The DecentralizedKV is a top base contract for the EthStorage contract. It provides the
+///         basic key-value store functionalities.
 contract DecentralizedKV is OwnableUpgradeable {
-    event Remove(uint256 indexed kvIdx, uint256 indexed kvEntryCount);
+    /// @notice Represents the metadata of the key-value .
+    /// @custom:field kvIdx  Internal address seeking.
+    /// @custom:field kvSize BLOB size.
+    /// @custom:field hash   Commitment.
+    struct PhyAddr {
+        uint40 kvIdx;
+        uint24 kvSize;
+        bytes24 hash;
+    }
 
+    /// @notice Enum representing different decoding types when getting key-value.
+    /// @custom:field RawData           Don't do any decoding.
+    /// @custom:field PaddingPer31Bytes Will remove the padding byte every 31 bytes.
     enum DecodeType {
         RawData,
         PaddingPer31Bytes
     }
 
-    uint256 public immutable storageCost; // Upfront storage cost (pre-dcf)
-    // Discounted cash flow factor in seconds
-    // E.g., 0.85 yearly discount in second = 0.9999999948465585 = 340282365167313208607671216367074279424 in Q128.128
-    uint256 public immutable dcfFactor;
-    uint256 public immutable startTime;
-    uint256 public immutable maxKvSize;
+    /// @notice Upfront storage cost (pre-dcf)
+    uint256 internal immutable STORAGE_COST;
+
+    /// @notice Discounted cash flow factor in seconds
+    ///         E.g., 0.85 yearly discount in second = 0.9999999948465585 = 340282365167313208607671216367074279424 in Q128.128
+    uint256 internal immutable DCF_FACTOR;
 
+    /// @notice The start time of the storage payment
+    uint256 internal immutable START_TIME;
+
+    /// @notice Maximum size of a single key-value pair
+    uint256 internal immutable MAX_KV_SIZE;
+
+    /// @custom:legacy
     /// @custom:spacer storageCost, dcfFactor, startTime, maxKvSize
     /// @notice Spacer for backwards compatibility.
-    uint256[4] public kvSpacers;
-
-    uint40 public kvEntryCount; // number of entries in the store
+    uint256[4] private kvSpacers;
 
-    struct PhyAddr {
-        /* Internal address seeking */
-        uint40 kvIdx;
-        /* BLOB size */
-        uint24 kvSize;
-        /* Commitment */
-        bytes24 hash;
-    }
+    /// @notice The number of entries in the store
+    uint40 public kvEntryCount;
 
-    /* skey - PhyAddr */
+    /// @notice skey and PhyAddr mapping
     mapping(bytes32 => PhyAddr) internal kvMap;
-    /* index - skey, reverse lookup */
+
+    /// @notice index and skey mapping, reverse lookup
     mapping(uint256 => bytes32) internal idxMap;
 
+    /// @notice Emitted when a key-value is removed.
+    /// @param kvIdx        The removed key-value index.
+    /// @param kvEntryCount The key-value entry count after removing the kvIdx.
+    event Remove(uint256 indexed kvIdx, uint256 indexed kvEntryCount);
+
     // TODO: Reserve extra slots (to a total of 50?) in the storage layout for future upgrades
 
+    /// @notice Constructs the DecentralizedKV contract. Initializes the immutables.
     constructor(uint256 _maxKvSize, uint256 _startTime, uint256 _storageCost, uint256 _dcfFactor) {
-        maxKvSize = _maxKvSize;
-        startTime = _startTime;
-        storageCost = _storageCost;
-        dcfFactor = _dcfFactor;
+        MAX_KV_SIZE = _maxKvSize;
+        START_TIME = _startTime;
+        STORAGE_COST = _storageCost;
+        DCF_FACTOR = _dcfFactor;
     }
 
-    function __init_KV(address _owner) public onlyInitializing {
+    /// @notice Initializer.
+    /// @param _owner The contract owner.
+    function __init_KV(address _owner) internal onlyInitializing {
         __Context_init();
         __Ownable_init(_owner);
         kvEntryCount = 0;
     }
 
-    function pow(uint256 fp, uint256 n) internal pure returns (uint256) {
-        return BinaryRelated.pow(fp, n);
+    /// @notice Pow function in Q128.
+    function _pow(uint256 _fp, uint256 _n) internal pure returns (uint256) {
+        return BinaryRelated.pow(_fp, _n);
     }
 
-    // Evaluate payment from [t0, t1) seconds
-    function _paymentInInterval(uint256 x, uint256 t0, uint256 t1) internal view returns (uint256) {
-        return (x * (pow(dcfFactor, t0) - pow(dcfFactor, t1))) >> 128;
+    /// @notice Evaluate payment from [t0, t1) seconds
+    function _paymentInInterval(uint256 _x, uint256 _t0, uint256 _t1) internal view returns (uint256) {
+        return (_x * (_pow(DCF_FACTOR, _t0) - _pow(DCF_FACTOR, _t1))) >> 128;
     }
 
-    // Evaluate payment from [t0, \inf).
-    function _paymentInf(uint256 x, uint256 t0) internal view returns (uint256) {
-        return (x * pow(dcfFactor, t0)) >> 128;
+    /// @notice Evaluate payment from [t0, \inf).
+    function _paymentInf(uint256 _x, uint256 _t0) internal view returns (uint256) {
+        return (_x * _pow(DCF_FACTOR, _t0)) >> 128;
     }
 
-    // Evaluate payment from timestamp [fromTs, toTs)
-    function _paymentIn(uint256 x, uint256 fromTs, uint256 toTs) internal view returns (uint256) {
-        return _paymentInInterval(x, fromTs - startTime, toTs - startTime);
+    /// @notice Evaluate payment from timestamp [fromTs, toTs)
+    function _paymentIn(uint256 _x, uint256 _fromTs, uint256 _toTs) internal view returns (uint256) {
+        return _paymentInInterval(_x, _fromTs - START_TIME, _toTs - START_TIME);
     }
 
-    function _upfrontPayment(uint256 ts) internal view returns (uint256) {
-        return _paymentInf(storageCost, ts - startTime);
+    /// @notice Evaluate payment given the timestamp.
+    function _upfrontPayment(uint256 _ts) internal view returns (uint256) {
+        return _paymentInf(STORAGE_COST, _ts - START_TIME);
     }
 
-    // Evaluate the storage cost of a single put().
+    /// @notice Evaluate the storage cost of a single put().
     function upfrontPayment() public view virtual returns (uint256) {
         return _upfrontPayment(block.timestamp);
     }
 
+    /// @notice Checks before appending the key-value.
     function _prepareAppend() internal virtual {
-        require(msg.value >= upfrontPayment(), "not enough payment");
+        require(msg.value >= upfrontPayment(), "DecentralizedKV: not enough payment");
     }
 
-    function _putInternal(bytes32 key, bytes32 dataHash, uint256 length) internal returns (uint256) {
-        require(length <= maxKvSize, "data too large");
-        bytes32 skey = keccak256(abi.encode(msg.sender, key));
+    /// @notice Called by public put method.
+    /// @param _key      Key of the data.
+    /// @param _dataHash Hash of the data.
+    /// @param _length   Length of the data.
+    /// @return          The index of the key-value.
+    function _putInternal(bytes32 _key, bytes32 _dataHash, uint256 _length) internal returns (uint256) {
+        require(_length <= MAX_KV_SIZE, "DecentralizedKV: data too large");
+        bytes32 skey = keccak256(abi.encode(msg.sender, _key));
         PhyAddr memory paddr = kvMap[skey];
 
         if (paddr.hash == 0) {
@@ -99,86 +130,100 @@ contract DecentralizedKV is OwnableUpgradeable {
             idxMap[paddr.kvIdx] = skey;
             kvEntryCount = kvEntryCount + 1;
         }
-        paddr.kvSize = uint24(length);
-        paddr.hash = bytes24(dataHash);
+        paddr.kvSize = uint24(_length);
+        paddr.hash = bytes24(_dataHash);
         kvMap[skey] = paddr;
 
         return paddr.kvIdx;
     }
 
-    // Return the size of the keyed value
-    function size(bytes32 key) public view returns (uint256) {
-        bytes32 skey = keccak256(abi.encode(msg.sender, key));
+    /// @notice Return the size of the keyed value.
+    function size(bytes32 _key) public view returns (uint256) {
+        bytes32 skey = keccak256(abi.encode(msg.sender, _key));
         return kvMap[skey].kvSize;
     }
 
-    // Return the dataHash of the keyed value
-    function hash(bytes32 key) public view returns (bytes24) {
-        bytes32 skey = keccak256(abi.encode(msg.sender, key));
+    /// @notice Return the dataHash of the keyed value.
+    function hash(bytes32 _key) public view returns (bytes24) {
+        bytes32 skey = keccak256(abi.encode(msg.sender, _key));
         return kvMap[skey].hash;
     }
 
-    // Exist
-    function exist(bytes32 key) public view returns (bool) {
-        bytes32 skey = keccak256(abi.encode(msg.sender, key));
+    /// @notice Check if the key-value exists.
+    function exist(bytes32 _key) public view returns (bool) {
+        bytes32 skey = keccak256(abi.encode(msg.sender, _key));
         return kvMap[skey].hash != 0;
     }
 
-    // Return the keyed data given off and len.  This function can be only called in JSON-RPC context of ES L2 node.
+    // @notice Return the keyed data given off and len.  This function can be only called in JSON-RPC context of ES L2 node.
+    /// @param _key        Key of the data.
+    /// @param _decodeType Type of decoding.
+    /// @param _off        Offset of the data.
+    /// @param _len        Length of the data.
+    /// @return            The data.
     function get(
-        bytes32 key,
-        DecodeType decodeType,
-        uint256 off,
-        uint256 len
+        bytes32 _key,
+        DecodeType _decodeType,
+        uint256 _off,
+        uint256 _len
     ) public view virtual returns (bytes memory) {
-        require(len > 0, "data len should be non zero");
+        require(_len > 0, "DecentralizedKV: data len should be non zero");
 
-        bytes32 skey = keccak256(abi.encode(msg.sender, key));
+        bytes32 skey = keccak256(abi.encode(msg.sender, _key));
         PhyAddr memory paddr = kvMap[skey];
-        require(paddr.hash != 0, "data not exist");
-        if (decodeType == DecodeType.PaddingPer31Bytes) {
+        require(paddr.hash != 0, "DecentralizedKV: data not exist");
+        if (_decodeType == DecodeType.PaddingPer31Bytes) {
             // kvSize is the actual data size that dApp contract stores
-            require((paddr.kvSize >= off + len) && (off + len <= maxKvSize - 4096), "beyond the range of kvSize");
+            require(
+                (paddr.kvSize >= _off + _len) && (_off + _len <= MAX_KV_SIZE - 4096),
+                "DecentralizedKV: beyond the range of kvSize"
+            );
         } else {
             // maxKvSize is blob size
-            require(maxKvSize >= off + len, "beyond the range of maxKvSize");
+            require(MAX_KV_SIZE >= _off + _len, "DecentralizedKV: beyond the range of maxKvSize");
         }
-        bytes memory input = abi.encode(paddr.kvIdx, decodeType, off, len, paddr.hash);
-        bytes memory output = new bytes(len);
+        bytes memory input = abi.encode(paddr.kvIdx, _decodeType, _off, _len, paddr.hash);
+        bytes memory output = new bytes(_len);
 
         uint256 retSize = 0;
 
         assembly {
-            if iszero(staticcall(not(0), 0x33301, add(input, 0x20), 0xa0, add(output, 0x20), len)) {
+            if iszero(staticcall(not(0), 0x33301, add(input, 0x20), 0xa0, add(output, 0x20), _len)) {
                 revert(0, 0)
             }
             retSize := returndatasize()
         }
 
         // If this function is called in a regular L1 node, there will no code in 0x33301,
         // and it will simply return immediately instead of revert
-        require(retSize > 0, "get() must be called on ES node");
+        require(retSize > 0, "DecentralizedKV: get() must be called on ES node");
 
         return output;
     }
 
-    // Remove an existing KV pair to a recipient.  Refund the cost accordingly.
-    function removeTo(bytes32 key, address to) public virtual {
-        require(false, "removeTo() unimplemented");
+    /// @notice Remove an existing KV pair to a recipient.  Refund the cost accordingly.
+    /// @param _key Key of the data.
+    /// @param _to  The recipient address.
+    function removeTo(bytes32 _key, address _to) public virtual {
+        revert("DecentralizedKV: removeTo() unimplemented");
     }
 
-    // Remove an existing KV pair.  Refund the cost accordingly.
-    function remove(bytes32 key) public {
-        removeTo(key, msg.sender);
+    /// @notice Remove an existing KV pair.  Refund the cost accordingly.
+    /// @param _key Key of the data.
+    function remove(bytes32 _key) public {
+        removeTo(_key, msg.sender);
     }
 
-    function getKvMetas(uint256[] memory kvIndices) public view virtual returns (bytes32[] memory) {
-        bytes32[] memory res = new bytes32[](kvIndices.length);
+    /// @notice Get the metadata of the key-value.
+    /// @param _kvIndices The indices of the key-value.
+    /// @return The metadatas of the key-value.
+    function getKvMetas(uint256[] memory _kvIndices) public view virtual returns (bytes32[] memory) {
+        bytes32[] memory res = new bytes32[](_kvIndices.length);
 
-        for (uint256 i = 0; i < kvIndices.length; i++) {
-            PhyAddr memory paddr = kvMap[idxMap[kvIndices[i]]];
+        for (uint256 i = 0; i < _kvIndices.length; i++) {
+            PhyAddr memory paddr = kvMap[idxMap[_kvIndices[i]]];
 
-            res[i] |= bytes32(uint256(kvIndices[i])) << 216;
+            res[i] |= bytes32(uint256(_kvIndices[i])) << 216;
             res[i] |= bytes32(uint256(paddr.kvSize)) << 192;
             res[i] |= bytes32(paddr.hash) >> 64;
         }
@@ -187,7 +232,27 @@ contract DecentralizedKV is OwnableUpgradeable {
     }
 
     /// @notice This is for compatibility with earlier versions and can be removed in the future.
-    function lastKvIndex() public view returns (uint40) {
+    function lastKvIdx() public view returns (uint40) {
         return kvEntryCount;
     }
+
+    /// @notice Getter for STORAGE_COST
+    function storageCost() public view returns (uint256) {
+        return STORAGE_COST;
+    }
+
+    /// @notice Getter for DCF_FACTOR
+    function dcfFactor() public view returns (uint256) {
+        return DCF_FACTOR;
+    }
+
+    /// @notice Getter for START_TIME
+    function startTime() public view returns (uint256) {
+        return START_TIME;
+    }
+
+    /// @notice Getter for MAX_KV_SIZE
+    function maxKvSize() public view returns (uint256) {
+        return MAX_KV_SIZE;
+    }
 }