| Issue | Contexts | Estimated Gas Saved | |
|---|---|---|---|
| GAS‑1 | abi.encode() is less efficient than abi.encodepacked() |
2 | 200 |
| GAS‑2 | <array>.length Should Not Be Looked Up In Every Loop Of A For-loop |
3 | 291 |
| GAS‑3 | Setting the constructor to payable |
6 | 78 |
| GAS‑4 | Do not calculate constants | 7 | - |
| GAS‑5 | Duplicated require()/revert() Checks Should Be Refactored To A Modifier Or Function |
4 | 112 |
| GAS‑6 | ++i Costs Less Gas Than i++, Especially When It’s Used In For-loops (--i/i-- Too) |
2 | 12 |
| GAS‑7 | Use assembly to write address storage values |
1 | - |
| GAS‑8 | Functions guaranteed to revert when called by normal users can be marked payable |
6 | 126 |
| GAS‑9 | Use hardcoded address instead address(this) |
9 | - |
| GAS‑10 | It Costs More Gas To Initialize Variables To Zero Than To Let The Default Of Zero Be Applied | 1 | - |
| GAS‑11 | internal functions only called once can be inlined to save gas |
14 | 308 |
| GAS‑12 | Multiplication/division By Two Should Use Bit Shifting | 1 | - |
| GAS‑13 | Optimize names to save gas | 10 | 220 |
| GAS‑14 | <x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables |
20 | - |
| GAS‑15 | Structs can be packed into fewer storage slots by editing time variables | 2 | 4000 |
| GAS‑16 | Using private rather than public for constants, saves gas |
5 | - |
| GAS‑17 | Public Functions To External | 37 | - |
| GAS‑18 | Save gas with the use of specific import statements | 28 | - |
| GAS‑19 | Splitting require() Statements That Use && Saves Gas |
1 | 9 |
| GAS‑20 | State variables can be packed into fewer storage slots | 1 | 2000 |
| GAS‑21 | Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead |
10 | - |
| GAS‑22 | ++i/i++ Should Be unchecked{++i}/unchecked{i++} When It Is Not Possible For Them To Overflow, As Is The Case When Used In For- And While-loops |
3 | 105 |
| GAS‑23 | Using unchecked blocks to save gas |
2 | 40 |
| GAS‑24 | Unnecessary look up in if condition |
1 | 2100 |
| GAS‑25 | Use of Custom Errors Instead of String | 12 | - |
| GAS‑26 | Use solidity version 0.8.19 to gain some gas boost | 10 | 880 |
| GAS‑27 | Using 10**X for constants isn't gas efficient |
3 | - |
Total: 201 contexts over 27 issues
See for more information: https://github.com/ConnorBlockchain/Solidity-Encode-Gas-Comparison
39: abi.encode(https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L39
64: abi.encode(https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L64
The overheads outlined below are PER LOOP, excluding the first loop
storage arrays incur a Gwarmaccess (100 gas) memory arrays use MLOAD (3 gas) calldata arrays use CALLDATALOAD (3 gas)
Caching the length changes each of these to a DUP (3 gas), and gets rid of the extra DUP needed to store the stack offset
192: for (uint i=0; i<helpers.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L192
196: for (uint j=i+1; j<helpers.length; j++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L196
312: for (uint256 i = 0; i<addressesToWipe.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L312
Saves ~13 gas per instance
93: constructor(Frankencoin zchf_) ERC20(18)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L93
59: constructor(uint8 _decimals)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L59
59: constructor(uint256 _minApplicationPeriod) ERC20(18)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L59
54: constructor(address _zchf, address factory)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L54
50: constructor(address _owner, address _hub, address _zchf, address _collateral,
uint256 _minCollateral, uint256 _initialLimit, uint256 initPeriod, uint256 _duration,
uint256 _challengePeriod, uint32 _mintingFeePPM, uint256 _liqPrice, uint32 _reservePPM)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L50
26: constructor(address other, address zchfAddress, uint256 limit_)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L26
Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.
41: uint256 private constant MINIMUM_EQUITY = 1000 * ONE_DEC18;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L41
59: uint256 public constant MIN_HOLDING_DURATION = 90*7200 << BLOCK_TIME_RESOLUTION_BITS; // Set to 5 for local testinghttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L59
25: uint256 public constant MIN_FEE = 1000 * (10**18);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L25
10: uint256 internal constant ONE_DEC18 = 10**18;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L10
11: uint256 internal constant THRESH_DEC18 = 10000000000000000;//0.01https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L11
20: uint256 public constant OPENING_FEE = 1000 * 10**18;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L20
26: uint32 public constant CHALLENGER_REWARD = 20000; // 2%https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L26
Saves deployment costs
152: require(recipient != address(0));
180: require(recipient != address(0));
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L152
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L180
158: require(challenge.challenger != address(0x0));
254: require(challenge.challenger != address(0x0));
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L158
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L254
Saves 6 gas per loop
192: for (uint i=0; i<helpers.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L192
312: for (uint256 i = 0; i<addressesToWipe.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L312
For example, use ++i instead of i++
337: _size = colBal;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L337
If a function modifier or require such as onlyOwner/onlyX is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.
function mint(address _target, uint256 _amount, uint32 _reservePPM, uint32 _feesPPM) override external minterOnly {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L165
function burn(uint256 amount, uint32 reservePPM) external override minterOnly {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L194
223: function burnFrom(address payer, uint256 targetTotalBurnAmount, uint32 _reservePPM) external override minterOnly returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L223
251: function burnWithReserve(uint256 _amountExcludingReserve, uint32 _reservePPM) external override minterOnly returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L251
280: function notifyLoss(uint256 _amount) override external minterOnly {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L280
31: function transferOwnership(address newOwner) public onlyOwner {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L31
Functions guaranteed to revert when called by normal users can be marked payable.
Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundry's script.sol and solmate's LibRlp.sol contracts can help achieve this.
References: https://book.getfoundry.sh/reference/forge-std/compute-create-address
https://twitter.com/transmissions11/status/1518507047943245824
68: address(this)
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L68
142: IERC20(position.collateral()).transferFrom(msg.sender, address(this), _collateralAmount);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L142
225: zchf.transferFrom(msg.sender, address(this), _bidAmountZCHF);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L225
138: collateral.transferFrom(msg.sender, address(this), newCollateral - colbal);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L138
170: return IERC20(collateral).balanceOf(address(this));
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L170
228: IERC20(zchf).transferFrom(msg.sender, address(this), amount);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L228
45: chf.transferFrom(msg.sender, address(this), amount);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L45
51: require(chf.balanceOf(address(this)) <= limit, "limit");
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L51
79: burnInternal(address(this), from, amount);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L79
Use hardcoded address
[GAS‑10] It Costs More Gas To Initialize Variables To Zero Than To Let The Default Of Zero Be Applied
312: for (uint256 i = 0; i<addressesToWipe.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L312
112: function _beforeTokenTransfer
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L112
144: function adjustTotalVotes
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L144
157: function adjustRecipientVoteAnchor
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L157
179: function _mint
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L179
200: function _burn
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L200
102: function allowanceInternal
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L102
18: function _cubicRoot
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L18
35: function _divD18
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L35
39: function _power3
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L39
287: function returnCollateral
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L287
39: function setOwner
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L39
45: function requireOwner
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L45
232: function repayInternal
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L232
37: function createClone
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L37
* 2 is equivalent to << 1 and / 2 is the same as >> 1. The MUL and DIV opcodes cost 5 gas, whereas SHL and SHR only cost 3 gas
98: uint256 reduction = (limit - minted - _minimum)/2;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L98
Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.
See more here
All in-scope contracts
Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Gauge.sol contract will be the most used; A lower method ID may be given.
199: _votes += votes(current);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L199
156: _balances[sender] -= amount;
157: _balances[recipient] += amount;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L156-L157
184: _totalSupply += amount;
185: _balances[recipient] += amount;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L184-L185
203: _totalSupply -= amount;
204: _balances[account] -= amount;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L203-L204
169: minterReserveE6 += _amount * _reservePPM;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L169
196: minterReserveE6 -= amount * reservePPM;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L196
227: minterReserveE6 -= targetTotalBurnAmount * _reservePPM;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L227
253: minterReserveE6 -= freedAmount * _reservePPM;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L253
167: challenge.bid -= copy.bid;
168: challenge.size -= copy.size;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L167-L168
291: pendingReturns[collateral][challenge.challenger] += challenge.size;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L291
99: limit -= reduction + _minimum;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L99
196: minted += amount;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L196
242: minted -= amount;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L242
295: challengedAmount += size;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L295
309: challengedAmount -= _collateralAmount;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L309
330: challengedAmount -= _size;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L330
The following structs contain time variables that are unlikely to ever reach max uint256 then it is possible to set them as uint64, saving gas slots.
39: struct Challenge {
address challenger; // the address from which the challenge was initiated
IPosition position; // the position that was challenged
uint256 size; // how much collateral the challenger provided
uint256 end; // the deadline of the challenge (block.timestamp)
address bidder; // the address from which the highest bid was made, if any
uint256 bid; // the highest bid in ZCHF (total amount, not price per unit)
}https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L39
Can save one storage slot by changing to:
struct Challenge {
address challenger; // the address from which the challenge was initiated
IPosition position; // the position that was challenged
uint256 size; // how much collateral the challenger provided
address bidder; // the address from which the highest bid was made, if any
uint64 end; // the deadline of the challenge (block.timestamp)
uint256 bid; // the highest bid in ZCHF (total amount, not price per unit)
}Can save an additional storage slot, if we do not expect that size and bid will ever reach max uint128, by changing to:
struct Challenge {
address challenger; // the address from which the challenge was initiated
IPosition position; // the position that was challenged
uint128 size; // how much collateral the challenger provided
uint128 bid; // the highest bid in ZCHF (total amount, not price per unit)
address bidder; // the address from which the highest bid was made, if any
uint64 end; // the deadline of the challenge (block.timestamp)
}Potentially a max total of 2 storage slots can be saved.
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table
39: uint32 public constant VALUATION_FACTOR = 3;
59: uint256 public constant MIN_HOLDING_DURATION = 90*7200 << BLOCK_TIME_RESOLUTION_BITS; // Set to 5 for local testing
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L39
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L59
25: uint256 public constant MIN_FEE = 1000 * (10**18);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L25
20: uint256 public constant OPENING_FEE = 1000 * 10**18;
26: uint32 public constant CHALLENGER_REWARD = 20000; // 2%
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L20
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L26
Set variable to private.
The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.
function price() public view returns (uint256){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L108
function canRedeem(address owner) public view returns (bool) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L127
function votes(address holder) public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L179
function totalVotes() public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L186
function votes(address sender, address[] calldata helpers) public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L179
function checkQualified(address sender, address[] calldata helpers) public override view {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L209
function calculateShares(uint256 investment) public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L262
function redeem(address target, uint256 shares) public returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L275
function calculateProceeds(uint256 shares) public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L290
function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L309
function totalSupply() public view override returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L66
function balanceOf(address account) public view override returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L73
function transfer(address recipient, uint256 amount) public virtual override returns (bool) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L85
function permit(
address owner,
address spender,
uint256 value,
uint256 deadline,
uint8 v,
bytes32 r,
bytes32 s
) public {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L21
function DOMAIN_SEPARATOR() public view returns (bytes32) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L61
function minterReserve() public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L117
function equity() public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L138
function calculateAssignedReserve(uint256 mintedAmount, uint32 _reservePPM) public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L204
function calculateFreedAmount(uint256 amountExcludingReserve , uint32 reservePPM ) public view returns (uint256){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L235
function isMinter(address _minter) override public view returns (bool){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L293
function isPosition(address _position) override public view returns (address){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L300
function openPosition(
address _collateralAddress, uint256 _minCollateral, uint256 _initialCollateral,
uint256 _mintingMaximum, uint256 _expirationSeconds, uint256 _challengeSeconds,
uint32 _mintingFeePPM, uint256 _liqPrice, uint32 _reservePPM) public returns (address) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L59
function openPosition(
address _collateralAddress, uint256 _minCollateral, uint256 _initialCollateral,
uint256 _mintingMaximum, uint256 _initPeriodSeconds, uint256 _expirationSeconds, uint256 _challengeSeconds,
uint32 _mintingFeePPM, uint256 _liqPrice, uint32 _reservePPM) public returns (address) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L59
function clonePosition(address position, uint256 _initialCollateral, uint256 _initialMint) public validPos(position) returns (address) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L124
function minBid(uint256 challenge) public view returns (uint256) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L181
function end(uint256 _challengeNumber, bool postponeCollateralReturn) public {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L235
function transferOwnership(address newOwner) public onlyOwner {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L31
function deny(address[] calldata helpers, string calldata message) public {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L109
function getUsableMint(uint256 totalMint, bool afterFees) public view returns (uint256){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L120
function adjust(uint256 newMinted, uint256 newCollateral, uint256 newPrice) public onlyOwner {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L132
function adjustPrice(uint256 newPrice) public onlyOwner noChallenge {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L159
function mint(address target, uint256 amount) public onlyOwner noChallenge noCooldown alive {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L177
function calculateCurrentFee() public view returns (uint32) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L181
function repay(uint256 amount) public onlyOwner {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L227
function withdrawCollateral(address target, uint256 amount) public onlyOwner noChallenge noCooldown {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L263
function isClosed() public view returns (bool) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L360
function mint(address target, uint256 amount) public {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L36
The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files
A good example:
import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {SafeCastLib} from "solmate/utils/SafeCastLib.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IProducer} from "src/interfaces/IProducer.sol";
import {GlobalState, UserState} from "src/Common.sol";6: import "./Frankencoin.sol";
7: import "./IERC677Receiver.sol";
8: import "./ERC20PermitLight.sol";
9: import "./MathUtil.sol";
10: import "./IReserve.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L6-L10
14: import "./IERC20.sol";
15: import "./IERC677Receiver.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L14-L15
7: import "./ERC20.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L7
4: import "./ERC20PermitLight.sol";
5: import "./Equity.sol";
6: import "./IReserve.sol";
7: import "./IFrankencoin.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L4-L7
4: import "./IERC20.sol";
5: import "./IReserve.sol";
6: import "./IFrankencoin.sol";
7: import "./Ownable.sol";
8: import "./IPosition.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L4-L8
4: import "./IERC20.sol";
5: import "./IPosition.sol";
6: import "./IReserve.sol";
7: import "./IFrankencoin.sol";
8: import "./Ownable.sol";
9: import "./MathUtil.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L4-L9
4: import "./Position.sol";
5: import "./IFrankencoin.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L4-L5
4: import "./IERC20.sol";
5: import "./IERC677Receiver.sol";
6: import "./IFrankencoin.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L4-L6
Use specific imports syntax per solidity docs recommendation.
Instead of using operator && on a single require. Using a two require can save more gas.
i.e.
for require(version == 1 && _bytecodeHash[1] == bytes1(0), "zf"); use:
require(version == 1);
require(_bytecodeHash[1] == bytes1(0));56: require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L56
require(recoveredAddress != address(0));
require(recoveredAddress == owner, "INVALID_SIGNER");If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper
Since uint256 internal constant ONE_DEC18 = 10**18;, it is possible to lower MINIMUM_EQUITY from uint256 to a lower to something that can hold 1000 * 10**18. As a result we can save at least 1 storage slot by even changing to uint224 and potentially more if we lower the uint more.
20: contract Equity is ERC20PermitLight, MathUtil, IReserve {
...
uint32 public constant VALUATION_FACTOR = 3;
uint256 private constant MINIMUM_EQUITY = 1000 * ONE_DEC18;
uint32 private constant QUORUM = 300;
uint8 private constant BLOCK_TIME_RESOLUTION_BITS = 24;
uint256 public constant MIN_HOLDING_DURATION = 90*7200 << BLOCK_TIME_RESOLUTION_BITS; // Set to 5 for local testing
Frankencoin immutable public zchf;
uint192 private totalVotesAtAnchor; // Total number of votes at the anchor time, see comment on the um
uint64 private totalVotesAnchorTime; // 40 Bit for the block number, 24 Bit sub-block time resolution
mapping (address => address) public delegates;
mapping (address => uint64) private voteAnchor; // 40 Bit for the block number, 24 Bit sub-block time resolution
event Delegation(address indexed from, address indexed to); // indicates a delegation
event Trade(address who, int amount, uint totPrice, uint newprice); // amount pos or neg for mint or redemption
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L20
When using elements that are smaller than 32 bytes, your contract's gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed
39: uint32 public constant VALUATION_FACTOR = 3;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L39
46: uint32 private constant QUORUM = 300;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L46
51: uint8 private constant BLOCK_TIME_RESOLUTION_BITS = 24;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L51
75: uint192 private totalVotesAtAnchor;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L75
76: uint64 private totalVotesAnchorTime;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L76
51: uint8 public immutable override decimals;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L51
26: uint32 public constant CHALLENGER_REWARD = 20000;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L26
38: uint32 public immutable mintingFeePPM;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L38
39: uint32 public immutable reserveContribution;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L39
352: internalWithdrawCollateral(_bidder, _size);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L352
[GAS‑22] ++i/i++ Should Be unchecked{++i}/unchecked{i++} When It Is Not Possible For Them To Overflow, As Is The Case When Used In For- And While-loops
The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas PER LOOP
192: for (uint i=0; i<helpers.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L192
196: for (uint j=i+1; j<helpers.length; j++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L196
312: for (uint256 i = 0; i<addressesToWipe.length; i++){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L312
Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block
294: uint256 newTotalShares = totalShares - shares;
296: return capital - newCapital;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L294
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L296
If the else-condition isn't required, the second condition will have been looked up unnecessarily.
106: } else if (isMinter(spender) || isMinter(isPosition(spender))){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L106
To save some gas the use of custom errors leads to cheaper deploy time cost and run time cost. The run time cost is only relevant when the revert condition is met.
File: ./Projects/frankencoin-2023-04/2023-04-frankencoin/contracts/Equity.solhttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L293
File: ./Projects/frankencoin-2023-04/2023-04-frankencoin/contracts/ERC20PermitLight.solhttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L56
File: ./Projects/frankencoin-2023-04/2023-04-frankencoin/contracts/MintingHub.solhttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L255
File: ./Projects/frankencoin-2023-04/2023-04-frankencoin/contracts/StablecoinBridge.solhttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L81
Use Custom Errors instead of strings.
Upgrade to the latest solidity version 0.8.19 to get additional gas savings. See latest release for reference: https://blog.soliditylang.org/2023/02/22/solidity-0.8.19-release-announcement/
pragma solidity >=0.8.0 <0.9.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L4
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L12
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L5
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L2
pragma solidity >=0.8.0 <0.9.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L3
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L2
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L9
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L2
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L2
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L2
In Solidity, a constant expression in a variable will compute the expression everytime the variable is called. It's not the result of the expression that is stored, but the expression itself.
As Solidity supports the scientific notation, constants of form 10**X can be rewritten as 1eX to save the gas cost from the calculation with the exponentiation operator **.
25: uint256 public constant MIN_FEE = 1000 * (10**18);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L25
10: uint256 internal constant ONE_DEC18 = 10**18;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L10
20: uint256 public constant OPENING_FEE = 1000 * 10**18;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L20
Replace 10**X with 1eX
| Issue | Contexts | |
|---|---|---|
| LOW‑1 | Use of ecrecover is susceptible to signature malleability |
1 |
| LOW‑2 | Event is missing parameters | 2 |
| LOW‑3 | Possible rounding issue | 1 |
| LOW‑4 | Low Level Calls With Solidity Version 0.8.14 Can Result In Optimiser Bug | 1 |
| LOW‑5 | Minting tokens to the zero address should be avoided | 1 |
| LOW‑6 | Missing Checks for Address(0x0) | 1 |
| LOW‑7 | Prevent division by 0 | 2 |
| LOW‑8 | Use safetransfer Instead Of transfer |
20 |
| LOW‑9 | Admin privilege - A single point of failure can allow a hacked or malicious owner use critical functions in the project | 7 |
| LOW‑10 | TransferOwnership Should Be Two Step | 1 |
| LOW‑11 | Unbounded loop | 2 |
| LOW‑12 | Use safeTransferOwnership instead of transferOwnership function |
1 |
Total: 49 contexts over 12 issues
| Issue | Contexts | |
|---|---|---|
| NC‑1 | Add a timelock to critical functions | 1 |
| NC‑2 | Avoid Floating Pragmas: The Version Should Be Locked | 8 |
| NC‑3 | Constants Should Be Defined Rather Than Using Magic Numbers | 5 |
| NC‑4 | Critical Changes Should Use Two-step Procedure | 1 |
| NC‑5 | Declare interfaces on separate files | 1 |
| NC‑6 | Duplicated require()/revert() Checks Should Be Refactored To A Modifier Or Function |
4 |
| NC‑7 | Event Is Missing Indexed Fields | 3 |
| NC‑8 | Function writing that does not comply with the Solidity Style Guide | 10 |
| NC‑9 | Large or complicated code bases should implement fuzzing tests | 1 |
| NC‑10 | Imports can be grouped together | 11 |
| NC‑11 | NatSpec return parameters should be included in contracts | 1 |
| NC‑12 | Initial value check is missing in Set Functions | 1 |
| NC‑13 | Lines are too long | 1 |
| NC‑14 | Implementation contract may not be initialized | 6 |
| NC‑15 | NatSpec comments should be increased in contracts | 1 |
| NC‑16 | Non-usage of specific imports | 28 |
| NC‑17 | Use a more recent version of Solidity | 10 |
| NC‑18 | Open TODOs | 1 |
| NC‑19 | Using >/>= without specifying an upper bound is unsafe |
2 |
| NC‑20 | Public Functions Not Called By The Contract Should Be Declared External Instead | 3 |
| NC‑21 | Empty blocks should be removed or emit something | 1 |
| NC‑22 | require() / revert() Statements Should Have Descriptive Reason Strings |
10 |
| NC‑23 | Large multiples of ten should use scientific notation | 6 |
| NC‑24 | Use bytes.concat() |
1 |
| NC‑25 | Use Underscores for Number Literals | 6 |
Total: 123 contexts over 25 issues
The built-in EVM precompile ecrecover is susceptible to signature malleability, which could lead to replay attacks.
References: https://swcregistry.io/docs/SWC-117, https://swcregistry.io/docs/SWC-121, and https://medium.com/cryptronics/signature-replay-vulnerabilities-in-smart-contracts-3b6f7596df57.
While this is not immediately exploitable, this may become a vulnerability if used elsewhere.
33: function permit(
address owner,
address spender,
uint256 value,
uint256 deadline,
uint8 v,
bytes32 r,
bytes32 s
) public {
require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED");
unchecked {
address recoveredAddress = ecrecover(
keccak256(
abi.encodePacked(
"/x19/x01",
DOMAIN_SEPARATOR(),
keccak256(
abi.encode(
bytes32(0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9),
owner,
spender,
value,
nonces[owner]++,
deadline
)
)
)
),
v,
r,
s
);
require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");
_approve(recoveredAddress, spender, value);
}
}https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L33
Consider using OpenZeppelin’s ECDSA library (which prevents this malleability) instead of the built-in function. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/cryptography/ECDSA.sol#L138-L149
The following functions are missing critical parameters when emitting an event.
When dealing with source address which uses the value of msg.sender, the msg.sender value must be specified in every transaction, a contract or web page listening to events cannot react to users, emit does not serve the purpose. Basically, this event cannot be used.
function suggestMinter(address _minter, uint256 _applicationPeriod, uint256 _applicationFee, string calldata _message) override external {
if (_applicationPeriod < MIN_APPLICATION_PERIOD && totalSupply() > 0) revert PeriodTooShort();
if (_applicationFee < MIN_FEE && totalSupply() > 0) revert FeeTooLow();
if (minters[_minter] != 0) revert AlreadyRegistered();
_transfer(msg.sender, address(reserve), _applicationFee);
minters[_minter] = block.timestamp + _applicationPeriod;
emit MinterApplied(_minter, _applicationPeriod, _applicationFee, _message);
}https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L89
function denyMinter(address _minter, address[] calldata _helpers, string calldata _message) override external {
if (block.timestamp > minters[_minter]) revert TooLate();
reserve.checkQualified(msg.sender, _helpers);
delete minters[_minter];
emit MinterDenied(_minter, _message);
}https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L156
Add msg.sender parameter in event-emit
Division by large numbers may result in the result being zero, due to solidity not supporting fractions. Consider requiring a minimum amount for the numerator to ensure that it is always larger than the denominator. Also, there is indication of multiplication and division without the use of parenthesis which could result in issues.
238: uint256 adjustedReservePPM = currentReserve < minterReserve_ ? reservePPM * currentReserve / minterReserve_ : reservePPM;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L238
The project contracts in scope are using low level calls with solidity version before 0.8.14 which can result in optimizer bug. https://medium.com/certora/overly-optimistic-optimizer-certora-bug-disclosure-2101e3f7994d
Simliar findings in Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#5-low-level-calls-with-solidity-version-0814-can-result-in-optimiser-bug
POC can be found in the above medium reference url.
Functions that execute low level calls in contracts with solidity version under 0.8.14
2: pragma solidity ^0.8.0;
...
39: assembly {
let clone := mload(0x40)
mstore(clone, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
mstore(add(clone, 0x14), targetBytes)
mstore(add(clone, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
result := create(0, clone, 0x37)
}https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L39
Consider upgrading to at least solidity v0.8.15.
The core function mint is used by users to mint an option position by providing token1 as collateral and borrowing the max amount of liquidity. Address(0) check is missing in both this function and the internal function _mint, which is triggered to mint the tokens to the to address. Consider applying a check in the function to ensure tokens aren't minted to the zero address.
function mint(address _target, uint256 _amount, uint32 _reservePPM, uint32 _feesPPM) override external minterOnly {
uint256 usableMint = (_amount * (1000_000 - _feesPPM - _reservePPM)) / 1000_000;
_mint(_target, usableMint);
_mint(address(reserve), _amount - usableMint);
minterReserveE6 += _amount * _reservePPM;
}
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L165
Lack of zero-address validation on address parameters may lead to transaction reverts, waste gas, require resubmission of transactions and may even force contract redeployments in certain cases within the protocol.
220: function delegateVoteTo: address delegatehttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L220
Consider adding explicit zero-address validation on input parameters of address type.
On several locations in the code precautions are not being taken for not dividing by 0, this will revert the code.
These functions can be called with 0 value in the input, this value is not checked for being bigger than 0, that means in some scenarios this can potentially trigger a division by zero.
36: return (_a * ONE_DEC18) / _bhttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L36
80: price = _mint * ONE_DEC18 / _collhttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L80
Recommend making sure division by 0 won’t occur by checking the variables beforehand and handling this edge case.
It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.
For example, Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert.
279: zchf.transfer(target, proceeds);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L279
108: zchf.transferFrom(msg.sender, address(zchf.reserve()), OPENING_FEE);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L108
110: IERC20(_collateralAddress).transferFrom(msg.sender, address(pos), _initialCollateral);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L110
129: existing.collateral().transferFrom(msg.sender, address(pos), _initialCollateral);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L129
142: IERC20(position.collateral()).transferFrom(msg.sender, address(this), _collateralAmount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L142
204: zchf.transfer(challenge.bidder, challenge.bid);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L204
210: zchf.transferFrom(msg.sender, challenge.challenger, _bidAmountZCHF);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L210
211: challenge.position.collateral().transfer(msg.sender, challenge.size);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L211
225: zchf.transferFrom(msg.sender, address(this), _bidAmountZCHF);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L225
263: IERC20(zchf).transfer(challenge.bidder, challenge.bid - effectiveBid);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L263
268: zchf.transfer(owner, effectiveBid - fundsNeeded);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L268
272: zchf.transfer(challenge.challenger, reward);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L272
284: IERC20(collateral).transfer(target, amount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L284
294: challenge.position.collateral().transfer(challenge.challenger, challenge.size);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L294
138: collateral.transferFrom(msg.sender, address(this), newCollateral - colbal);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L138
228: IERC20(zchf).transferFrom(msg.sender, address(this), amount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L228
253: IERC20(token).transfer(target, amount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L253
269: IERC20(collateral).transfer(target, amount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L269
45: chf.transferFrom(msg.sender, address(this), amount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L45
69: chf.transfer(target, amount);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L69
Consider using safeTransfer/safeTransferFrom or require() consistently.
[LOW‑9] Admin privilege - A single point of failure can allow a hacked or malicious owner use critical functions in the project
The owner role has a single point of failure and onlyOwner can use critical a few functions.
owner role in the project:
Owner is not behind a multisig and changes are not behind a timelock.
Even if protocol admins/developers are not malicious there is still a chance for Owner keys to be stolen. In such a case, the attacker can cause serious damage to the project due to important functions. In such a case, users who have invested in project will suffer high financial losses.
Hacked owner or malicious owner can immediately use critical functions in the project.
31: function transferOwnership(address newOwner) public onlyOwner {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L31
132: function adjust(uint256 newMinted, uint256 newCollateral, uint256 newPrice) public onlyOwner {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L132
159: function adjustPrice(uint256 newPrice) public onlyOwner noChallenge {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L159
177: function mint(address target, uint256 amount) public onlyOwner noChallenge noCooldown alive {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L177
227: function repay(uint256 amount) public onlyOwner {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L227
249: function withdraw(address token, address target, uint256 amount) external onlyOwner {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L249
263: function withdrawCollateral(address target, uint256 amount) public onlyOwner noChallenge noCooldown {
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L263
Add a time lock to critical functions. Admin-only functions that change critical parameters should emit events and have timelocks. Events allow capturing the changed parameters so that off-chain tools/interfaces can register such changes with timelocks that allow users to evaluate them and consider if they would like to engage/exit based on how they perceive the changes as affecting the trustworthiness of the protocol or profitability of the implemented financial services.
Allow only multi-signature wallets to call the function to reduce the likelihood of an attack.
https://twitter.com/danielvf/status/1572963475101556738?s=20&t=V1kvzfJlsx-D2hfnG0OmuQ
Recommend considering implementing a two step process where the owner or admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.
31: function transferOwnership: function transferOwnership(address newOwner) public onlyOwner {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L31
Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
New items are pushed into the challenges array.
Currently, the array can grow indefinitely. E.g. there's no maximum limit and there's no functionality to remove array values.
If the array grows too large, calling functions that use challenges might run out of gas and revert.
144: challenges.push(Challenge(msg.sender, position, _collateralAmount, block.timestamp + position.challengePeriod(), address(0x0), 0));
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L144
175: challenges.push(copy);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L175
Add a functionality to delete array values or add a maximum size limit for arrays.
Use safeTransferOwnership which is safer. Use it as it is more secure due to 2-stage ownership transfer.
31: function transferOwnership: function transferOwnership(address newOwner) public onlyOwner {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L31
Use Ownable2Step.sol
function acceptOwnership() external {
address sender = _msgSender();
require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
_transferOwnership(sender);
}
}It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate (less risk of a malicious owner making a sandwich attack on a user). Consider adding a timelock to the following functions:
39: function setOwner(address newOwner) internal {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L39
Avoid floating pragmas for non-library contracts.
While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.
A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.
It is recommended to pin to a concrete compiler version.
Found usage of floating pragmas ^0.8.0 of Solidity in [ERC20.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L12
Found usage of floating pragmas ^0.8.0 of Solidity in [ERC20PermitLight.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L5
Found usage of floating pragmas ^0.8.0 of Solidity in [Frankencoin.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L2
Found usage of floating pragmas ^0.8.0 of Solidity in [MintingHub.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L2
Found usage of floating pragmas ^0.8.0 of Solidity in [Ownable.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L9
Found usage of floating pragmas ^0.8.0 of Solidity in [Position.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L2
Found usage of floating pragmas ^0.8.0 of Solidity in [PositionFactory.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L2
Found usage of floating pragmas ^0.8.0 of Solidity in [StablecoinBridge.sol]https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L2
253: require(totalSupply() < 2**128, "total supply exceeded");https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L253
64: 7 dayshttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L64
189: return (challenge.bid * 1005) / 1000;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L189
161: restrictMinting(3 days);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L161
312: restrictMinting(1 days);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L312
The critical procedures should be two step process.
See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure
39: function setOwner(address newOwner) internal {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L39
Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
Interfaces should be declared on a separate file and not on the same file where the contract resides in.
299: interface IPositionFactory {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L299
Saves deployment costs
152: require(recipient != address(0));
180: require(recipient != address(0));
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L152
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L180
158: require(challenge.challenger != address(0x0));
254: require(challenge.challenger != address(0x0));
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L158
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L254
Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields).
Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
event Trade(address who, int amount, uint totPrice, uint newprice);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L91
event NewBid(uint256 challengedId, uint256 bidAmount, address bidder);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L51
event MintingUpdate(uint256 collateral, uint256 price, uint256 minted, uint256 limit);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L42
Order of Functions; ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this.
https://docs.soliditylang.org/en/v0.8.17/style-guide.html
Functions should be grouped according to their visibility and ordered:
- constructor
- receive function (if exists)
- fallback function (if exists)
- external
- public
- internal
- private
- within a grouping, place the view and pure functions last
All in-scope contracts
See Position.sol for example.
Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement fuzzing tests. Fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.
Various in-scope contract files.
Consider importing FrankenCoin-related files first, then all interfaces, then all utils.
6: import "./Frankencoin.sol";
7: import "./IERC677Receiver.sol";
8: import "./ERC20PermitLight.sol";
9: import "./MathUtil.sol";
10: import "./IReserve.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L6-L10
4: import "./IERC20.sol";
5: import "./IPosition.sol";
6: import "./IReserve.sol";
7: import "./IFrankencoin.sol";
8: import "./Ownable.sol";
9: import "./MathUtil.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L4-L9
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability.
https://docs.soliditylang.org/en/v0.8.15/natspec-format.html
All in-scope contracts
See functions Position.sol for example.
Include return parameters in NatSpec comments
Recommendation Code Style: (from Uniswap3)
/// @notice Adds liquidity for the given recipient/tickLower/tickUpper position
/// @dev The caller of this method receives a callback in the form of IUniswapV3MintCallback#uniswapV3MintCallback
/// in which they must pay any token0 or token1 owed for the liquidity. The amount of token0/token1 due depends
/// on tickLower, tickUpper, the amount of liquidity, and the current price.
/// @param recipient The address for which the liquidity will be created
/// @param tickLower The lower tick of the position in which to add liquidity
/// @param tickUpper The upper tick of the position in which to add liquidity
/// @param amount The amount of liquidity to mint
/// @param data Any data that should be passed through to the callback
/// @return amount0 The amount of token0 that was paid to mint the given amount of liquidity. Matches the value in the callback
/// @return amount1 The amount of token1 that was paid to mint the given amount of liquidity. Matches the value in the callback
function mint(
address recipient,
int24 tickLower,
int24 tickUpper,
uint128 amount,
bytes calldata data
) external returns (uint256 amount0, uint256 amount1);A check regarding whether the current value and the new value are the same should be added
39: function setOwner(address newOwner) internal {
address oldOwner = owner;
owner = newOwner;
emit OwnershipTransferred(oldOwner, newOwner);
}https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L39
Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length
53: // Copied from https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4139/files#diff-fa792f7d08644eebc519dac2c29b00a54afc4c6a76b9ef3bba56c8401fe674f6https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L53
OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5
93: constructor(Frankencoin zchf_) ERC20(18)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L93
59: constructor(uint8 _decimals)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L59
59: constructor(uint256 _minApplicationPeriod) ERC20(18)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L59
54: constructor(address _zchf, address factory)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L54
50: constructor(address _owner, address _hub, address _zchf, address _collateral,
uint256 _minCollateral, uint256 _initialLimit, uint256 initPeriod, uint256 _duration,
uint256 _challengePeriod, uint32 _mintingFeePPM, uint256 _liqPrice, uint32 _reservePPM)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L50
26: constructor(address other, address zchfAddress, uint256 limit_)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L26
It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html
All in-scope contracts
See MintingHub.sol.openPosition() for example.
NatSpec comments should be increased in contracts
The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files
A good example:
import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol";
import {SafeCastLib} from "solmate/utils/SafeCastLib.sol";
import {ERC20} from "solmate/tokens/ERC20.sol";
import {IProducer} from "src/interfaces/IProducer.sol";
import {GlobalState, UserState} from "src/Common.sol";6: import "./Frankencoin.sol";
7: import "./IERC677Receiver.sol";
8: import "./ERC20PermitLight.sol";
9: import "./MathUtil.sol";
10: import "./IReserve.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L6-L10
14: import "./IERC20.sol";
15: import "./IERC677Receiver.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L14-L15
7: import "./ERC20.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L7
4: import "./ERC20PermitLight.sol";
5: import "./Equity.sol";
6: import "./IReserve.sol";
7: import "./IFrankencoin.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L4-L7
4: import "./IERC20.sol";
5: import "./IReserve.sol";
6: import "./IFrankencoin.sol";
7: import "./Ownable.sol";
8: import "./IPosition.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L4-L8
4: import "./IERC20.sol";
5: import "./IPosition.sol";
6: import "./IReserve.sol";
7: import "./IFrankencoin.sol";
8: import "./Ownable.sol";
9: import "./MathUtil.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L4-L9
4: import "./Position.sol";
5: import "./IFrankencoin.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L4-L5
4: import "./IERC20.sol";
5: import "./IERC677Receiver.sol";
6: import "./IFrankencoin.sol";
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L4-L6
Use specific imports syntax per solidity docs recommendation.
0.8.4: bytes.concat() instead of abi.encodePacked(,)
0.8.12: string.concat() instead of abi.encodePacked(,)
0.8.13: Ability to use using for with a list of free functions
ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.
Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.
Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.
Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.
0.8.19: SMTChecker: New trusted mode that assumes that any compile-time available code is the actual used code, even in external calls. Bug Fixes:
- Assembler: Avoid duplicating subassembly bytecode where possible.
- Code Generator: Avoid including references to the deployed label of referenced functions if they are called right away.
- ContractLevelChecker: Properly distinguish the case of missing base constructor arguments from having an unimplemented base function.
- SMTChecker: Fix internal error caused by unhandled z3 expressions that come from the solver when bitwise operators are used.
- SMTChecker: Fix internal error when using the custom NatSpec annotation to abstract free functions.
- TypeChecker: Also allow external library functions in using for.
pragma solidity >=0.8.0 <0.9.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L4
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L12
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L5
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L2
pragma solidity >=0.8.0 <0.9.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L3
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L2
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Ownable.sol#L9
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L2
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/PositionFactory.sol#L2
pragma solidity ^0.8.0;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/StablecoinBridge.sol#L2
Consider updating to a more recent solidity version.
An open TODO is present. It is recommended to avoid open TODOs as they may indicate programming errors that still need to be fixed.
74: * TODO: in future versions, it might be better to fix the interest and not the feehttps://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L74
There will be breaking changes in future versions of solidity, and at that point your code will no longer be compatable. While you may have the specific version to use in a configuration file, others that include your source files may not.
4: pragma solidity >=0.8.0 <0.9.0;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L4
3: pragma solidity >=0.8.0 <0.9.0;
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L3
Contracts are allowed to override their parents’ functions and change the visibility from external to public.
function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L309
function getUsableMint(uint256 totalMint, bool afterFees) public view returns (uint256){https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L120
function isClosed() public view returns (bool) {https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L360
240: function _beforeTokenTransfer(address from, address to, uint256 amount) virtual internal {
}
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L240
194: require(current != sender);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L194
197: require(current != helpers[j]);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L197
276: require(canRedeem(msg.sender));https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L276
310: require(zchf.equity() < MINIMUM_EQUITY);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L310
152: require(recipient != address(0));https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L152
180: require(recipient != address(0));https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20.sol#L180
158: require(challenge.challenger != address(0x0));https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L158
171: require(challenge.size >= min);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L171
172: require(copy.size >= min);https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L172
254: require(challenge.challenger != address(0x0));https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L254
Use (e.g. 1e6) rather than decimal literals (e.g. 100000), for better code readability.
211: if (_votes * 10000 < QUORUM * totalVotes()) revert NotQualified();https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L211
118: return minterReserveE6 / 1000000;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L118
205: uint256 theoreticalReserve = _reservePPM * mintedAmount / 1000000;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L205
239: return 1000000 * amountExcludingReserve / (1000000 - adjustedReservePPM); // 41 / (1-18%) = 50https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L239
11: uint256 internal constant THRESH_DEC18 = 10000000000000000;//0.01https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L11
217: * amount = minted * (1000000 - reservePPM)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L217
Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))
35: abi.encodePacked(
"/x19/x01",
DOMAIN_SEPARATOR(),
keccak256(
abi.encode(
// keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"),
bytes32(0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9),
owner,
spender,
value,
nonces[owner]++,
deadline
)
)
)
),
v,
r,
s
)
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/ERC20PermitLight.sol#L35
Use bytes.concat() and upgrade to at least Solidity version 0.8.4 if required.
211: if (_votes * 10000 < QUORUM * totalVotes()) revert NotQualified();https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L211
118: return minterReserveE6 / 1000000;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L118
205: uint256 theoreticalReserve = _reservePPM * mintedAmount / 1000000;https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L205
239: return 1000000 * amountExcludingReserve / (1000000 - adjustedReservePPM); // 41 / (1-18%) = 50https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Frankencoin.sol#L239
11: uint256 internal constant THRESH_DEC18 = 10000000000000000;//0.01https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MathUtil.sol#L11
217: * amount = minted * (1000000 - reservePPM)https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Position.sol#L217
| Issue | Contexts | |
|---|---|---|
| MED‑1 | Contracts do not work with fee-on-transfer tokens | 2 |
| MED‑2 | Consider the case where totalsupply is 0 |
1 |
Some tokens take a transfer fee (e.g. STA, PAXG), some do not currently charge a fee but may do so in the future (e.g. USDT, USDC).
Should a fee-on-transfer token be added as an asset and deposited, it could be abused to mint more shares.
In the current implementation the function calls on contracts of openPosition and launchChallenge do not work well with fee-on-transfer tokens as the amount variable is the pre-fee amount, including the fee, whereas the totalAssets do not include the fee anymore.
This affects the following contracts that send the ERC20 collateral to the contract address(this) or address(pos). Since there is no check for before and after balance of the transferred amount, this could case miscalculations that can be depdendant on the actual amount received.
110: IERC20(_collateralAddress).transferFrom(msg.sender, address(pos), _initialCollateral);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L110
142: IERC20(position.collateral()).transferFrom(msg.sender, address(this), _collateralAmount);
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/MintingHub.sol#L142
- Consider comparing before and after balance to get the actual transferred amount.
- Alternatively, disallow tokens with fee-on-transfer mechanics to be added as reward tokens.
Consider the case where totalsupply is 0. When totalsupply is 0, it should return 0 directly, because there will be an error of dividing by 0.
This would cause the affected functions to revert and as a result can lead to potential loss.
109: return VALUATION_FACTOR * zchf.equity() * ONE_DEC18 / totalSupply();
https://github.com/code-423n4/2023-04-frankencoin/tree/main/contracts/Equity.sol#L109
Add check for zero value and return 0.
if ( totalSupply() == 0) return 0;
Alex the Entreprenerd commented:
GAS‑1 abi.encode() is less efficient than abi.encodepacked() 2 200
Disputing, not the same
GAS‑2 .length Should Not Be Looked Up In Every Loop Of A For-loop 3 291
NC
GAS‑3 Setting the constructor to payable 6 78
Disputing
GAS‑4 Do not calculate constants 7 -
Disputing
GAS‑5 Duplicated require()/revert() Checks Should Be Refactored To A Modifier Or Function 4 112
Disputing
GAS‑6 ++i Costs Less Gas Than i++, Especially When It’s Used In For-loops (--i/i-- Too) 2 12
NC
GAS‑7 Use assembly to write address storage values 1 -
NC
GAS‑8 Functions guaranteed to revert when called by normal users can be marked payable 6 126
Disputing
GAS‑9 Use hardcoded address instead address(this) 9 -
Disputing
GAS‑10 It Costs More Gas To Initialize Variables To Zero Than To Let The Default Of Zero Be Applied 1 -
NC
GAS‑11 internal functions only called once can be inlined to save gas 14 308
NC
GAS‑12 Multiplication/division By Two Should Use Bit Shifting 1 -
NC
GAS‑13 Optimize names to save gas 10 220
Disputing
GAS‑14 += Costs More Gas Than = + For State Variables 20 -
NC
GAS‑15 Structs can be packed into fewer storage slots by editing time variables 2 4000
L
GAS‑16 Using private rather than public for constants, saves gas 5 -
Disputing
GAS‑17 Public Functions To External 37 -
Disputing
GAS‑18 Save gas with the use of specific import statements 28 -
Disputing
GAS‑19 Splitting require() Statements That Use && Saves Gas 1 9
NC
GAS‑20 State variables can be packed into fewer storage slots 1 2000
Invalid
GAS‑21 Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead 10 -
NC
GAS‑22 ++i/i++ Should Be unchecked{++i}/unchecked{i++} When It Is Not Possible For Them To Overflow, As Is The Case When Used In For- And While-loops 3 105
NC
GAS‑23 Using unchecked blocks to save gas 2 40
NC
GAS‑24 Unnecessary look up in if condition 1 2100
Unclear, ignoring
GAS‑25 Use of Custom Errors Instead of String 12 -
NC
GAS‑26 Use solidity version 0.8.19 to gain some gas boost 10 880
L
GAS‑27 Using 10**X for constants isn't gas efficient
Disputing