Frankencoin - slvDev's results

Low-Risk Issues List

Total 10 issues

Non-Critical Issues List

Total 12 issues

[L-01] Use safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom() for ERC20

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.

Use OpenZeppelin's SafeERC20's safeTransfer() and safeTransferFrom() instead.

File: .\contracts\MintingHub.sol

110: IERC20(_collateralAddress).transferFrom(msg.sender, address(pos), _initialCollateral);
263: IERC20(zchf).transfer(challenge.bidder, challenge.bid - effectiveBid);
284: IERC20(collateral).transfer(target, amount);

204: zchf.transfer(challenge.bidder, challenge.bid); // return old bid
268: zchf.transfer(owner, effectiveBid - fundsNeeded);
272: zchf.transfer(challenge.challenger, reward); // pay out the challenger reward

File: .\contracts\Position.sol

228: IERC20(zchf).transferFrom(msg.sender, address(this), amount);
253: IERC20(token).transfer(target, amount);
269: IERC20(collateral).transfer(target, amount);

File: .\contracts\Equity.sol

279: zchf.transfer(target, proceeds);

File: .\contracts\StablecoinBridge.sol

69: chf.transfer(target, amount);

[L-02] Signature Malleability of EVM's ecrecover()

Description: Description: The function calls the Solidity ecrecover() function directly to verify the given signatures. However, the ecrecover() EVM opcode allows malleable (non-unique) signatures and thus is susceptible to replay attacks.

Although a replay attack seems not possible for this contract, I recommend using the battle-tested OpenZeppelin's ECDSA library.

Recommendation:* Use the ecrecover function from OpenZeppelin's ECDSA library for signature verification. (Ensure using a version > 4.7.3 for there was a critical bug >= 4.1.0 < 4.7.3).

File: contracts\ERC20PermitLight.sol

