--- title: BTCST2 CertiK Preliminary Comments For BTCST2 tags: pre-report --- <div><div style="background-image:url(https://i.imgur.com/n9A7QyA.png); height: 800px;background-position: center; background-repeat: no-repeat;background-size: cover;"><div style="position: relative;top: 25%;left:5.6%;line-height: 2.5em; color: white;"> <p style="font-size: 28px">Preliminary Comments</p> <p style="font-size: 22px">Security Assessment</p> <p style="font-size: 18px">January 22th, 2021</p> <p style="font-size: 18px; color: darkred">Preliminary Report</p> </div> <div style="position: relative;top: 44%;left:5.6%; color: #8D8D8D;">For : <p style="font-size: 12px; color: #C6CCD3">Standard Hashrate Group</p> <p style="font-size: 11px;color: #C6CCD3; line-height: 1.5rem;">By :</p> <p style="font-size: 11px; line-height: 0.5rem; color: #C6CCD3;">Daniel Tan @ CertiK</p> <p style="font-size: 12px; line-height: 0.03rem; color:#F6AA42">daniel.tan@certik.org</p> <br/> <p style="font-size: 12px; line-height: 0.5rem; color: #C6CCD3;">Owan Li @ CertiK</p> <p style="font-size: 12px; line-height: 0.055rem; color:#F6AA42">guilong.li@certik.org</p> </div> </div></div> <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Overview #### Project Summary <table> <tr> <td width="50%" valign="top"><b>Project Name</b></td> <td width="50%" valign="top"><a href="https://github.com/Standard-Hashrate-Group/SmartContracts">Standard Hashrate Token</a></td> </tr> <tr> <td width="50%" valign="top"><b>Description</b></td> <td width="50%" valign="top">An ERC20 token implementation with an linear release mechanism</td> </tr> <tr> <td width="50%" valign="top"><b>Platform</b></td> <td width="50%" valign="top">Ethereum; Solidity</td> </tr> <tr> <td width="50%" valign="top"><b>Codebase</b></td> <td width="50%" valign="top"><a href="https://github.com/Standard-Hashrate-Group/SmartContracts">GitHub Repository</a></td> </tr> <tr> <td width="50%" valign="top"><b>Commit</b></td> <td width="50%" valign="top"> <a href="https://github.com/Standard-Hashrate-Group/SmartContracts/commit/62d2ccb456339c1094d57325adb0eab95d43ab8f"> 62d2ccb456339c1094d57325adb0eab95d43ab8f</a><br/> </td> </tr> </table> #### Audit Summary <table> <tr> <td width="50%" valign="top"><b>Delivery Date</b></td> <td width="50%" valign="top">Jan. 22, 2021</td> </tr> <tr> <td width="50%" valign="top"><b>Method of Audit</b></td> <td width="50%" valign="top">Static Analysis, Manual Review</td> </tr> <tr> <td width="50%" valign="top"><b>Consultants Engaged</b></td> <td width="50%" valign="top">2</td> </tr> <tr> <td width="50%" valign="top"><b>Timeline</b></td> <td width="50%" valign="top">Jan. 18, 2021 - Jan. 22, 2021</td> </tr> </table> #### Vulnerability Summary <table> <tr> <td width="50%" valign="top"><b>Total Issues</b></td> <td width="50%" valign="top">14</td> </tr> <tr> <td width="50%" valign="top"><b>Total Critical</b></td> <td width="50%" valign="top">0</td> </tr> <tr> <td width="50%" valign="top"><b>Total Major</b></td> <td width="50%" valign="top">1</td> </tr> <tr> <td width="50%" valign="top"><b>Total Minor</b></td> <td width="50%" valign="top">1</td> </tr> <tr> <td width="50%" valign="top"><b>Total Informational</b></td> <td width="50%" valign="top">11</td> </tr> <tr> <td width="50%" valign="top"><b>Total Discussion</b></td> <td width="50%" valign="top">1</td> </tr> </table> --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Executive Summary This report has been prepared for **BTCST** protocol to discover issues and vulnerabilities in the source code of their Smart Contract as well as any contract dependencies that were not part of an officially recognized library. A comprehensive examination has been performed, utilizing Dynamic Analysis, Static Analysis, and Manual Review techniques. The auditing process pays special attention to the following considerations: * Testing the smart contracts against both common and uncommon attack vectors. * Assessing the codebase to ensure compliance with current best practices and industry standards. * Ensuring contract logic meets the specifications and intentions of the client. * Cross referencing contract structure and implementation against similar smart contracts produced by industry leaders. * Thorough line-by-line manual review of the entire codebase by industry experts. --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> File in Scope | Contract | SHA-256 Checksum | | ----------------------------- | ------------------------------------------------------------ | | **FarmWithApi.sol** | 7aa50953493c5a44937ce3b2e4c3e4082ef9c82802b2de820eb65493106d7532 | | **MiningFarm.sol** | 69d9589d67b5af2a94bd57b12c77e635d7470f435e85bf40064e83d145e0507e | | **FarmAllowLockedToken.sol** | c47ecff1d91637153d3973a55dbd3d849f991ccb7a54ba39fde452729caa85f5 | | **FarmOperator.sol** | 1dc319bf1001e733a8575fdd78d6bcb1185fa6809bd5d16b7378dfc66e397980 | | **IFarm.sol** | f53096ecac27b204047f07f52956c050ac2ab75e6136227ed91a53ea45fe04c4 | | **IBEP20.sol** | 5a9a102b610acd9a3aa60e1df429238cf19f60830b40e81df150b4569c57f577 | | **TransferHelper.sol** | 4df6715ebc2d1b3f0aed22ff7376c13c9fa5fa1c490b0c531bf10949cace6f56 | | **IMiningFarm.sol** | 25a9612ddf9381bec54084d9859055796e95344fa1981e1b948c550c651726c7 | | **ISTokenERC20.sol** | 458aa65113a36a1c4468ede4bb2e7d4246e5a71332beddad79ba92ea7425eb13| --- | ID | Title | Type | Severity | | -----: | --------------------------------------- | ----------------------- | ------------- | | EXH-01 | Incorrect Naming Convention Utilization | Coding Style | Informational | | EXH-02 | Proper Usage of "public" and "external" type | Gas Optimization | Informational | | EXH-03 | Proper Imports | Dead Code | Informational | | EXH-04 | Uninitialized state variables | Logical Issue | Informational | | EXH-05 | Code Redundancy | Dead Code | Informational | | EXH-06 | Missing Emit Events | Optimization| Minor | | EXH-07 | Administrator Capability | Optimization| Major | | EXH-08 | Commented Useless Codes | Optimization| Informational | | EXH-09 | Return Value Checking | Optimization| Informational | | EXH-10 | Balance Checking | Optimization| Informational | | EXH-11 | Code Optimization | Optimization| Informational | | EXH-12 | Mining Reward Distribution | Optimization| Informational | | EXH-13 | Code Optimization | Optimization| Informational | | EXH-14 | Variable Naming | Optimization| Discussion | --- ### <a name="UNP-01" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-01: Incorrect Naming Convention Utilization | Type | Severity | Location | | ------------ | ------------- | ----------------------------------------------------------- | | Coding Style | Informational | [MiningFarm.sol L35 FarmOperator L27, FarmAllowLockedToken L19](#) | #### Description: Solidity defines a naming convention that should be followed. In general, parameters should use mixedCase, refer to: https://solidity.readthedocs.io/en/v0.6.12/style-guide.html#naming-conventions Variables should use mixedCase. Examples: Variable like: `_devaddr`, `COMMIT_ID` Constands should use UPPER_CASE_WITH_UNDERSCORES. Examples: Parameters like: `_miniSeedTokenNeedsForLockedStaking` #### Recommendation: The recommendations outlined here are intended to improve the readability, and thus they are not rules, but rather guidelines to try and help convey the most information through the names of things. ### <a name="UNP-02" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-02: Proper Usage of "public" and "external" type | Type | Severity | Location | | ------------ | ------------- | ----------------------------------------------------------- | | Gas Optimization | Informational | [MiningFarm.sol, FarmAllowLockedToken.sol, FarmOperator.sol](#) | #### Description: "public" functions that are never called by the contract could be declared "external" . When the inputs are arrays "external" functions are more efficient than "public" functions. Examples: Functions `withdrawAllSToken()`, `ownerPause()`, `ownerUnpause()`, `changeBaseTime()`, `changeMiniStakePeriodInSeconds()`, `changeRewardToken()`, `changeSToken()`, `totalMinedRewardFrom()`, `totalClaimedRewardFrom()`, `totalRewardInPoolFrom()`, `totalRewardInPool()`, `miningRewardIn()`, `stakeRecord()`, `getTotalRewardBalanceInPool()`, `depositToMining()`, `depositRewardFromForYesterday()`, `depositRewardFromForToday()`, `depositRewardFromForTime()`, `claimAllReward()` in contract `MiningFarm`. Functions `withdrawAllSToken()`, `withdrawAllLockedSToken()` in contract `FarmAllowLockedToken`. Functions `initialize()`, `adminChangeFarm()`, `adminChangeRToken()`, `adminChangeSToken()`, `rootSTokenCall()` in contract `FarmOperator`. #### Recommendation: Consider using the "external" attribute for functions never called from the contract. ### <a name="UNP-03" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-03: Proper Imports | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Dead Code | Informational | [FarmWithApi.sol L13](#) | #### Description: There are some imported files not used in the contract `FarmWithApi`. ``` import "./MiningFarm.sol"; ``` There are some imported files not used in the contract `MiningFarm`. ``` import "./BTCST.sol"; ``` There are some OpenZeppelin libraries are imported by copying to the project. Better import libraries from github rather than copy it to directory. #### Recommendation: We recommend to remove the unused imports, and import neccessary libraries from github. ### <a name="UNP-04" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-04: Uninitialized state variables | Type | Severity | Location | | --------- | ------------- | ----------------------------- | | Logical Issue | Informational | [FarmAllowLockedToken.sol L22,L37,L202](#) | #### Description: State variable `_stakedLockedBalanceFreeTimeKeys` is never initialized before usage. Since its type is unit, it's default value is zero. ```Solidity L:22 mapping(address=>uint[]) _stakedLockedBalanceFreeTimeKeys; L:37 uint[] storage stakedLockedBalanceFreeTime = _stakedLockedBalanceFreeTimeKeys[account]; L:202 uint[] memory stakeLockedTimekeys = _stakedLockedBalanceFreeTimeKeys[to]; ``` #### Recommendation: Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero. ### <a name="UNP-05" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-05: Code Redundancy | Type | Severity | Location | | --------- | ------------- | ----------------------------- | | Dead Code | Informational | [FarmAllowLockedToken.sol L19](#) | #### Description: Unused state variable. ```Solidity L:19 uint256 public _miniSeedTokenNeedsForLockedStaking; ``` #### Recommendation: We recommend to remove unused state variables. ### <a name="UNP-06" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-06: Missing Emit Events | Type | Severity | Location | | ------------ | -------- | ------------------------------------------------------------ | | Optimization | Minor | [FarmOperator.sol L39,L43,L47](#) | #### Description: Several sensitive actions are defined without event declarations. Examples: `adminChangeFarm()` , `adminChangeRToken()` , `adminChangeSToken()` in contract `FarmOperator`. #### Recommendation: Consider adding events for sensitive actions, and emit it in the function like below. ```Solidity event AdminChangeFarm(address indexed farm); function adminChangeFarm(address farm)public onlyOwner{ _farmContract = farm; emit AdminChangeFarm(farmamount); } ``` ### <a name="UNP-07" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-07: Administrator Capability | Type | Severity | Location | | ------------ | -------- | ------------------------------------------------------------ | | Optimization | Major | [FarmWithApi.sol L83,L91](#) | #### Description: To bridge the trust gap between administrator and users, administrator needs to express a sincere attitude with the consideration of the administrator team's anonymousness. The administrator has the responsibility to notify users with the following capability of the administrator: * Administrator can transfer reward tokens to own account under unpredicted cases via `emergencyWithdrawReward` method * Administrator can transfer hashrate tokens to own account under unpredicted cases via `emergencyWithdrawSToken` method Examples: ```Solidity /** * @dev emergency withdraw reward tokens to owner's account if there is some unusual thing happend */ function emergencyWithdrawReward(uint256 amount) external onlyOwner{ uint256 bal =_rewardToken.balanceOf(address(this)); require(bal>=amount,"withdraw amount exceeds the reward token's balance"); _rewardToken.transfer(owner(),amount); } /** * @dev emergency withdraw hashrate tokens to owner's account if there is some unusual thing happend */ function emergencyWithdrawSToken(uint256 amount) external onlyOwner{ uint256 bal =_stoken.balanceOf(address(this)); require(bal>=amount,"withdraw amount exceeds the hashrate token's balance"); _stoken.transfer(owner(),amount); } ``` #### Recommendation: The advantage of `emergencyWithdrawReward` and `emergencyWithdrawSToken` methods in the protocol is that the administrator reserves the ability to rescue the assets in this contract under unexpected cases. It is also worthy of note the downside of `emergencyWithdrawReward` and `emergencyWithdrawSToken` methods, where the treasury in this contract can be migrated to administrator's addresses. To improve the trustworthiness of the project, any dynamic runtime changes on the protocol should be notified to clients. Any plan to call this `emergencyWithdrawReward` and `emergencyWithdrawSToken` methods is better to move to the execution queue of Timelock, and also emit events. ### <a name="UNP-08" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-08: Proper Imports | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Dead Code | Informational | [TokenUtility.sol L50](#) | #### Description: There are some useless commented codes, which are ignored by the compiler, in the source file `TokenUtility.sol` from line 50 to line 55. Examples: ```Solidity L:50 //uint round = time.sub(_farmStartedTime).div(_miniStakePeriodInSeconds); L:51 //uint end = _farmStartedTime.add(round.mul(_miniStakePeriodInSeconds)); ... ``` #### Recommendation: We recommend to remove those commented codes. ### <a name="UNP-09" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-09: Return Value Checking | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Optimization | Informational | [MiningFarm.sol L:260 L:262](#) | #### Description: Return value checking is required for a safe transfer method in case that the transfer failed in the contract `MiningFarm` Examples: ```Solidity L:260 token.transfer(to,bal); L:262 token.transfer(to,amount); ``` #### Recommendation: It’s a good practice to add the checking to the return value of the safe transfer function. For example: ```Solidity reuqired(token.transfer(to,bal), "A transfer failed"); ``` ### <a name="UNP-10" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-10: Balance Checking | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Optimization | Informational | [MiningFarm.sol L:260 L:262 L:385](#) | #### Description: There are functions maybe transfer zero to the recipient in the contract `MiningFarm`. Examples: ```Solidity L:260 token.transfer(to,bal); L:262 token.transfer(to,amount); ``` There is no validation that compare account balance to deposit amount before deposit in the contract `MiningFarm`. Examples: ```Solidity L:385 _rewardToken.safeTransferFrom(account,address(this),amount); ``` #### Recommendation: Non-zero value checking is required before a transfer. require(bal > 0, "bal == 0"); ```Solidity require(amount > 0, "amount == 0"); ``` Compare the account balance to deposit amount before the deposit. ```Solidity require(account.balance >= amount, "account.balance < amount"); ``` ### <a name="UNP-11" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-11: Code Optimization | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Optimization | Informational | [MiningFarm.sol L:520](#) | #### Description: There is a storage type value `previouSlot`, which does not be updated. Examples: ```Solidity L:520 RoundSlotInfo storage previouSlot = _roundSlots[slotMaxLast]; ``` #### Recommendation: The variable `previouSlot` is better to be defined as a memory type value since there is no update to it. ```Solidity L:520 RoundSlotInfo memory previouSlot = _roundSlots[slotMaxLast]; ``` ### <a name="UNP-12" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-12: Mining Reward Distribution | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Mining Reward Distribution | Informational | [WhitePaper Page:6](#) | #### Description: That the User B Mined BTC in the Day 5 is 1.10 means User B received the same reward as the User A despite that User B staked more BTCSTs in the Day 4. #### Recommendation: User B Mined BTC, in the Day 5, is supposed to be 1.40 since User B staked more BTCSTs. ### <a name="UNP-13" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-13: Code Optimization | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Optimization | Informational | [MiningFarm.sol L:237 L:238](#) | #### Description: There are some `int` do the addition by operator `+`. Examples: ```Solidity L:237 lastUpdate+_miniStakePeriodInSeconds L:238 before+_miniStakePeriodInSeconds ``` #### Recommendation: It's better to do the addition by the function `.add` to avoid overflow. ```Solidity lastUpdate.add(_miniStakePeriodInSeconds) before.add(_miniStakePeriodInSeconds) ``` ### <a name="UNP-14" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-14: Variable Naming | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Optimization | Discussion | [MiningFarm.sol L:96](#) | #### Description: What's the word `lowest` stands for, in the variable `stakedLowestWaterMark` <div STYLE="page-break-after: always;"></div> ### **Appendix** ------ ### **Finding Categories** **Gas Optimization** Gas Optimization findings refer to exhibits that do not affect the functionality of the code but generate different, more optimal EVM opcodes resulting in a reduction on the total gas cost of a transaction. **Mathematical Operations** Mathematical Operation exhibits entail findings that relate to mishandling of math formulas, such as overflows, incorrect operations etc. **Logical Issue** Logical Issue findings are exhibits that detail a fault in the logic of the linked code, such as an incorrect notion on how `block.timestamp` works. **Control Flow** Control Flow findings concern the access control imposed on functions, such as owner-only functions being invoke-able by anyone under certain circumstances. **Volatile Code** Volatile Code findings refer to segments of code that behave unexpectedly on certain edge cases that may result in a vulnerability. **Data Flow** Data Flow findings describe faults in the way data is handled at rest and in memory, such as the result of a `struct` assignment operation affecting an in-memory `struct` rather than an instorage one. **Language Specific** Language Specific findings are issues that would only arise within Solidity, i.e. incorrect usage of `private` or `delete` . **Coding Style** Coding Style findings usually do not affect the generated byte-code and comment on how to make the codebase more legible and as a result easily maintainable. **Inconsistency** Inconsistency findings refer to functions that should seemingly behave similarly yet contain different code, such as a `constructor` assignment imposing different `require` statements on the input variables than a setter function. **Magic Numbers** Magic Number findings refer to numeric literals that are expressed in the codebase in their raw format and should otherwise be specified as `constant` contract variables aiding in their legibility and maintainability. **Compiler Error** Compiler Error findings refer to an error in the structure of the code that renders it impossible to compile using the specified version of the project. **Dead Code** Code that otherwise does not affect the functionality of the codebase and can be safely omitted.