Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

QA Report #59

Open
code423n4 opened this issue Jun 30, 2022 · 2 comments
Open

QA Report #59

code423n4 opened this issue Jun 30, 2022 · 2 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Overview

Risk Rating Number of issues
Low Risk 2
Non-Critical Risk 11

Table of Contents

1. Low Risk Issues

1.1. Deprecated approve() function

While safeApprove() in itself is deprecated, it is still better than approve which is subject to a known front-running attack and failing for certain token implementations that do not return a boolean value. Consider using safeApprove instead (or better: safeIncreaseAllowance()/safeDecreaseAllowance()):

lending-market-v2/contracts/Accountant/AccountantDelegate.sol:41:  note.approve(cnoteAddress_, type(uint).max); // approve lending market, to transferFrom Accountant as needed
lending-market-v2/contracts/CDaiDelegate.sol:57:        dai.approve(daiJoinAddress, type(uint).max);

1.2. abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Similar issue in the past: here

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.

lending-market-v2/contracts/Governance/Comp.sol:164:        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
lending-market-v2/contracts/Governance/GovernorAlpha.sol:256:        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:427:            abi.encodePacked(
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:528:        bytes32 salt = keccak256(abi.encodePacked(token0, token1, stable)); // notice salt includes stable as well, 3 parameters
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:112:        pair = address(uint160(uint256(keccak256(abi.encodePacked(
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:115:            keccak256(abi.encodePacked(token0, token1, stable)),
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:539:        return (keccak256(abi.encodePacked(str1)) == keccak256(abi.encodePacked(str2)));
lending-market-v2/contracts/SimplePriceOracle.sol:42:        return (keccak256(abi.encodePacked((a))) == keccak256(abi.encodePacked((b))));
lending-market-v2/contracts/Timelock.sol:96:            callData = abi.encodePacked(bytes4(keccak256(bytes(signature))), data);

2. Non-Critical Issues

2.1. import "hardhat/console.sol"; shouldn't exist in production code

Please delete the following before deploying:

lending-market-v2/contracts/CErc20.sol:
  5: import "hardhat/console.sol";

lending-market-v2/contracts/Accountant/AccountantDelegate.sol:
  7: import "hardhat/console.sol";

lending-market-v2/contracts/Stableswap/BaseV1-core.sol:
    4: import "hardhat/console.sol";
  207:         console.log("tokenIn: ", tokenIn);

2.2. require() should be used for checking error conditions on inputs and return values while assert() should be used for invariant checking

Properly functioning code should never reach a failing assert statement, unless there is a bug in your contract you should fix. Here, I believe the assert should be a require or a revert:

lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:100:        assert(msg.sender == address(wcanto)); // only accept ETH via fallback from the WETH contract
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:245:                assert(amountAOptimal <= amountADesired);
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:291:        assert(wcanto.transfer(pair, amountCANTO));
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:437:        assert(wcanto.transfer(pairFor(routes[0].from, routes[0].to, routes[0].stable), amounts[0]));

As the Solidity version is > 0.8.* the remaining gas would still be refunded in case of failure.

2.3. It's better to emit after all processing is done

lending-market-v2/contracts/CNote.sol:
  16      function setAccountantContract(address accountant_) public {
  17          require(msg.sender == admin, "CNote::_setAccountantContract:Only admin may call this function");
  18          
  19:         emit AccountantSet(accountant_, address(_accountant));
  20       _accountant = AccountantInterface(accountant_);
  21      }
lending-market-v2/contracts/NoteInterest.sol:
   91      constructor(uint _baseRatePerYear) {
   92          baseRatePerYear = _baseRatePerYear;
   93          baseRatePerBlock = _baseRatePerYear.div(BlocksPerYear);
   94:         emit NewInterestParams(baseRatePerBlock);
   95          admin = msg.sender;
   96          lastUpdateBlock = block.number;
   97      }

lending-market-v2/contracts/SimplePriceOracle.sol:
  25      function setUnderlyingPrice(CToken cToken, uint underlyingPriceMantissa) public {
  26          address asset = _getUnderlyingAddress(cToken);
  27:         emit PricePosted(asset, prices[asset], underlyingPriceMantissa, underlyingPriceMantissa);
  28          prices[asset] = underlyingPriceMantissa;
  29      }

  31      function setDirectPrice(address asset, uint price) public {
  32:         emit PricePosted(asset, prices[asset], price, price);
  33          prices[asset] = price;
  34      }

lending-market-v2/contracts/Accountant/AccountantDelegate.sol:
  34  
  35    uint[] memory err = comptroller.enterMarkets(MarketEntered); // check if market entry returns without error
  36    if (err[0] != 0) {
  37     revert ErrorMarketEntering(err[0]);
  38    }
  39:   emit AcctInit(cnoteAddress_);
  40  
  41    note.approve(cnoteAddress_, type(uint).max); // approve lending market, to transferFrom Accountant as needed
  42   }

lending-market-v2/contracts/Accountant/AccountantDelegator.sol:
  44:         emit NewImplementation(implementation, implementation_);
  45         
  46          implementation = implementation_;
  47      }

lending-market-v2/contracts/Governance/Comp.sol:
  137:             emit Approval(src, spender, newAllowance);
  138          }
  139  
  140          _transferTokens(src, dst, amount);
  141          return true;
  142      }

  228:         emit DelegateChanged(delegator, currentDelegate, delegatee);
  229  
  230          _moveDelegates(currentDelegate, delegatee, delegatorBalance);
  231      }

  239:         emit Transfer(src, dst, amount);
  240  
  241          _moveDelegates(delegates[src], delegates[dst], amount);
  242      }