32: unchecked { // unchecked to save a little gas with the nonce increment...
33:        address recoveredAddress = ecrecover(
34:            keccak256(

[L-03] Unsafe casting from uint256

Consider using OpenZeppelin’s SafeCast library to prevent unexpected overflows when casting from uint256.

File: .\\contracts\\Equity.sol

146: totalVotesAtAnchor = uint192(totalVotes() - roundingLoss - lostVotes);
161: voteAnchor[to] = uint64(anchorTime() - recipientVotes / newbalance); 
173: return uint64(block.number << BLOCK_TIME_RESOLUTION_BITS);

File: .\\contracts\\Position.sol

187: return uint32(mintingFeePPM - mintingFeePPM * (time - start) / (exp - start));

[L-04] Owner can renounce Ownership

The contract uses OpenZeppelin's Ownable(Upgradable).sol and contains onlyOwner functions. Consider implementing custom logic for renounceOwnership() to handle renouncing ownership.

onlyOwner functions:

File: .\\contracts\\Position.sol

132: function adjust(uint256 newMinted, uint256 newCollateral, uint256 newPrice) public onlyOwner {
159: function adjustPrice(uint256 newPrice) public onlyOwner noChallenge {
177: function mint(address target, uint256 amount) public onlyOwner noChallenge noCooldown alive {
227: function repay(uint256 amount) public onlyOwner {
249: function withdraw(address token, address target, uint256 amount) external onlyOwner {
263: function withdrawCollateral(address target, uint256 amount) public onlyOwner noChallenge noCooldown {

[L-05] Single-step transfer ownership

Consider using transferOwnership() from Ownable2Step.sol (Ownable2StepUpgradeable.sol) for a more secure 2-step ownership transfer.

File: .\\contracts\\MintingHub.sol

7: import "./Ownable.sol";

File: .\\contracts\\Position.sol

8: import "./Ownable.sol";

[L-06] In the constructor there is no return of incorrect address identification

In case of incorrect address definition in the constructor , there is no way to fix it because of the variables are immutable

It is recommended to fix the architecture:

• Address definitions can be done by changeable architecture

• Because of owner = address(0) at the end of the constructor, there is no way to fix it, so the owner's authority can be maintained.

File: contracts\Equity.sol

constructor(Frankencoin zchf_) ERC20(18) {
    zchf = zchf_;
}

File: contracts\Frankencoin.sol

constructor(uint256 _minApplicationPeriod) ERC20(18){
  MIN_APPLICATION_PERIOD = _minApplicationPeriod;
}

[L-07] Project Upgrade and Stop Scenario should be

At the start of the project, the system may need to be stopped or upgraded, I suggest you have a script beforehand and add it to the documentation. This can also be called an " EMERGENCY STOP (CIRCUIT BREAKER) PATTERN ".

https://github.com/maxwoe/solidity_patterns/blob/master/security/EmergencyStop.sol

[L-08] Add to blacklist function

Description: Cryptocurrency mixing service, Tornado Cash, has been blacklisted in the OFAC. A lot of blockchain companies, token projects, NFT Projects have blacklisted all Ethereum addresses owned by Tornado Cash listed in the US Treasury Department's sanction against the protocol. https://home.treasury.gov/policy-issues/financial-sanctions/recent-actions/20220808 In addition, these platforms even ban accounts that have received ETH on their account with Tornadocash.

Some of these Projects; USDC (https://www.circle.com/en/usdc) Flashbots (https://www.paradigm.xyz/portfolio/flashbots ) Aave (https://aave.com/) Uniswap Balancer Infura Alchemy Opensea dYdX

Details on the subject; https://twitter.com/bantg/status/1556712790894706688?s=20&t=HUTDTeLikUr6Dv9JdMF7AA

The ban on Tornado Cash makes little sense, because in the end, no one can prevent people from using other mixer smart contracts, or forking the existing ones. It neither hinders cybercrime, nor privacy.

Here is the most beautiful and close to the project example; Manifold

Manifold Contract https://etherscan.io/address/0xe4e4003afe3765aca8149a82fc064c0b125b9e5a#code

modifier nonBlacklistRequired(address extension) {
   require(!_blacklistedExtensions.contains(extension), "Extension blacklisted");
   _;
}

Recommended Mitigation Steps: add to Blacklist function and modifier.

[L-09] Function Calls in Loop Could Lead to DoS due to Array length not being checked

Function calls made in unbounded loop are error-prone with potential resource exhaustion as it can trap the contract due to gas limitations or failed transactions. Consider bounding the loop length if the array is expected to be growing and/or handling a huge list of elements to avoid unnecessary gas wastage and denial of service.

File: contracts\Equity.sol

function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {
    require(zchf.equity() < MINIMUM_EQUITY);
    checkQualified(msg.sender, helpers);
    for (uint256 i = 0; i<addressesToWipe.length; i++){
        address current = addressesToWipe[0];
        _burn(current, balanceOf(current));
    }
}

function votes(address sender, address[] calldata helpers) public view returns (uint256) {
    uint256 _votes = votes(sender);
    for (uint i=0; i<helpers.length; i++){
        address current = helpers[i];
        require(current != sender);
        require(canVoteFor(sender, current));
        for (uint j=i+1; j<helpers.length; j++){
            require(current != helpers[j]); // ensure helper unique
        }
        _votes += votes(current);
    }
    return _votes;
}

[L-10] Use uint256 instead uint

Some developers prefer to use uint256 because it is consistent with other uint data types, which also specify their size, and also because making the size of the data explicit reminds the developer and the reader how much data they've got to play with, which may help prevent or detect bugs.

File: contracts\Equity.sol

for (uint i=0; i<helpers.length; i++){
for (uint j=i+1; j<helpers.length; j++){
event Trade(address who, int amount, uint totPrice, uint newprice); // amount pos or neg for mint or redemption

[Q-01] require()/revert() statements without descriptive reason strings

Consider adding descriptive reason strings to require() and revert() statements for better error handling and debugging.

File: .\\contracts\\Equity.sol

194: require(current != sender);
195: require(canVoteFor(sender, current));
197: require(current != helpers[j]); // ensure helper unique
276: require(canRedeem(msg.sender));
310: require(zchf.equity() < MINIMUM_EQUITY);

File: .\\contracts\\ERC20.sol

152: require(recipient != address(0));
180: require(recipient != address(0));

File: .\\contracts\\MintingHub.sol

116: require(zchf.isPosition(position) == address(this), "not our pos");
158: require(challenge.challenger != address(0x0));
171: require(challenge.size >= min);
172: require(copy.size >= min);
254: require(challenge.challenger != address(0x0));

File: .\\contracts\\Position.sol

53: require(initPeriod >= 3 days); // must be at least three days, recommended to use higher values

File: .\\contracts\\StablecoinBridge.sol

50: require(block.timestamp <= horizon, "expired");
51: require(chf.balanceOf(address(this)) <= limit, "limit");

[Q-02] Event is missing indexed fields

Index event fields to make them more quickly accessible to off-chain tools that parse events. However, be aware that each indexed field costs extra gas during emission.

File: .\\contracts\\Equity.sol

91: event Trade(address who, int amount, uint totPrice, uint newprice); // amount pos or neg for mint or redemption

File: .\\contracts\\Frankencoin.sol

52: event MinterApplied(address indexed minter, uint256 applicationPeriod, uint256 applicationFee, string message);
53: event MinterDenied(address indexed minter, string message);

File: .\\contracts\\IERC20.sol

86: event Transfer(address indexed from, address indexed to, uint256 value);
92: event Approval(address indexed owner, address indexed spender, uint256 value);

File: .\\contracts\\MintingHub.sol

48: event ChallengeStarted(address indexed challenger, address indexed position, uint256 size, uint256 number);
49: event ChallengeAverted(address indexed position, uint256 number);
50: event ChallengeSucceeded(address indexed position, uint256 bid, uint256 number);
51: event NewBid(uint256 challengedId, uint256 bidAmount, address bidder);
52: event PostPonedReturn(address collateral, address indexed beneficiary, uint256 amount);

File: .\\contracts\\Position.sol

41: event PositionOpened(address indexed owner, address original, address zchf, address collateral, uint256 price);
42: event MintingUpdate(uint256 collateral, uint256 price, uint256 minted, uint256 limit);
43: event PositionDenied(address indexed sender, string message); // emitted if closed by governance

[Q-03] Use scientific notation(1e18) instead of exponentiation(10**18)

Consider using scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18) for better coding practice and readability.

File: .\\contracts\\Frankencoin.sol

25: uint256 public constant MIN_FEE = 1000 * (10**18);

File: .\\contracts\\MathUtil.sol

10: uint256 internal constant ONE_DEC18 = 10**18;

File: .\\contracts\\MintingHub.sol

20: uint256 public constant OPENING_FEE = 1000 * 10**18;

[Q-04] Non-constant/immutable variables with all capital letters

Variable names that consist of all capital letters should be reserved for constant/immutable variables.

File: .\\contracts\\Equity.sol

61: Frankencoin immutable public zchf;

File: .\\contracts\\Frankencoin.sol

31: IReserve override public immutable reserve;

File: .\\contracts\\MintingHub.sol

30: IFrankencoin public immutable zchf; // currency

File: .\\contracts\\Position.sol

24: uint256 public immutable challengePeriod; // challenge period in seconds
29: uint256 public immutable start; // timestamp when minting can start
30: uint256 public immutable expiration; // timestamp at which the position expires
32: address public immutable original; // originals point to themselves, clone to their origin
33: address public immutable hub; // the hub this position was created by
34: IFrankencoin public immutable zchf; // currency
35: IERC20 public override immutable collateral; // collateral
36: uint256 public override immutable minimumCollateral; // prevent dust amounts
38: uint32 public immutable mintingFeePPM;
39: uint32 public immutable reserveContribution; // in ppm

File: .\\contracts\\StablecoinBridge.sol

13: IERC20 public immutable chf; // the source stablecoin
14: IFrankencoin public immutable zchf; // the Frankencoin
19: uint256 public immutable horizon;
24: uint256 public immutable limit;

[Q-05] NatSpec comments should be in a common format

Context: All NatSpecs are not formatted properly, lack of @notice, @param, @return

Description: 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

Recommendation: NatSpec comments should be in a common format

[Q-06] For modern and more readable code; update import usage

Description: Solidity code is also cleaner in another way that might not be noticeable: the struct Point. We were importing it previously with global import but not using it. The Point struct polluted the source code with an unnecessary object we were not using because we did not need it. This was breaking the rule of modularity and modular programming: only import what you need Specific imports with curly braces allow us to apply this rule better.

Recommendation: import {contract1 , contract2} from "filename.sol";

A good example:

import {Owned} from "solmate/auth/Owned.sol";
import {ERC721} from "solmate/tokens/ERC721.sol";
import {LibString} from "solmate/utils/LibString.sol";
import {MerkleProofLib} from "solmate/utils/MerkleProofLib.sol";
import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol";
import {ERC1155, ERC1155TokenReceiver} from "solmate/tokens/ERC1155.sol";
import {toWadUnsafe, toDaysWadUnsafe} from "solmate/utils/SignedWadMath.sol";

[Q-07] Assembly Codes Specific – Should Have Comments

Since this is a low-level language that is more difficult to parse readers, include extensive documentation, comments on the rationale behind its use, and clearly explaining what each assembly instruction does

This will make it easier for users to trust the code, for reviewers to validate the code, and for developers to build on or update the code.

Note that using Assembly removes several important security features of Solidity, which can make the code more insecure and more error-prone.

File: contracts\PositionFactory.sol

function createClone(address target) internal returns (address result) {
    bytes20 targetBytes = bytes20(target);
    assembly {
        let clone := mload(0x40)
        mstore(clone, 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000000000000000000000)
        mstore(add(clone, 0x14), targetBytes)
        mstore(add(clone, 0x28), 0x5af43d82803e903d91602b57fd5bf30000000000000000000000000000000000)
        result := create(0, clone, 0x37)
    }
}

[Q-08] Take advantage of Custom Error's return value property

An important feature of Custom Error is that values such as address, tokenID, msg.value can be written inside the () sign, this kind of approach provides a serious advantage in debugging and examining the revert details of dapps such as tenderly.

For Example:

File: contracts\Position.sol

if(_coll < minimumCollateral) revert InsufficientCollateral();
if (price > _price) revert InsufficientCollateral();
if (block.timestamp >= start) revert TooLate();
if (collateralReserve * atPrice < minted * ONE_DEC18) revert InsufficientCollateral();
if (size < minimumCollateral && size < collateralBalance()) revert ChallengeTooSmall();
if (block.timestamp > expiration) revert Expired();
if (block.timestamp <= cooldown) revert Hot();
if (challengedAmount > 0) revert Challenged();
if (msg.sender != address(hub)) revert NotHub();

[Q-09] Function Naming suggestions

Proper use of _ as a function name prefix and a common pattern is to prefix internal and private function names with _. This pattern is correctly applied in the Party contracts, however, there are some inconsistencies in the libraries.

File: .\\contracts\\Equity.sol

144: function adjustTotalVotes(address from, uint256 amount, uint256 roundingLoss) internal {
157: function adjustRecipientVoteAnchor(address to, uint256 amount) internal returns (uint256){
172: function anchorTime() internal view returns (uint64){
225: function canVoteFor(address delegate, address owner) internal view returns (bool) {
266: function calculateSharesInternal(uint256 capitalBefore, uint256 investment) internal view returns (uint256) {

File: .\\contracts\\ERC20.sol

97: function allowanceInternal(address owner, address spender) internal view virtual returns (uint256) {

File: .\\contracts\\Frankencoin.sol

102: function allowanceInternal(address owner, address spender) internal view override returns (uint256) {

File: .\\contracts\\MintingHub.sol

188: function minBid(Challenge storage challenge) internal view returns (uint256) {
287: function returnCollateral(Challenge storage challenge, bool postpone) internal {

File: .\\contracts\\Ownable.sol

39: function setOwner(address newOwner) internal {
45: function requireOwner(address sender) internal view {

File: .\\contracts\\Position.sol

169: function collateralBalance() internal view returns (uint256){
193: function mintInternal(address target, uint256 amount, uint256 collateral_) internal {
202: function restrictMinting(uint256 period) internal {
232: function repayInternal(uint256 burnable) internal {
240: function notifyRepaidInternal(uint256 amount) internal {
268: function internalWithdrawCollateral(address target, uint256 amount) internal returns (uint256) {
282: function checkCollateral(uint256 collateralReserve, uint256 atPrice) internal view {
286: function emitUpdate() internal {

File: .\\contracts\\PositionFactory.sol

37: function createClone(address target) internal returns (address result) {

File: .\\contracts\\StablecoinBridge.sol

49: function mintInternal(address target, uint256 amount) internal {
67: function burnInternal(address zchfHolder, address target, uint256 amount) internal {

[Q-10] Order of functions not following the Style Guide

Order of Functions; ordering helps readers identify which functions they can call and find the constructor and fallback definitions easier. But there are contracts in the project that do not comply with this. Functions should be grouped according to their visibility and ordered:

• constructor
• receive function (if exists)
• fallback function (if exists)
• external
• public
• internal
• private.

File: .\\contracts\\Equity.sol

209: function checkQualified(address sender, address[] calldata helpers) public override view {
220: function delegateVoteTo(address delegate) external {

File: .\\contracts\\MintingHub.sol

281: function returnPostponedCollateral(address collateral, address target) external {

File: .\\contracts\\Position.sol

227: function repay(uint256 amount) public onlyOwner {
249: function withdraw(address token, address target, uint256 amount) external onlyOwner {
292: function notifyChallengeStarted(uint256 size) external onlyHub {

File: .\\contracts\\StablecoinBridge.sol

55: function burn(uint256 amount) external {

[Q-11] Long lines are not suitable for the Solidity Style Guide

It is generally recommended that lines in the source code should not exceed 80-120 characters. Multiline output parameters and return statements should follow the same style recommended for wrapping long lines found in the Maximum Line Length section.

File: .\\contracts\\Equity.sol

117: // Recipient votes should stay the same, but grow faster in the future, requiring an adjustment of the anchor.
161: voteAnchor[to] = uint64(anchorTime() - recipientVotes / newbalance); // new example anchor is only 21 / 11 = 1 block in the past
268: uint256 newTotalShares = totalShares < 1000 * ONE_DEC18 ? 1000 * ONE_DEC18 : _mulD18(totalShares, _cubicRoot(_divD18(capitalBefore + investment, capitalBefore)));

File: .\\contracts\\ERC20.sol

53: // Copied from <https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4139/files#diff-fa792f7d08644eebc519dac2c29b00a54afc4c6a76b9ef3bba56c8401fe674f6>

File: .\\contracts\\ERC20PermitLight.sol

40: // keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"),

File: .\\contracts\\Frankencoin.sol

78: * adds value to the Frankencoin system. Complex proposals should have application periods and applications fees above
79: * the minimum. It is assumed that over time, informal ways to coordinate on new minters emerge. The message parameter
80: * might be useful for initiating further communication. Maybe it contains a link to a website describing the proposed
83: function suggestMinter(address _minter, uint256 _applicationPeriod, uint256 _applicationFee, string calldata _message) override external {
115: * The minter reserve can be used to cover losses after all else failed and the equity holders have already been wiped out.
169: minterReserveE6 += _amount * _reservePPM; // minter reserve must be kept accurately in order to ensure we can get back to exactly 0
185: * share holders. This can make sense in combination with 'notifyLoss', i.e. when it is the pool share holders that bear the risk
188: * Design rule: Minters calling this method are only allowed to so for tokens amounts they previously minted with the same _reservePPM amount.
190: * For example, if someone minted 50 ZCHF earlier with a 20% reserve requirement (200000 ppm), they got 40 ZCHF and paid
191: * 10 ZCHF into the reserve. Now they want to repay the debt by burning 50 ZCHF. When doing so using this method, 50 ZCHF get
192: * burned and on top of that, 10 ZCHF previously assigned to the minter's reserved are reassigned to the pool share holders.
201: * Under normal circumstances, this is just the reserver requirement multiplied by the amount. However, after a severe loss
217: * The caller is only allowed to use this method for tokens also minted through the caller with the same _reservePPM amount.
219: * Example: the calling contract has previously minted 100 ZCHF with a reserve ratio of 20% (i.e. 200000 ppm). To burn half
220: * of that again, the minter calls burnFrom with a target amount of 50 ZCHF. Assuming that reserves are only 90% covered,
221: * this call will deduct 41 ZCHF from the payer's balance and 9 from the reserve, while reducing the minter reserve by 10.
223: function burnFrom(address payer, uint256 targetTotalBurnAmount, uint32 _reservePPM) external override minterOnly returns (uint256) {
232: * Calculate the amount that is freed when returning amountExcludingReserve given a reserve ratio of reservePPM, taking
235: function calculateFreedAmount(uint256 amountExcludingReserve /* 41 */, uint32 reservePPM /* 20% */) public view returns (uint256){
238: uint256 adjustedReservePPM = currentReserve < minterReserve_ ? reservePPM * currentReserve / minterReserve_ : reservePPM; // 18%
243: * Burns the provided number of tokens plus whatever reserves are associated with that amount given the reserve requirement.
244: * The caller is only allowed to use this method for tokens also minted through the caller with the same _reservePPM amount.
246: * Example: the calling contract has previously minted 100 ZCHF with a reserve ratio of 20% (i.e. 200000 ppm). Now they have
248: * the call to burnWithReserve will burn the 41 plus 9 from the reserve, reducing the outstanding 'debt' of the caller by
251: function burnWithReserve(uint256 _amountExcludingReserve, uint32 _reservePPM) external override minterOnly returns (uint256) {
254: _transfer(address(reserve), msg.sender, freedAmount - _amountExcludingReserve); // collect assigned reserve, maybe less than original reserve

File: .\\contracts\\IPosition.sol

36: function notifyChallengeSucceeded(address bidder, uint256 bid, uint256 size) external returns (address, uint256, uint256, uint256, uint32);

File: .\\contracts\\MintingHub.sol

121: * Clones an existing position and immediately tries to mint the specified amount using the given amount of collateral.
122: * This requires an allowance to be set on the collateral contract such that the minting hub can withdraw the collateral.
124: function clonePosition(address position, uint256 _initialCollateral, uint256 _initialMint) public validPos(position) returns (address) {
140: function launchChallenge(address _positionAddr, uint256 _collateralAmount) external validPos(_positionAddr) returns (uint256) {
144: challenges.push(Challenge(msg.sender, position, _collateralAmount, block.timestamp + position.challengePeriod(), address(0x0), 0));
247: * In case that the collateral cannot be transfered back to the challenger (i.e. because the collateral token has a blacklist and the
250: * @param postponeCollateralReturn Can be used to postpone the return of the collateral to the challenger. Usually false.
256: // challenge must have been successful, because otherwise it would have immediately ended on placing the winning bid
258: // notify the position that will send the collateral to the bidder. If there is no bid, send the collateral to msg.sender
260: (address owner, uint256 effectiveBid, uint256 volume, uint256 repayment, uint32 reservePPM) = challenge.position.notifyChallengeSucceeded(recipient, challenge.bid, challenge.size);
289: // Postponing helps in case the challenger was blacklisted on the collateral token or otherwise cannot receive it at the moment.
294: challenge.position.collateral().transfer(challenge.challenger, challenge.size); // return the challenger's collateral

File: .\\contracts\\Position.sol

21: uint256 public price; // the zchf price per unit of the collateral below which challenges succeed, (36 - collateral.decimals) decimals
76: function initializeClone(address owner, uint256 _price, uint256 _limit, uint256 _coll, uint256 _mint) external onlyHub {
213: * The repaid amount should fulfill the following equation in order to close the position, i.e. bring the minted amount to 0:
219: * For example, if minted is 50 and reservePPM is 200000, it is necessary to repay 40 to be able to close the position.
221: * Only the owner is allowed to repay a position. This is necessary to prevent a 'limit stealing attack': if a popular position
222: * has reached its limit, an attacker could try to repay the position, clone it, and take a loan himself. This is prevented by
223: * requiring the owner to do the repayment. Other restrictions are not necessary. In particular, it must be possible to repay
224: * the position once it is expired or subject to cooldown. Also, repaying it during a challenge is no problem as the collateral
327: * @return (position owner, effective bid size in ZCHF, effective challenge size in ZCHF, repaid amount, reserve ppm)
329: function notifyChallengeSucceeded(address _bidder, uint256 _bid, uint256 _size) external onlyHub returns (address, uint256, uint256, uint256, uint32) {
347: uint256 volumeZCHF = _mulD18(price, _size); // How much could have minted with the challenged amount of the collateral

File: .\\contracts\\PositionFactory.sol

36: // copied from <https://github.com/optionality/clone-factory/blob/32782f82dfc5a00d103a7e61a17a5dedbd1e8e9d/contracts/CloneFactory.sol>

[Q-12] Floating pragma

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

https://swcregistry.io/docs/SWC-103

All Contracts

[G-01] Pack state variables

Solidity docs: https://docs.soliditylang.org/en/v0.8.17/internals/layout_in_storage.html

State variable packaging in Equity.sol contract (2 * 2k = 4k gas saved)

File: contracts\Equity.sol

  uint32 public constant VALUATION_FACTOR = 3;
- uint256 private constant MINIMUM_EQUITY = 1000 * ONE_DEC18; 
// 1000 * ONE_DEC18 will require almost 70 bits, so we can use uint96
+ uint96 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;
// 90*7200 << BLOCK_TIME_RESOLUTION_BITS = 10,871,635,968,000
// smallest uint type that can accommodate is uint48
+  uint48 public constant MIN_HOLDING_DURATION = 90*7200 << BLOCK_TIME_RESOLUTION_BITS;

Combining variables that fit within one storage slot can save up to 2 slots in storage.

Additionally, you can change BLOCK_TIME_RESOLUTION_BITS to a larger uint type, such as uint32, and for MIN_HOLDING_DURATION, you can use uint64. These modifications will still allow the variables to be packed into a single storage slot.

State variable packaging in Frankencoin.sol contract (1 * 2k = 4k gas saved) or (2 * 2k = 4k gas saved)

File: contracts\Frankencoin.sol

- uint256 public constant MIN_FEE = 1000 * (10**18);
- uint256 public immutable MIN_APPLICATION_PERIOD; // for example 10 days

// 1000 * (10**18) will require almost 70 bits, so we can use uint96
+ uint96 public constant MIN_FEE = 1000 * (10**18);
// uint160 for MIN_APPLICATION_PERIOD is more than enough
+ uint160 public immutable MIN_APPLICATION_PERIOD; // for example 10 days
// The maximum value that can be represented by a uint160 variable, when expressed in years
// is approximately ≈ 46,254,607,991,882,226,990 years

// OR

// additionaly if you are shure that minterReserveE6 can't be more than uint128
- uint256 private minterReserveE6;
// We can reduce MIN_APPLICATION_PERIOD to uint32 since
// maximum value that can be represented by a uint32 variable, when expressed in years, is approximately 136 years
+ uint32 public immutable MIN_APPLICATION_PERIOD; // for example 10 days
+ uint128 private minterReserveE6;

State variable packaging in MathUtil.sol contract (1 * 2k = 4k gas saved)

No need to pack tighter since there are no other storage variables

File: contracts\MathUtil.sol

- uint256 internal constant ONE_DEC18 = 10**18;
- uint256 internal constant THRESH_DEC18 =  10000000000000000;//0.01

+ uint128 internal constant ONE_DEC18 = 10**18;
+ uint128 internal constant THRESH_DEC18 =  10000000000000000;//0.01

State variable packaging in MintingHub.sol contract (1 * 2k = 4k gas saved)

File: contracts\MintingHub.sol

- uint256 public constant OPENING_FEE = 1000 * 10**18;
// 1000 * (10**18) will require almost 70 bits, so we can use uint96
+ uint96 public constant OPENING_FEE = 1000 * 10**18;
  uint32 public constant CHALLENGER_REWARD = 20000; // 2%

State variable packaging in Position.sol contract (4 * 2k = 8k gas saved)

File: contracts\Position.sol

uint256 public price; // the zchf price per unit of the collateral below which challenges succeed, (36 - collateral.decimals) decimals
uint256 public minted; // net minted amount, including reserve
uint256 public challengedAmount; // amount of the collateral that is currently under a challenge
+ uint256 public limit; // the minted amount must never exceed the limit
- uint256 public immutable challengePeriod; // challenge period in seconds

- uint256 public cooldown; // timestamp of the end of the latest cooldown
- uint256 public limit; // the minted amount must never exceed the limit

- uint256 public immutable start; // timestamp when minting can start
- uint256 public immutable expiration; // timestamp at which the position expires

// maximum value that can be represented by a uint32 variable, when expressed in years
// is approximately 136 years
+ uint32 public immutable challengePeriod; // challenge period in seconds

// uint32 timestamp will overflow on February 7, 2106
+ uint32 public cooldown; // timestamp of the end of the latest cooldown
+ uint32 public immutable start; // timestamp when minting can start
+ uint32 public immutable expiration; // timestamp at which the position expires

+ uint32 public immutable mintingFeePPM;
+ uint32 public immutable reserveContribution; // in ppm

// ^^ all uint32 will be packed in 192bits so you can increase some of them to uint64

address public immutable original; // originals point to themselves, clone to their origin
address public immutable hub; // the hub this position was created by
IFrankencoin public immutable zchf; // currency
IERC20 public override immutable collateral; // collateral
uint256 public override immutable minimumCollateral; // prevent dust amounts

- uint32 public immutable mintingFeePPM;
- uint32 public immutable reserveContribution; // in ppm

[G-02] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Combining mappings into a single storage slot can lead to gas savings in certain situations, depending on the types and sizes of the variables involved. Merging mappings may help avoid a 20,000 gas cost (Gsset) for each combined mapping. Additionally, when a function requires both values and they fit into the same storage slot, read and write operations can become more efficient.

Moreover, if both fields are accessed within the same function, it can result in approximately 42 gas savings per access. This is because there's no need to recalculate the key's keccak256 hash (Gkeccak256, which costs 30 gas) and the associated stack operations for each field.

File: contracts\Equity.sol

- mapping (address => address) public delegates;
- mapping (address => uint64) private voteAnchor;

+ struct VoterData {
+    address delegate;
+    uint64 voteAnchor;
+ }
+ mapping (address => VoterData) public voters;

File: contracts\Frankencoin.sol

- mapping (address => uint256) public minters;
- mapping (address => address) public positions;

+ struct MinterData {
+   uint256 approvalTimeStamp; // Approval timestamp for the minter contract to mint Frankencoins
+   address position;          // Position that is allowed to mint and the minter that registered it
+ }

/**
 * Combined mapping for minters and positions.
 */
+ mapping (address => MinterData) public mintersAndPositions;

[G-03] Efficient Struct Packing to Save Storage Slots

Reorganizing the Challenge struct can lead to fewer storage slots being used, which can save gas.

To save approximately 2k gas per slot, consider reorganizing the Challenge struct as follows

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)
+   uint96 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)
}

[G-04] Remove function canRedeem()

Eliminate the canRedeem() function to save gas on deployment without causing issues.

Users can directly call canRedeem(address owner) with their address instead of using the canRedeem() function. This change will reduce gas consumption during deployment while maintaining functionality.

File: contracts\Equity.sol
/**
 * Returns whether the sender address is allowed to redeem FPS.
 */
function canRedeem() external view returns (bool){
    return canRedeem(msg.sender);
}

/**
 * Returns whether the given address is allowed to redeem FPS, which is the
 * case after their average holding duration is larger than the required minimum.
 */
function canRedeem(address owner) public view returns (bool) {
    return anchorTime() - voteAnchor[owner] >= MIN_HOLDING_DURATION;
}

[G-05] Improve For Loop Structure

Adjust the structure of for loops to reduce gas costs. This optimization can save approximately 400 gas for an array with 6 members.

File: contracts\Equity.sol

192: for (uint i=0; i<helpers.length; i++){
196: for (uint j=i+1; j<helpers.length; j++){
312: for (uint256 i = 0; i<addressesToWipe.length; i++){

Recommendation: Use Unchecked Increment and Cache Loop Length Locally

To achieve lower gas consumption in for loops, consider the following changes:

1. Use unchecked increment for the loop variable, as it can exceed the loop condition.
2. Cache the loop length locally to avoid repeatedly accessing the array length.

By implementing these changes, you can optimize for loops to be more gas-efficient while maintaining the desired functionality.

+ uint256 loopLength = addressesToWipe.length;
- for (uint256 i = 0; i<addressesToWipe.length; i++){
+ for (uint256 i = 0; i<loopLength ;){
    address current = addressesToWipe[0];
    _burn(current, balanceOf(current));
+		unchecked {
+      i++;      
+   }
	}

[G-06] Deploy the contract with clone instead of new (x10 Gas Saved)

Utilize OpenZeppelin Clones to save gas during deployment. Cloning contracts with Create2 is more gas-efficient than using new.

Recommendation: Implement OpenZeppelin Clones

Leverage OpenZeppelin Clones to deploy minimal proxy contracts, reducing gas costs significantly.

Comparison: Gas Usage

A Create2 clone is 10 times cheaper in gas usage compared to deploying with new.

Gas usage difference between the two? (answer: a new clone is 10x cheaper)

File: contracts\Frankencoin.sol

constructor(uint256 _minApplicationPeriod) ERC20(18){
  MIN_APPLICATION_PERIOD = _minApplicationPeriod;
  reserve = new Equity(this);
}

[G-07] Use nested if and, avoid multiple check combinations

Using nested is cheaper than using && multiple check combinations. There are more advantages, such as easier-to-read code and better coverage reports.

File: contracts\Frankencoin.sol

if (_applicationPeriod < MIN_APPLICATION_PERIOD && totalSupply() > 0) revert PeriodTooShort();
if (_applicationFee < MIN_FEE  && totalSupply() > 0) revert FeeTooLow();
if (!isMinter(msg.sender) && !isMinter(positions[msg.sender])) revert NotMinter();

File: contracts\Position.sol

if (size < minimumCollateral && size < collateralBalance()) revert ChallengeTooSmall();

[G-08] Use hardcode address instead address(this)

Instead of address(this), it is more gas-efficient to pre-calculate and use the address with a hardcode. Foundry's script.sol and solmate LibRlp.sol contracts can do this.

Reference: https://book.getfoundry.sh/reference/forge-std/compute-create-address

File: contracts\MintingHub.sol

116: require(zchf.isPosition(position) == address(this), "not our pos");
142: IERC20(position.collateral()).transferFrom(msg.sender, address(this), _collateralAmount);
225: zchf.transferFrom(msg.sender, address(this), _bidAmountZCHF);

File: contracts\Position.sol

55: original = address(this);
138: collateral.transferFrom(msg.sender, address(this), newCollateral - colbal);
170: return IERC20(collateral).balanceOf(address(this));
228: IERC20(zchf).transferFrom(msg.sender, address(this), amount);

File: contracts\StablecoinBridge.sol

45: chf.transferFrom(msg.sender, address(this), amount);
51: require(chf.balanceOf(address(this)) <= limit, "limit");
79: burnInternal(address(this), from, amount);

[G-09] Do not calculate constants variables

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.

File: contracts\Equity.sol

uint256 private constant MINIMUM_EQUITY = 1000 * ONE_DEC18;
uint256 public constant MIN_HOLDING_DURATION = 90*7200 << BLOCK_TIME_RESOLUTION_BITS; // Set to 5 for local testing

File: contracts\Frankencoin.sol

uint256 public constant MIN_FEE = 1000 * (10**18);

File: contracts\MathUtil.sol

uint256 internal constant ONE_DEC18 = 10**18;
uint256 internal constant THRESH_DEC18 =  10000000000000000;//0.01

File: contracts\MintingHub.sol

uint256 public constant OPENING_FEE = 1000 * 10**18;

[G-10] Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata.

File: contracts\Frankencoin.sol

function isMinter(address _minter) override public view returns (bool){
+  uint256 minter = minters[_minter];
+  return minter != 0 && block.timestamp >= minter;
-  return minters[_minter] != 0 && block.timestamp >= minters[_minter];
}

[G-11] Change public function visibility to external

Manually searched since the automatic tool has a lot of false positive findings.

In contracts, certain functions are not called internally by other functions within the contract. To optimize gas usage, these functions can be made external.

Identify functions that are only called from outside the contract. Modify their visibility from public to external. This change will reduce gas consumption during external calls as external functions have a lower gas cost compared to public functions.

File: contracts\Position.sol

function adjust(uint256 newMinted, uint256 newCollateral, uint256 newPrice) public onlyOwner {
function repay(uint256 amount) public onlyOwner {
function deny(address[] calldata helpers, string calldata message) public {
function getUsableMint(uint256 totalMint, bool afterFees) public view returns (uint256){
function isClosed() public view returns (bool) {

File: contracts\MintingHub.sol

function clonePosition(address position, uint256 _initialCollateral, uint256 _initialMint) public validPos(position) returns (address) {

File: contracts\Frankencoin.sol

function equity() public view returns (uint256) {

File: contracts\Equity.sol

function calculateShares(uint256 investment) public view returns (uint256) {
function redeem(address target, uint256 shares) public returns (uint256) {
function restructureCapTable(address[] calldata helpers, address[] calldata addressesToWipe) public {

[G-12] Use assembly to write address storage values

File: contracts\Equity.sol

constructor(Frankencoin zchf_) ERC20(18) {
    zchf = zchf_;

File: contracts\Frankencoin.sol

constructor(uint256 _minApplicationPeriod) ERC20(18){
  reserve = new Equity(this);

File: contracts\MintingHub.sol

constructor(address _zchf, address factory) {
    zchf = IFrankencoin(_zchf);
    POSITION_FACTORY = IPositionFactory(factory);

File: 
constructor(...) {
        original = address(this);
        hub = _hub;
        zchf = IFrankencoin(_zchf);
        collateral = IERC20(_collateral);

File: contracts\StablecoinBridge.sol

constructor(address other, address zchfAddress, uint256 limit_){
    chf = IERC20(other);
    zchf = IFrankencoin(zchfAddress);

[G-13] Setting the constructor to payable (13 gas)

You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployments with no security risks.

File: contracts\Equity.sol
constructor(Frankencoin zchf_) ERC20(18) {

File: contracts\Frankencoin.sol
constructor(uint256 _minApplicationPeriod) ERC20(18){

File: contracts\MintingHub.sol
constructor(address _zchf, address factory) {

File: contracts\Position.sol
constructor(address _owner, address _hub, address _zchf, address _collateral,

File: contracts\StablecoinBridge.sol
constructor(address other, address zchfAddress, uint256 limit_){

[G-14] x += y (x -= y) costs more gas than x = x + y (x = x - y) for state variables

File: contracts\Frankencoin.sol

169: minterReserveE6 += _amount * _reservePPM;
196: minterReserveE6 -= amount * reservePPM;
217: minterReserveE6 -= targetTotalBurnAmount * _reservePPM;
253: minterReserveE6 -= freedAmount * _reservePPM;

File: contracts\Position.sol

99: limit -= reduction + _minimum;
196: minted += amount;
242: minted -= amount;
295: challengedAmount += size;
309: challengedAmount -= _collateralAmount;
330: challengedAmount -= _size;

File: contracts\ERC20.sol

184: _totalSupply += amount;
203: _totalSupply -= amount;

[G-15] The solady Library's Ownable contract is significantly gas-optimized, which can be used

The project uses the onlyOwner authorization model I recommend using Solady's highly gas optimized contract.

https://github.com/Vectorized/solady/blob/main/src/auth/OwnableRoles.sol

File: contracts\Position.sol

132: function adjust(uint256 newMinted, uint256 newCollateral, uint256 newPrice) public onlyOwner {
159: function adjustPrice(uint256 newPrice) public onlyOwner noChallenge {
177: function mint(address target, uint256 amount) public onlyOwner noChallenge noCooldown alive {
227: function repay(uint256 amount) public onlyOwner {
249: function withdraw(address token, address target, uint256 amount) external onlyOwner {
263: function withdrawCollateral(address target, uint256 amount) public onlyOwner noChallenge noCooldown {

[G-16] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

File: contracts\MintingHub.sol
function returnCollateral(Challenge storage challenge, bool postpone) internal {

File: contracts\PositionFactory.sol
function createClone(address target) internal returns (address result) {

File: contracts\Equity.sol

function adjustTotalVotes(address from, uint256 amount, uint256 roundingLoss) internal {
function adjustRecipientVoteAnchor(address to, uint256 amount) internal returns (uint256){

File: contracts\Position.sol
function repayInternal(uint256 burnable) internal {

[G-17] Use Solmate SafeTransferLib  contracts

Use the gas-optimized Solmate SafeTransferLib contract for Erc20

https://github.com/transmissions11/solmate/blob/main/src/utils/SafeTransferLib.sol

[G-18] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner 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.

File: contracts\Position.sol

function adjust(uint256 newMinted, uint256 newCollateral, uint256 newPrice) public onlyOwner {
function adjustPrice(uint256 newPrice) public onlyOwner noChallenge {
function mint(address target, uint256 amount) public onlyOwner noChallenge noCooldown alive {
function repay(uint256 amount) public onlyOwner {
function withdraw(address token, address target, uint256 amount) external onlyOwner {
function withdrawCollateral(address target, uint256 amount) public onlyOwner noChallenge noCooldown {

[G-19] Optimize names to save gas (22 gas)

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.

Context:  All Contracts

Recommendation:  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 Erc20Quest.sol contract will be the most used; A lower method ID may be given.

Proof of Consept: https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92

[G-20] Sort Solidity operations using short-circuit mode

Short-circuiting is a solidity contract development model that uses OR/AND logic to sequence different cost operations. It puts low gas cost operations in the front and high gas cost operations in the back, so that if the front is low If the cost operation is feasible, you can skip (short-circuit) the subsequent high-cost Ethereum virtual machine operation.

//f(x) is a low gas cost operation 
//g(y) is a high gas cost operation 

//Sort operations with different gas costs as follows 
f(x) || g(y) 
f(x) && g(y)

File: contracts\Frankencoin.sol

} else if (isMinter(spender) || isMinter(isPosition(spender))){

[G-21] Use a more recent version of solidity

All contracts included in the scope has unspecified pragma solidity ^0.8.0. I recommend that you upgrade the versions of all contracts in scope to the latest version of robustness, '0.8.17’.

Solidity 0.8.10 has a useful change that reduced gas costs of external calls which expect a return value.

In 0.8.15 the conditions necessary for inlining are relaxed. Benchmarks show that the change significantly decreases the bytecode size (which impacts the deployment cost) while the effect on the runtime gas usage is smaller.

In 0.8.17 prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call; Simplify the starting offset of zero-length operations to zero. More efficient overflow checks for multiplication.