lending-market-v2/contracts/Stableswap/BaseV1-core.sol:
  453:             emit Approval(src, spender, newAllowance);
  454          }
  455  
  456          _transferTokens(src, dst, amount);
  457          return true;
  458      }

2.4. Typos

lending-market-v2/contracts/Accountant/AccountantDelegate.sol:12:      * @notice Method used to initialize the contract during delegator contructor
lending-market-v2/contracts/Governance/GovernorAlpha.sol:138:        require(targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, "GovernorAlpha::propose: proposal function information arity mismatch");
lending-market-v2/contracts/Governance/GovernorAlpha.sol:156:        require(newProposal.id == 0, "GovernorAlpha::propose: ProposalID collsion");
lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol:45:                "GovernorBravo::propose: proposal function information arity mismatch");
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:64:    // Structure to capture time period obervations every 30 minutes, used for local oracles
lending-market-v2/contracts/CNote.sol:24:    * @dev return the current address of the Accounant
lending-market-v2/contracts/CToken.sol:291:     * @dev This function does not accrue efore calculating the exchange rate
lending-market-v2/contracts/DAIInterestRateModelV3.sol:65:     * @param reserves The total amnount of reserves the market has

2.5. Deprecated library used for Solidity >= 0.8 : SafeMath

lending-market-v2/contracts/NoteInterest.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/NoteInterest.sol:5:import "./SafeMath.sol";
lending-market-v2/contracts/NoteInterest.sol:15:    using SafeMath for uint;
lending-market-v2/contracts/Timelock.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Timelock.sol:4:import "./SafeMath.sol";
lending-market-v2/contracts/Timelock.sol:7:    using SafeMath for uint;

2.6. Missing friendly revert strings

lending-market-v2/contracts/Accountant/AccountantDelegator.sol:22:        require(admin_ != address(0));
lending-market-v2/contracts/Governance/Comp.sol:276:        require(n < 2**32, errorMessage);
lending-market-v2/contracts/Governance/Comp.sol:281:        require(n < 2**96, errorMessage);
lending-market-v2/contracts/Governance/Comp.sol:287:        require(c >= a, errorMessage);
lending-market-v2/contracts/Governance/Comp.sol:292:        require(b <= a, errorMessage);
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:127:        require(_unlocked == 1);
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:288:        require(!BaseV1Factory(factory).isPaused());
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:468:        require(token.code.length > 0);
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:471:        require(success && (data.length == 0 || abi.decode(data, (bool))));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:501:        require(msg.sender == pauser);
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:506:        require(msg.sender == pendingPauser);
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:511:        require(msg.sender == pauser);
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:228:        require(amountADesired >= amountAMin);
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:229:        require(amountBDesired >= amountBMin);
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:309:        require(IBaseV1Pair(pair).transferFrom(msg.sender, pair, liquidity)); // send liquidity to pair
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:474:        require(token.code.length > 0);
lending-market-v2/contracts/Treasury/TreasuryDelegator.sol:13:        require(admin_ != address(0));
lending-market-v2/contracts/Note.sol:22:        require(msg.sender == admin);
lending-market-v2/contracts/Note.sol:23:        require(address(accountant) == address(0));
manifest-v2/contracts/Proposal-Store.sol:44:        require(msg.sender == UniGovModAcct);

2.7. Open TODOS

Consider resolving the TODOs before deploying.

lending-market-v2/contracts/Reservoir.sol:49:    uint reservoirBalance_ = token_.balanceOf(address(this)); // TODO: Verify this is a static call

2.8. 1000000000000000000 should be changed to 1e18 for readability reasons

manifest-v2/contracts/ERC20MaliciousDelayed.sol:11:  uint256 private _bigNum = 1000000000000000000; // ~uint256(0)

2.9. Use string.concat() or bytes.concat()

Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))

Solidity version 0.8.12 introduces string.concat() (vs abi.encodePacked(<str>,<str>))

lending-market-v2/contracts/Governance/Comp.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/Comp.sol:164:        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
lending-market-v2/contracts/Governance/GovernorAlpha.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorAlpha.sol:256:        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:2:pragma solidity 0.8.11;
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:111:            name = string(abi.encodePacked("StableV1 AMM - ", erc20(_token0).symbol(), "/", erc20(_token1).symbol()));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:112:            symbol = string(abi.encodePacked("sAMM-", erc20(_token0).symbol(), "/", erc20(_token1).symbol()));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:114:            name = string(abi.encodePacked("VolatileV1 AMM - ", erc20(_token0).symbol(), "/", erc20(_token1).symbol()));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:115:            symbol = string(abi.encodePacked("vAMM-", erc20(_token0).symbol(), "/", erc20(_token1).symbol()));
lending-market-v2/contracts/Stableswap/BaseV1-core.sol:528:        bytes32 salt = keccak256(abi.encodePacked(token0, token1, stable)); // notice salt includes stable as well, 3 parameters
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:3:pragma solidity 0.8.11;
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:112:        pair = address(uint160(uint256(keccak256(abi.encodePacked(
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:115:            keccak256(abi.encodePacked(token0, token1, stable)),
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:539:        return (keccak256(abi.encodePacked(str1)) == keccak256(abi.encodePacked(str2)));
lending-market-v2/contracts/SimplePriceOracle.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/SimplePriceOracle.sol:42:        return (keccak256(abi.encodePacked((a))) == keccak256(abi.encodePacked((b))));
lending-market-v2/contracts/Timelock.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Timelock.sol:96:            callData = abi.encodePacked(bytes4(keccak256(bytes(signature))), data);

2.10. The pragmas used are not the same everywhere

lending-market-v2/contracts/Stableswap/BaseV1-core.sol:2:pragma solidity 0.8.11;
lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol:3:pragma solidity 0.8.11;
lending-market-v2/contracts/Accountant/AccountantDelegate.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Accountant/AccountantDelegator.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Accountant/AccountantInterfaces.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/Comp.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorAlpha.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorBravoDelegator.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorBravoInterfaces.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Treasury/TreasuryDelegate.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Treasury/TreasuryDelegator.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Treasury/TreasuryInterfaces.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/BaseJumpRateModelV2.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CDaiDelegate.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20Delegate.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20Delegator.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20Immutable.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CEther.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CNote.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/CToken.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CTokenInterfaces.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/DAIInterestRateModelV3.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/EIP20Interface.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/EIP20NonStandardInterface.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/ERC20.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/ErrorReporter.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/ExponentialNoError.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/InterestRateModel.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/JumpRateModel.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/JumpRateModelV2.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Maximillion.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Note.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/NoteInterest.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/PriceOracle.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Reservoir.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/SafeMath.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/SimplePriceOracle.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Timelock.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/WETH.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/WhitePaperInterestRateModel.sol:2:pragma solidity ^0.8.10;
manifest-v2/contracts/ERC20Burnable.sol:4:pragma solidity ^0.8.0;
manifest-v2/contracts/ERC20DirectBalanceManipulation.sol:3:pragma solidity ^0.8.0;
manifest-v2/contracts/ERC20MaliciousDelayed.sol:3:pragma solidity ^0.8.0;
manifest-v2/contracts/ERC20MinterBurnerDecimals.sol:4:pragma solidity ^0.8.0;
manifest-v2/contracts/Proposal-Store.sol:3:pragma solidity ^0.8.0;

2.11. Non-library/interface files should use fixed compiler versions, not floating ones

lending-market-v2/contracts/Accountant/AccountantDelegate.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Accountant/AccountantDelegator.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Accountant/AccountantInterfaces.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/Comp.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorAlpha.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorBravoDelegator.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Governance/GovernorBravoInterfaces.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Treasury/TreasuryDelegate.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Treasury/TreasuryDelegator.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/Treasury/TreasuryInterfaces.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/BaseJumpRateModelV2.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CDaiDelegate.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20Delegate.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20Delegator.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CErc20Immutable.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CEther.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CNote.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/CToken.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/CTokenInterfaces.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/DAIInterestRateModelV3.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/EIP20Interface.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/EIP20NonStandardInterface.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/ERC20.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/ErrorReporter.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/ExponentialNoError.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/InterestRateModel.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/JumpRateModel.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/JumpRateModelV2.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Maximillion.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Note.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/NoteInterest.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/PriceOracle.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Reservoir.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/SafeMath.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/SimplePriceOracle.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/Timelock.sol:2:pragma solidity ^0.8.10;
lending-market-v2/contracts/WETH.sol:1:pragma solidity ^0.8.10;
lending-market-v2/contracts/WhitePaperInterestRateModel.sol:2:pragma solidity ^0.8.10;
manifest-v2/contracts/ERC20Burnable.sol:4:pragma solidity ^0.8.0;
manifest-v2/contracts/ERC20DirectBalanceManipulation.sol:3:pragma solidity ^0.8.0;
manifest-v2/contracts/ERC20MaliciousDelayed.sol:3:pragma solidity ^0.8.0;
manifest-v2/contracts/ERC20MinterBurnerDecimals.sol:4:pragma solidity ^0.8.0;
manifest-v2/contracts/Proposal-Store.sol:3:pragma solidity ^0.8.0;
@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jun 30, 2022
code423n4 added a commit that referenced this issue Jun 30, 2022
@code423n4
Meera issue #59
1bdd425
@GalloDaSballo
Copy link
Collaborator

1.1. Deprecated approve() function

Disagree for this case (0 -> max)

1.2. abi.encodePacked()

Disputed in lack of POC, all the listed lines will not allow "dynamic types" and sorting of pairs is determinsitic

2.1. import "hardhat/console.sol"; shouldn't exist in production code

Valid R

2.2. require() should be used for checking error

Valid Low

2.3. It's better to emit after all processing is done

Disagree as Slither will give false positive, is preference, ultimately up to developer

2.4. Typos

NC

2.5. Deprecated library used for Solidity >= 0.8 : SafeMath

Valid R

2.6. Missing friendly revert strings

NC

2.8. 1000000000000000000 should be changed to 1e18 for readability reasons

R

2.9. Use string.concat() or bytes.concat()

NC

2.10. The pragmas used are not the same everywhere & 2.11

NC

@GalloDaSballo
Copy link
Collaborator

GalloDaSballo commented Aug 14, 2022 •
edited
Loading

1L 3R 4NC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@GalloDaSballo @code423n4