Platform: Code4rena
Start Date: 08/06/2022
Pot Size: $115,000 USDC
Total HM: 26
Participants: 72
Period: 11 days
Judge: leastwood
Total Solo HM: 14
Id: 132
League: ETH
Rank: 16/72
Findings: 2
Award: $989.95
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: BowTiedWardens
Also found by: 0x1f8b, 0x29A, 0x52, 0xNazgul, 0xNineDec, 0xf15ers, 0xkatana, 0xmint, Chom, ElKu, Funen, IllIllI, JMukesh, Jujic, Kaiziron, Lambda, MiloTruck, Ruhum, SmartSek, SooYa, TerrierLover, TomJ, WatchPug, Waze, _Adam, asutorufos, auditor0517, bardamu, c3phas, catchup, cccz, ch13fd357r0y3r, cloudjunky, cmichel, cryptphi, csanuragjain, defsec, fatherOfBlocks, hansfriese, hyh, jayjonah8, joestakey, k, kenta, obtarian, oyc_109, robee, sach1r0, shenwilly, simon135, slywaters, sorrynotsorry, tintin, unforgiven, xiaoming90, zzzitron
163.3266 USDC - $163.33
| Issue | Instances | |
|---|---|---|
| 1 | Unused/empty receive()/fallback() function | 1 |
| 2 | safeApprove() is deprecated | 1 |
| 3 | Missing checks for address(0x0) when assigning values to address state variables | 8 |
| 4 | abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() | 1 |
| 5 | Open TODOs | 5 |
| 6 | Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions | 2 |
Total: 18 instances over 6 issues
| Issue | Instances | |
|---|---|---|
| 1 | Missing initializer modifier on constructor | 2 |
| 2 | Unused file | 1 |
| 3 | The nonReentrant modifier should occur before all other modifiers | 2 |
| 4 | Contract implements interface without extending the interface | 3 |
| 5 | public functions not called by the contract should be declared external instead | 45 |
| 6 | Non-assembly method available | 1 |
| 7 | constants should be defined rather than using magic numbers | 30 |
| 8 | Expressions for constant values such as a call to keccak256(), should use immutable rather than constant | 1 |
| 9 | Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18) | 4 |
| 10 | Variable names that consist of all capital letters should be reserved for constant/immutable variables | 6 |
| 11 | Non-library/interface files should use fixed compiler versions, not floating ones | 1 |
| 12 | File is missing NatSpec | 2 |
| 13 | NatSpec is incomplete | 36 |
| 14 | Event is missing indexed fields | 69 |
| 15 | Not using the named return variables anywhere in the function is confusing | 2 |
Total: 205 instances over 15 issues
receive()/fallback() functionIf the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))
There is 1 instance of this issue:
File: contracts/contracts/core/promise/PromiseRouter.sol #1 132: receive() external payable {}
safeApprove() is deprecatedDeprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance(). If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance() can be used instead
There is 1 instance of this issue:
File: contracts/contracts/core/connext/libraries/AssetLogic.sol #1 347: SafeERC20.safeApprove(IERC20(_assetIn), address(pool), _amountIn);
address(0x0) when assigning values to address state variablesThere are 8 instances of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 77: wrapped = _wrapped; 165: v1PriceOracle = _v1PriceOracle; 170: admin = newAdmin;
File: contracts/contracts/core/connext/helpers/Executor.sol 48: connext = _connext;
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol 315: _owner = newOwner; 322: _proposed = newlyProposed;
File: contracts/contracts/core/shared/ProposedOwnable.sol 163: _owner = newOwner; 171: _proposed = newlyProposed;
abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.
If all arguments are strings and or bytes, bytes.concat() should be used instead
There is 1 instance of this issue:
File: contracts/contracts/core/connext/libraries/ConnextMessage.sol #1 178: return keccak256(abi.encodePacked(bytes(_name).length, _name, bytes(_symbol).length, _symbol, _decimals));
Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment
There are 5 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 492: // TODO: do we want to store a mapping of custodied token balances here? 579: // TODO: do we need to keep this 594: // FIXME: routers can repay any-amount out-of-band using the `repayAavePortal` method 1027: // TODO: Should we call approve(0) and approve(totalRepayAmount) instead? or with a try catch to not affect gas on all cases?
File: contracts/contracts/core/connext/libraries/LibConnextStorage.sol 303: // BridgeFacet (cont.) TODO: can we move this
__gap[50] storage variable to allow for new storage variables in later versionsSee this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
There are 2 instances of this issue:
File: contracts/contracts/core/connext/helpers/LPToken.sol #1 13: contract LPToken is ERC20BurnableUpgradeable, OwnableUpgradeable {
File: contracts/contracts/core/connext/helpers/StableSwap.sol #2 31: contract StableSwap is IStableSwap, OwnerPausableUpgradeable, ReentrancyGuardUpgradeable {
initializer modifier on constructorOpenZeppelin recommends that the initializer modifier be applied to constructors
There are 2 instances of this issue:
File: contracts/contracts/core/connext/helpers/StableSwap.sol #1 31: contract StableSwap is IStableSwap, OwnerPausableUpgradeable, ReentrancyGuardUpgradeable {
File: contracts/contracts/core/promise/PromiseRouter.sol #2 23: contract PromiseRouter is Version, Router, ReentrancyGuardUpgradeable {
The file is never imported by any other file
There is 1 instance of this issue:
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #1 0: // SPDX-License-Identifier: UNLICENSED
nonReentrant modifier should occur before all other modifiersThis is a best-practice to protect against reentrancy in other modifiers
There are 2 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol #1 279: function xcall(XCallArgs calldata _args) external payable whenNotPaused nonReentrant returns (bytes32) {
File: contracts/contracts/core/connext/facets/BridgeFacet.sol #2 411: function execute(ExecuteArgs calldata _args) external whenNotPaused nonReentrant returns (bytes32) {
Not extending the interface may lead to the wrong function signature being used, leading to unexpected behavior. If the interface is in fact being implemented, use the override keyword to indicate that fact
There are 3 instances of this issue:
File: contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol #1 /// @audit owner(), renounceOwnership() 28: contract ProposedOwnableFacet is BaseConnextFacet, IProposedOwnable {
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #2 /// @audit owner(), renounceOwnership() 26: abstract contract ProposedOwnableUpgradeable is Initializable {
File: contracts/contracts/core/shared/ProposedOwnable.sol #3 /// @audit owner(), renounceOwnership() 28: abstract contract ProposedOwnable is IProposedOwnable {
public functions not called by the contract should be declared external insteadContracts are allowed to override their parents' functions and change the visibility from external to public.
There are 45 instances of this issue:
File: contracts/contracts/core/connext/facets/VersionFacet.sol 20: function VERSION() public pure returns (uint8) {
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 199: function relayerFees(bytes32 _transferId) public view returns (uint256) { 203: function routedTransfers(bytes32 _transferId) public view returns (address[] memory) { 207: function reconciledTransfers(bytes32 _transferId) public view returns (bool) { 211: function domain() public view returns (uint256) { 215: function executor() public view returns (IExecutor) { 219: function nonce() public view returns (uint256) { 223: function sponsorVault() public view returns (ISponsorVault) {
File: contracts/contracts/core/connext/facets/AssetFacet.sol 66: function canonicalToAdopted(bytes32 _canonicalId) public view returns (address) { 70: function adoptedToCanonical(address _adopted) public view returns (ConnextMessage.TokenId memory) { 78: function approvedAssets(bytes32 _asset) public view returns (bool) { 82: function adoptedToLocalPools(bytes32 _adopted) public view returns (IStableSwap) { 86: function wrapper() public view returns (IWrapped) { 90: function tokenRegistry() public view returns (ITokenRegistry) {
File: contracts/contracts/core/connext/facets/RelayerFacet.sol 70: function transferRelayer(bytes32 _transferId) public view returns (address) { 74: function approvedRelayers(address _relayer) public view returns (bool) {
File: contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol 76: function routerOwnershipRenounced() public view returns (bool) { 83: function assetOwnershipRenounced() public view returns (bool) { 90: function proposed() public view returns (address) { 97: function proposedTimestamp() public view returns (uint256) { 104: function routerOwnershipTimestamp() public view returns (uint256) { 111: function assetOwnershipTimestamp() public view returns (uint256) { 118: function delay() public view returns (uint256) { 128: function proposeRouterOwnershipRenunciation() public onlyOwner { 142: function renounceRouterOwnership() public onlyOwner { 162: function proposeAssetOwnershipRenunciation() public onlyOwner { 175: function renounceAssetOwnership() public onlyOwner { 195: function renounced() public view returns (bool) { 203: function proposeNewOwner(address newlyProposed) public onlyOwner { 217: function renounceOwnership() public onlyOwner { 236: function acceptProposedOwner() public onlyProposed { 253: function pause() public onlyOwner { 258: function unpause() public onlyOwner {
File: contracts/contracts/core/connext/facets/NomadFacet.sol 11: function xAppConnectionManager() public view returns (XAppConnectionManager) { 15: function remotes(uint32 _domain) public view returns (bytes32) {
File: contracts/contracts/core/connext/facets/RoutersFacet.sol 181: function LIQUIDITY_FEE_NUMERATOR() public view returns (uint256) { 185: function LIQUIDITY_FEE_DENOMINATOR() public view returns (uint256) { 222: function getProposedRouterOwner(address _router) public view returns (address) { 231: function getProposedRouterOwnerTimestamp(address _router) public view returns (uint256) { 235: function maxRoutersPerTransfer() public view returns (uint256) { 239: function routerBalances(address _router, address _asset) public view returns (uint256) { 247: function getRouterApprovalForPortal(address _router) public view returns (bool) {
File: contracts/contracts/core/promise/PromiseRouter.sol 146: function initialize(address _xAppConnectionManager) public initializer { 230: function process(bytes32 transferId, bytes calldata _message) public nonReentrant {
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 80: function initialize(address _xAppConnectionManager) public initializer {
assembly{ id := chainid() } => uint256 id = block.chainid, assembly { size := extcodesize() } => uint256 size = address().code.length
There is 1 instance of this issue:
File: contracts/contracts/core/connext/libraries/LibDiamond.sol #1 245: contractSize := extcodesize(_contract)
constants should be defined rather than using magic numbersEven assembly can benefit from using readable constants instead of hex/numeric literals
There are 30 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol /// @audit 32 408: if (_pooledTokens.length <= 1 || _pooledTokens.length > 32)
File: contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol /// @audit 9995 76: s.LIQUIDITY_FEE_NUMERATOR = 9995; /// @audit 10000 77: s.LIQUIDITY_FEE_DENOMINATOR = 10000; /// @audit 5 78: s.maxRoutersPerTransfer = 5;
File: contracts/contracts/core/connext/facets/RoutersFacet.sol /// @audit 95 349: if (_numerator < (denominator * 95) / 100) revert RoutersFacet__setLiquidityFeeNumerator_tooSmall(); /// @audit 100 349: if (_numerator < (denominator * 95) / 100) revert RoutersFacet__setLiquidityFeeNumerator_tooSmall();
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol /// @audit 18 103: uint256 tokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.token).decimals()); /// @audit 18 106: uint256 baseTokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.baseToken).decimals()); /// @audit 18 134: uint256 price = retVal.mul(10**(18 - uint256(aggregator.decimals())));
File: contracts/contracts/core/connext/helpers/StableSwap.sol /// @audit 32 76: require(_pooledTokens.length <= 32, "_pooledTokens.length > 32");
File: contracts/contracts/core/connext/libraries/SwapUtils.sol /// @audit 4 626: return swapFee.mul(numTokens).div(numTokens.sub(1).mul(4));
File: contracts/contracts/core/connext/libraries/ConnextMessage.sol /// @audit 4 232: return uint32(_tokenId.indexUint(0, 4)); /// @audit 4 242: return _tokenId.index(4, 32); /// @audit 32 242: return _tokenId.index(4, 32); /// @audit 16 252: return _tokenId.indexAddress(16); /// @audit 32 282: return _transferAction.index(1, 32); /// @audit 13 292: return _transferAction.indexAddress(13); /// @audit 33 302: return _transferAction.indexUint(33, 32); /// @audit 32 302: return _transferAction.indexUint(33, 32); /// @audit 97 312: return _transferAction.index(97, 32); /// @audit 32 312: return _transferAction.index(97, 32); /// @audit 65 322: return _transferAction.index(65, 32); /// @audit 32 322: return _transferAction.index(65, 32);
File: contracts/contracts/core/connext/libraries/LibCrossDomainProperty.sol /// @audit 5 120: return _property.indexAddress(5); /// @audit 4 130: return uint32(_property.indexUint(1, 4));
File: contracts/contracts/core/promise/PromiseRouter.sol /// @audit 32 313: return (uint64(_origin) << 32) | _nonce;
File: contracts/contracts/core/promise/libraries/PromiseMessage.sol /// @audit 32 81: return _view.index(1, 32); /// @audit 33 91: return _view.indexAddress(33); /// @audit 53 101: return _view.indexUint(53, 1) == 1;
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol /// @audit 32 166: return (uint64(_origin) << 32) | _nonce;
keccak256(), should use immutable rather than constantThere is 1 instance of this issue:
File: contracts/contracts/core/connext/libraries/LibDiamond.sol #1 14: bytes32 constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");
1e18) rather than exponentiation (e.g. 10**18)There are 4 instances of this issue:
File: contracts/contracts/core/connext/libraries/AmplificationUtils.sol #1 22: uint256 public constant MAX_A = 10**6;
File: contracts/contracts/core/connext/libraries/SwapUtils.sol #2 104: uint256 internal constant FEE_DENOMINATOR = 10**10;
File: contracts/contracts/core/connext/libraries/SwapUtils.sol #3 107: uint256 internal constant MAX_SWAP_FEE = 10**8;
File: contracts/contracts/core/connext/libraries/SwapUtils.sol #4 113: uint256 internal constant MAX_ADMIN_FEE = 10**10;
constant/immutable variablesIf the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).
There are 6 instances of this issue:
File: contracts/contracts/core/connext/helpers/Executor.sol 38: uint256 public FAILURE_GAS = 100_000; // Allowance decrease + transfer 43: uint16 public MAX_COPY = 256;
File: contracts/contracts/core/connext/libraries/LibConnextStorage.sol 115: uint256 LIQUIDITY_FEE_NUMERATOR; 117: uint256 LIQUIDITY_FEE_DENOMINATOR;
File: contracts/contracts/core/promise/PromiseRouter.sol 64: uint256[49] private __GAP;
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 38: uint256[49] private __GAP;
There is 1 instance of this issue:
File: contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol #1 2: pragma solidity ^0.8.0;
There are 2 instances of this issue:
File: contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol (various lines) #1
File: contracts/contracts/core/connext/libraries/LibDiamond.sol (various lines) #2
There are 36 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol /// @audit Missing: '@return' 235 * @param deadline latest timestamp to accept this transaction 236 */ 237 function swap( 238 bytes32 canonicalId, 239 uint8 tokenIndexFrom, 240 uint8 tokenIndexTo, 241 uint256 dx, 242 uint256 minDy, 243 uint256 deadline 244: ) external nonReentrant deadlineCheck(deadline) whenNotPaused returns (uint256) { /// @audit Missing: '@param minAmountOut' 248 /** 249 * @notice Swap two tokens using this pool 250 * @param canonicalId the canonical token id 251 * @param assetIn the token the user wants to swap from 252 * @param assetOut the token the user wants to swap to 253 * @param amountIn the amount of tokens the user wants to swap from 254 */ 255 function swapExact( 256 bytes32 canonicalId, 257 uint256 amountIn, 258 address assetIn, 259 address assetOut, 260 uint256 minAmountOut, 261 uint256 deadline 262: ) external payable nonReentrant deadlineCheck(deadline) whenNotPaused returns (uint256) { /// @audit Missing: '@param deadline' 248 /** 249 * @notice Swap two tokens using this pool 250 * @param canonicalId the canonical token id 251 * @param assetIn the token the user wants to swap from 252 * @param assetOut the token the user wants to swap to 253 * @param amountIn the amount of tokens the user wants to swap from 254 */ 255 function swapExact( 256 bytes32 canonicalId, 257 uint256 amountIn, 258 address assetIn, 259 address assetOut, 260 uint256 minAmountOut, 261 uint256 deadline 262: ) external payable nonReentrant deadlineCheck(deadline) whenNotPaused returns (uint256) { /// @audit Missing: '@return' 248 /** 249 * @notice Swap two tokens using this pool 250 * @param canonicalId the canonical token id 251 * @param assetIn the token the user wants to swap from 252 * @param assetOut the token the user wants to swap to 253 * @param amountIn the amount of tokens the user wants to swap from 254 */ 255 function swapExact( 256 bytes32 canonicalId, 257 uint256 amountIn, 258 address assetIn, 259 address assetOut, 260 uint256 minAmountOut, 261 uint256 deadline 262: ) external payable nonReentrant deadlineCheck(deadline) whenNotPaused returns (uint256) { /// @audit Missing: '@param maxAmountIn' 272 /** 273 * @notice Swap two tokens using this pool 274 * @param canonicalId the canonical token id 275 * @param assetIn the token the user wants to swap from 276 * @param assetOut the token the user wants to swap to 277 * @param amountOut the amount of tokens the user wants to swap to 278 */ 279 function swapExactOut( 280 bytes32 canonicalId, 281 uint256 amountOut, 282 address assetIn, 283 address assetOut, 284 uint256 maxAmountIn, 285 uint256 deadline 286: ) external payable nonReentrant deadlineCheck(deadline) returns (uint256) { /// @audit Missing: '@param deadline' 272 /** 273 * @notice Swap two tokens using this pool 274 * @param canonicalId the canonical token id 275 * @param assetIn the token the user wants to swap from 276 * @param assetOut the token the user wants to swap to 277 * @param amountOut the amount of tokens the user wants to swap to 278 */ 279 function swapExactOut( 280 bytes32 canonicalId, 281 uint256 amountOut, 282 address assetIn, 283 address assetOut, 284 uint256 maxAmountIn, 285 uint256 deadline 286: ) external payable nonReentrant deadlineCheck(deadline) returns (uint256) { /// @audit Missing: '@return' 272 /** 273 * @notice Swap two tokens using this pool 274 * @param canonicalId the canonical token id 275 * @param assetIn the token the user wants to swap from 276 * @param assetOut the token the user wants to swap to 277 * @param amountOut the amount of tokens the user wants to swap to 278 */ 279 function swapExactOut( 280 bytes32 canonicalId, 281 uint256 amountOut, 282 address assetIn, 283 address assetOut, 284 uint256 maxAmountIn, 285 uint256 deadline 286: ) external payable nonReentrant deadlineCheck(deadline) returns (uint256) {
File: contracts/contracts/core/connext/facets/BridgeFacet.sol /// @audit Missing: '@return' 625 * @param _sig The signature you are recovering the signer from 626 */ 627: function _recoverSignature(bytes32 _signed, bytes calldata _sig) internal pure returns (address) { /// @audit Missing: '@return' 743 * @param _liquidityFeeDen Liquidity fee denominator 744 */ 745 function _getFastTransferAmount( 746 uint256 _amount, 747 uint256 _liquidityFeeNum, 748 uint256 _liquidityFeeDen 749: ) private pure returns (uint256) { /// @audit Missing: '@param _message' 910 /** 911 * @notice Parses the message and process the transfer 912 * @dev Will mint the tokens if the token originates on a remote origin 913 * @return The message amount 914 * @return The message token 915 * @return The message transfer id 916 */ 917 function _reconcileProcessMessage(bytes memory _message) 918 internal 919 returns ( 920 uint256, 921 address, 922: bytes32
File: contracts/contracts/core/connext/facets/AssetFacet.sol /// @audit Missing: '@param _stableSwapPool' 121 /** 122 * @notice Used to add supported assets. This is an admin only function 123 * @dev When whitelisting the canonical asset, all representational assets would be 124 * whitelisted as well. In the event you have a different adopted asset (i.e. PoS USDC 125 * on polygon), you should *not* whitelist the adopted asset. The stable swap pool 126 * address used should allow you to swap between the local <> adopted asset 127 * @param _canonical - The canonical asset to add by id and domain. All representations 128 * will be whitelisted as well 129 * @param _adoptedAssetId - The used asset id for this domain (i.e. PoS USDC for 130 * polygon) 131 */ 132 function setupAsset( 133 ConnextMessage.TokenId calldata _canonical, 134 address _adoptedAssetId, 135 address _stableSwapPool 136: ) external onlyOwner {
File: contracts/contracts/core/connext/facets/PortalFacet.sol /// @audit Missing: '@param _transferId' 71 /** 72 * @notice Used by routers to perform a manual repayment to Aave Portals to cover any outstanding debt 73 * @dev The router must be approved for portal and with enough liquidity, and must be the caller of this 74 * function 75 * @param _local The local asset (what router stores liquidity in) 76 * @param _backingAmount The principle to be paid (in adopted asset) 77 * @param _feeAmount The fee to be paid (in adopted asset) 78 * @param _maxIn The max value of the local asset to swap for the _backingAmount of adopted asset 79 */ 80 function repayAavePortal( 81 address _local, 82 uint256 _backingAmount, 83 uint256 _feeAmount, 84 uint256 _maxIn, 85: bytes32 _transferId
File: contracts/contracts/core/connext/facets/RoutersFacet.sol /// @audit Missing: '@return' 191 * @param _router The relevant router address 192 */ 193: function getRouterApproval(address _router) public view returns (bool) { /// @audit Missing: '@return' 200 * @param _router The relevant router address 201 */ 202: function getRouterRecipient(address _router) public view returns (address) { /// @audit Missing: '@return' 210 * @param _router The relevant router address 211 */ 212: function getRouterOwner(address _router) public view returns (address) { /// @audit Missing: '@return' 220 * @param _router The relevant router address 221 */ 222: function getProposedRouterOwner(address _router) public view returns (address) { /// @audit Missing: '@return' 229 * @param _router The relevant router address 230 */ 231: function getProposedRouterOwnerTimestamp(address _router) public view returns (uint256) { /// @audit Missing: '@return' 245 * @param _router The relevant router address 246 */ 247: function getRouterApprovalForPortal(address _router) public view returns (bool) {
File: contracts/contracts/core/connext/facets/BaseConnextFacet.sol /// @audit Missing: '@return' 112 * @param _router The address of the potential remote xApp Router 113 */ 114: function _isRemoteRouter(uint32 _domain, bytes32 _router) internal view returns (bool) { /// @audit Missing: '@param _potentialReplica' 136 /** 137 * @notice Determine whether _potentialReplcia is an enrolled Replica from the xAppConnectionManager 138 * @return True if _potentialReplica is an enrolled Replica 139 */ 140: function _isReplica(address _potentialReplica) internal view returns (bool) {
File: contracts/contracts/core/connext/helpers/Executor.sol /// @audit Missing: '@return' 111 * @param _args ExecutorArgs to function. 112 */ 113: function execute(ExecutorArgs memory _args) external payable override onlyConnext returns (bool, bytes memory) {
File: contracts/contracts/core/connext/helpers/LPToken.sol /// @audit Missing: '@return' 19 * @param symbol symbol of this token 20 */ 21: function initialize(string memory name, string memory symbol) external initializer returns (bool) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol /// @audit Missing: '@return' 317 * @param deadline latest timestamp to accept this transaction 318 */ 319 function swap( 320 uint8 tokenIndexFrom, 321 uint8 tokenIndexTo, 322 uint256 dx, 323 uint256 minDy, 324 uint256 deadline 325: ) external override nonReentrant whenNotPaused deadlineCheck(deadline) returns (uint256) { /// @audit Missing: '@return' 334 * @param minAmountOut the min amount of tokens the user wants to swap to 335 */ 336 function swapExact( 337 uint256 amountIn, 338 address assetIn, 339 address assetOut, 340 uint256 minAmountOut 341: ) external payable override nonReentrant whenNotPaused returns (uint256) { /// @audit Missing: '@return' 352 * @param maxAmountIn the max amount of tokens the user wants to swap from 353 */ 354 function swapExactOut( 355 uint256 amountOut, 356 address assetIn, 357 address assetOut, 358 uint256 maxAmountIn 359: ) external payable override nonReentrant whenNotPaused returns (uint256) {
File: contracts/contracts/core/connext/libraries/AssetLogic.sol /// @audit Missing: '@return' 31 * @param canonicalId the canonical token id 32 */ 33: function stableSwapPoolExist(bytes32 canonicalId) internal view returns (bool) { /// @audit Missing: '@param _maxIn' 218 /** 219 * @notice Swaps a local nomad asset for the adopted asset using the stored stable swap 220 * @dev Will not swap if the asset passed in is the adopted asset 221 * @param _asset - The address of the local asset to swap into the adopted asset 222 * @param _amount - The exact amount to receive out of the swap 223 * @return The amount of local asset put into swap 224 * @return The address of asset received post-swap 225 */ 226 function swapFromLocalAssetIfNeededForExactOut( 227 address _asset, 228 uint256 _amount, 229 uint256 _maxIn 230 ) 231 internal 232 returns ( 233 bool, 234 uint256, 235: address /// @audit Missing: '@param _slippageTol' 252 /** 253 * @notice Swaps assetIn t assetOut using the stored stable swap or internal swap pool 254 * @dev Will not swap if the asset passed in is the adopted asset 255 * @param _canonicalId - The canonical token id 256 * @param _assetIn - The address of the from asset 257 * @param _assetOut - The address of the to asset 258 * @param _amount - The amount of the local asset to swap 259 * @return The amount of assetOut 260 * @return The address of assetOut 261 */ 262 function _swapAsset( 263 bytes32 _canonicalId, 264 address _assetIn, 265 address _assetOut, 266 uint256 _amount, 267 uint256 _slippageTol 268: ) internal returns (uint256, address) { /// @audit Missing: '@param _maxIn' 294 /** 295 * @notice Swaps assetIn t assetOut using the stored stable swap or internal swap pool 296 * @dev Will not swap if the asset passed in is the adopted asset 297 * @param _canonicalId - The canonical token id 298 * @param _assetIn - The address of the from asset 299 * @param _assetOut - The address of the to asset 300 * @param _amountOut - The amount of the _assetOut to swap 301 * @return The amount of assetIn 302 * @return The address of assetOut 303 */ 304 function _swapAssetOut( 305 bytes32 _canonicalId, 306 address _assetIn, 307 address _assetOut, 308 uint256 _amountOut, 309 uint256 _maxIn 310 ) 311 internal 312 returns ( 313 bool, 314 uint256, 315: address
File: contracts/contracts/core/connext/libraries/SwapUtils.sol /// @audit Missing: '@param totalSupply' 166 /** 167 * @notice Calculate the dy of withdrawing in one token 168 * @param self Swap struct to read from 169 * @param tokenIndex which token will be withdrawn 170 * @param tokenAmount the amount to withdraw in the pools precision 171 * @return the d and the new y after withdrawing one token 172 */ 173 function calculateWithdrawOneTokenDY( 174 Swap storage self, 175 uint8 tokenIndex, 176 uint256 tokenAmount, 177 uint256 totalSupply 178 ) 179 internal 180 view 181 returns ( 182 uint256, 183 uint256, 184: uint256 /// @audit Missing: '@param balances' 470 /** 471 * @notice Internally calculates a swap between two tokens. 472 * 473 * @dev The caller is expected to transfer the actual amounts (dx and dy) 474 * using the token contracts. 475 * 476 * @param self Swap struct to read from 477 * @param tokenIndexFrom the token to sell 478 * @param tokenIndexTo the token to buy 479 * @param dx the number of tokens to sell. If the token charges a fee on transfers, 480 * use the amount that gets transferred after the fee. 481 * @return dy the number of tokens the user will get 482 * @return dyFee the associated fee 483 */ 484 function _calculateSwap( 485 Swap storage self, 486 uint8 tokenIndexFrom, 487 uint8 tokenIndexTo, 488 uint256 dx, 489 uint256[] memory balances 490: ) internal view returns (uint256 dy, uint256 dyFee) { /// @audit Missing: '@param balances' 501 /** 502 * @notice Internally calculates a swap between two tokens. 503 * 504 * @dev The caller is expected to transfer the actual amounts (dx and dy) 505 * using the token contracts. 506 * 507 * @param self Swap struct to read from 508 * @param tokenIndexFrom the token to sell 509 * @param tokenIndexTo the token to buy 510 * @param dy the number of tokens to buy. If the token charges a fee on transfers, 511 * use the amount that gets transferred after the fee. 512 * @return dx the number of tokens the user have to deposit 513 * @return dxFee the associated fee 514 */ 515 function _calculateSwapInv( 516 Swap storage self, 517 uint8 tokenIndexFrom, 518 uint8 tokenIndexTo, 519 uint256 dy, 520 uint256[] memory balances 521: ) internal view returns (uint256 dx, uint256 dxFee) { /// @audit Missing: '@param self' 536 /** 537 * @notice A simple method to calculate amount of each underlying 538 * tokens that is returned upon burning given amount of 539 * LP tokens 540 * 541 * @param amount the amount of LP tokens that would to be burned on 542 * withdrawal 543 * @return array of amounts of tokens user will receive 544 */ 545: function calculateRemoveLiquidity(Swap storage self, uint256 amount) internal view returns (uint256[] memory) { /// @audit Missing: '@return' 623 * @param numTokens number of tokens pooled 624 */ 625: function _feePerToken(uint256 swapFee, uint256 numTokens) internal pure returns (uint256) {
File: contracts/contracts/core/connext/libraries/LibCrossDomainProperty.sol /// @audit Missing: '@return' 155 * @param _property The bytes representation of the property 156 */ 157: function parseDomainAndSenderBytes(bytes memory _property) internal pure returns (bytes29) {
File: contracts/contracts/core/promise/PromiseRouter.sol /// @audit Missing: '@param _message' 226 /** 227 * @notice Process stored callback function 228 * @param transferId The transferId to process 229 */ 230: function process(bytes32 transferId, bytes calldata _message) public nonReentrant {
indexed fieldsEach event should use three indexed fields if there are three or more fields
There are 69 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 75 event XCalled( 76 bytes32 indexed transferId, 77 XCallArgs xcallArgs, 78 XCalledEventArgs args, 79 uint256 nonce, 80 bytes message, 81 address caller 82: ); 93 event Reconciled( 94 bytes32 indexed transferId, 95 uint32 indexed origin, 96 address[] routers, 97 address asset, 98 uint256 amount, 99 address caller 100: ); 114 event Executed( 115 bytes32 indexed transferId, 116 address indexed to, 117 ExecuteArgs args, 118 address transactingAsset, 119 uint256 transactingAmount, 120 address caller 121: ); 129: event TransferRelayerFeesUpdated(bytes32 indexed transferId, uint256 relayerFee, address caller); 139 event ForcedReceiveLocal( 140 bytes32 indexed transferId, 141 bytes32 indexed canonicalId, 142 uint32 canonicalDomain, 143 uint256 amount 144: ); 153: event AavePortalMintUnbacked(bytes32 indexed transferId, address indexed router, address asset, uint256 amount); 162: event AavePortalRepayment(bytes32 indexed transferId, address asset, uint256 amount, uint256 fee); 171: event AavePortalRepaymentDebt(bytes32 indexed transferId, address asset, uint256 amount, uint256 fee); 179: event SponsorVaultUpdated(address oldSponsorVault, address newSponsorVault, address caller); 187: event PromiseRouterUpdated(address oldRouter, address newRouter, address caller); 195: event ExecutorUpdated(address oldExecutor, address newExecutor, address caller);
File: contracts/contracts/core/connext/facets/AssetFacet.sol 27: event WrapperUpdated(address oldWrapper, address newWrapper, address caller); 35: event TokenRegistryUpdated(address oldTokenRegistry, address newTokenRegistry, address caller); 44: event StableSwapAdded(bytes32 canonicalId, uint32 domain, address swapPool, address caller); 55: event AssetAdded(bytes32 canonicalId, uint32 domain, address adoptedAsset, address supportedAsset, address caller); 62: event AssetRemoved(bytes32 canonicalId, address caller);
File: contracts/contracts/core/connext/facets/PortalFacet.sol 30: event AavePortalRouterRepayment(address indexed router, address asset, uint256 amount, uint256 fee);
File: contracts/contracts/core/connext/facets/RelayerFacet.sol 25: event RelayerFeeRouterUpdated(address oldRouter, address newRouter, address caller); 32: event RelayerAdded(address relayer, address caller); 39: event RelayerRemoved(address relayer, address caller); 48: event InitiatedClaim(uint32 indexed domain, address indexed recipient, address caller, bytes32[] transferIds); 56: event Claimed(address indexed recipient, uint256 total, bytes32[] transferIds);
File: contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol 52: event RouterOwnershipRenunciationProposed(uint256 timestamp); 54: event RouterOwnershipRenounced(bool renounced); 56: event AssetOwnershipRenunciationProposed(uint256 timestamp); 58: event AssetOwnershipRenounced(bool renounced);
File: contracts/contracts/core/connext/facets/RoutersFacet.sol 65: event RouterAdded(address indexed router, address caller); 72: event RouterRemoved(address indexed router, address caller); 103: event MaxRoutersPerTransferUpdated(uint256 maxRoutersPerTransfer, address caller); 110: event LiquidityFeeNumeratorUpdated(uint256 liquidityFeeNumerator, address caller); 117: event RouterApprovedForPortal(address router, address caller); 124: event RouterUnapprovedForPortal(address router, address caller); 133 event RouterLiquidityAdded( 134 address indexed router, 135 address local, 136 bytes32 canonicalId, 137 uint256 amount, 138 address caller 139: ); 149: event RouterLiquidityRemoved(address indexed router, address to, address local, uint256 amount, address caller);
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 65: event NewAdmin(address oldAdmin, address newAdmin); 66: event PriceRecordUpdated(address token, address baseToken, address lpToken, bool _active); 67: event DirectPriceUpdated(address token, uint256 oldPrice, uint256 newPrice); 68: event AggregatorUpdated(address tokenAddress, address source); 69: event V1PriceOracleUpdated(address oldAddress, address newAddress);
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol 62: event RouterOwnershipRenunciationProposed(uint256 timestamp); 64: event RouterOwnershipRenounced(bool renounced); 66: event AssetOwnershipRenunciationProposed(uint256 timestamp); 68: event AssetOwnershipRenounced(bool renounced);
File: contracts/contracts/core/connext/helpers/SponsorVault.sol 73: event ConnextUpdated(address oldConnext, address newConnext, address caller); 78: event RateUpdated(uint32 originDomain, Rate oldRate, Rate newRate, address caller); 83: event RelayerFeeCapUpdated(uint256 oldRelayerFeeCap, uint256 newRelayerFeeCap, address caller); 88: event GasTokenOracleUpdated(address oldOracle, address newOracle, address caller); 93: event TokenExchangeUpdated(address token, address oldTokenExchange, address newTokenExchange, address caller); 98: event ReimburseLiquidityFees(address token, uint256 amount, address receiver); 103: event ReimburseRelayerFees(uint256 amount, address receiver); 108: event Deposit(address token, uint256 amount, address caller); 113: event Withdraw(address token, address receiver, uint256 amount, address caller);
File: contracts/contracts/core/connext/libraries/AmplificationUtils.sol 17: event RampA(uint256 oldA, uint256 newA, uint256 initialTime, uint256 futureTime); 18: event StopRampA(uint256 currentA, uint256 time);
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 25: event TokenSwap(address indexed buyer, uint256 tokensSold, uint256 tokensBought, uint128 soldId, uint128 boughtId); 26 event AddLiquidity( 27 address indexed provider, 28 uint256[] tokenAmounts, 29 uint256[] fees, 30 uint256 invariant, 31 uint256 lpTokenSupply 32: ); 33: event RemoveLiquidity(address indexed provider, uint256[] tokenAmounts, uint256 lpTokenSupply); 34 event RemoveLiquidityOne( 35 address indexed provider, 36 uint256 lpTokenAmount, 37 uint256 lpTokenSupply, 38 uint256 boughtId, 39 uint256 tokensBought 40: ); 41 event RemoveLiquidityImbalance( 42 address indexed provider, 43 uint256[] tokenAmounts, 44 uint256[] fees, 45 uint256 invariant, 46 uint256 lpTokenSupply 47: ); 48: event NewAdminFee(uint256 newAdminFee); 49: event NewSwapFee(uint256 newSwapFee);
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 69: event DiamondCutProposed(IDiamondCut.FacetCut[] _diamondCut, address _init, bytes _calldata, uint256 deadline); 81: event DiamondCutRescinded(IDiamondCut.FacetCut[] _diamondCut, address _init, bytes _calldata); 92: event DiamondCut(IDiamondCut.FacetCut[] _diamondCut, address _init, bytes _calldata);
File: contracts/contracts/core/promise/PromiseRouter.sol 78 event Send( 79 uint32 domain, 80 bytes32 remote, 81 bytes32 transferId, 82 address callbackAddress, 83 bool success, 84 bytes data, 85 bytes message 86: ); 99 event Receive( 100 uint64 indexed originAndNonce, 101 uint32 indexed origin, 102 bytes32 transferId, 103 address callbackAddress, 104 bool success, 105 bytes data, 106 bytes message 107: ); 116: event CallbackFeeAdded(bytes32 indexed transferId, uint256 addedFee, uint256 totalFee, address caller); 123: event CallbackExecuted(bytes32 indexed transferId, address relayer);
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 50: event Send(uint32 domain, address recipient, bytes32[] transferIds, bytes32 remote, bytes message);
Consider changing the variable to be an unnamed one
There are 2 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol #1 /// @audit availableTokenAmount 208 function calculateRemoveSwapLiquidityOneToken( 209 bytes32 canonicalId, 210 uint256 tokenAmount, 211 uint8 tokenIndex 212: ) external view returns (uint256 availableTokenAmount) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol #2 /// @audit availableTokenAmount 291 function calculateRemoveLiquidityOneToken(uint256 tokenAmount, uint8 tokenIndex) 292 external 293 view 294 override 295: returns (uint256 availableTokenAmount)
#0 - jakekidd
2022-07-02T00:46:50Z
really appreciate the formatting here!! props to this auditor
#1 - jakekidd
2022-07-02T00:54:25Z
Low risk: 1 is invalid - common practice here 2+4 are iffy 5 is not low risk, no errors are specified, but it is QA
QA: 1,2,4 are invalid
#2 - IllIllI000
2022-08-19T22:08:48Z
@0xleastwood can you compare and contrast this report with this other one? https://github.com/code-423n4/2022-06-connext-findings/issues/236 It's not clear why the other one is ranked higher even though there are more issues here
#3 - IllIllI000
2022-08-20T12:05:56Z
Low risk: 1 is invalid - common practice here 2+4 are iffy 5 is not low risk, no errors are specified, but it is QA
QA: 1,2,4 are invalid
L-1: common practice does not mean safe. At one point use of payable.transfer was common, and is now considered unsafe. Having an open payable receive() means uses may send funds which are then inaccessible. Loss of funds is not invalid
L-2: It's deprecated because of the risks involved, as is seen by the Medium bugs identified in this contest related to such calls
L-4: There is no compelling reason, and the documentation says there should be one
L-5: If these comments had been addressed, it's possible some of the other issues wardens filed would not have been flagged: Should we call approve(0) and approve(totalRepayAmount) instead? or with a try catch
N-1: Not invalid - see this report https://github.com/code-423n4/2022-06-badger-findings/issues/118#issuecomment-1162499207
N-2: Is the file used somewhere that I missed? If not, I don't see how it's invalid
N-4: Can you elaborate on why you believe these to be invalid? The right tool should be used for the job, not one that happens to work
#4 - 0xleastwood
2022-08-24T02:49:14Z
@0xleastwood can you compare and contrast this report with this other one? #236 It's not clear why the other one is ranked higher even though there are more issues here
While your report may have more findings, most of them are non-critical which are technically not rewarded but I always like to take them into consideration because the majority of them are helpful. #236 contained a severe issue related to the unsafe use of safeApprove which did not satisfy the medium severity level because the impact was not clearly outlined. But nonetheless, the impact is severe and I weighed that report heavily in favour of that single issue.
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, BowTiedWardens, ElKu, Fitraldys, Funen, Kaiziron, Lambda, Metatron, MiloTruck, Randyyy, Ruhum, SmartSek, TomJ, Tomio, UnusualTurtle, Waze, _Adam, apostle0x01, asutorufos, c3phas, catchup, csanuragjain, defsec, fatherOfBlocks, hansfriese, hyh, ignacio, joestakey, k, kaden, nahnah, oyc_109, rfa, robee, sach1r0, simon135, slywaters
826.618 USDC - $826.62
| Issue | Instances | |
|---|---|---|
| 1 | Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate | 1 |
| 2 | State variables only set in the constructor should be declared immutable | 1 |
| 3 | Structs can be packed into fewer storage slots | 2 |
| 4 | Using calldata instead of memory for read-only arguments in external functions saves gas | 10 |
| 5 | Using storage instead of memory for structs/arrays saves gas | 2 |
| 6 | State variables should be cached in stack variables rather than re-reading them from storage | 13 |
| 7 | Multiple accesses of a mapping/array should use a local variable cache | 13 |
| 8 | internal functions only called once can be inlined to save gas | 9 |
| 9 | Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement | 4 |
| 10 | <array>.length should not be looked up in every loop of a for-loop | 19 |
| 11 | ++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 | 26 |
| 12 | require()/revert() strings longer than 32 bytes cost extra gas | 17 |
| 13 | Optimize names to save gas | 3 |
| 14 | Using bools for storage incurs overhead | 3 |
| 15 | Use a more recent version of solidity | 1 |
| 16 | Using > 0 costs more gas than != 0 when used on a uint in a require() statement | 2 |
| 17 | >= costs less gas than > | 2 |
| 18 | It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied | 22 |
| 19 | internal functions not called by the contract should be removed to save deployment gas | 2 |
| 20 | ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) | 26 |
| 21 | Splitting require() statements that use && saves gas | 6 |
| 22 | Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead | 142 |
| 23 | Using private rather than public for constants, saves gas | 6 |
| 24 | Don't use SafeMath once the solidity version is 0.8.0 or greater | 3 |
| 25 | Duplicated require()/revert() checks should be refactored to a modifier or function | 7 |
| 26 | Multiple if-statements with mutually-exclusive conditions should be changed to if-else statements | 1 |
| 27 | require() or revert() statements that check input arguments should be at the top of the function | 6 |
| 28 | Empty blocks should be removed or emit something | 1 |
| 29 | Superfluous event fields | 4 |
| 30 | Use custom errors rather than revert()/require() strings to save gas | 79 |
| 31 | Functions guaranteed to revert when called by normal users can be marked payable | 85 |
Total: 518 instances over 31 issues
address mappings can be combined into a single mapping of an address to a struct, where appropriateSaves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~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.
There is 1 instance of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol #1 62 mapping(address => PriceInfo) public priceRecords; 63: mapping(address => uint256) public assetPrices;
immutableAvoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).
There is 1 instance of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol #1 49: address public wrapped;
Each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the struct. Subsequent reads as well as writes have smaller gas savings
There are 2 instances of this issue:
File: contracts/contracts/core/connext/libraries/LibConnextStorage.sol #1 /// @audit Variable ordering with 8 slots instead of the current 9: /// @audit bytes(32):callData, uint256(32):callbackFee, uint256(32):relayerFee, uint256(32):slippageTol, address(20):to, uint32(4):originDomain, uint32(4):destinationDomain, bool(1):forceSlow, bool(1):receiveLocal, address(20):agent, address(20):recovery, address(20):callback 38 struct CallParams { 39 address to; 40 bytes callData; 41 uint32 originDomain; 42 uint32 destinationDomain; 43 address agent; 44 address recovery; 45 address callback; 46 uint256 callbackFee; 47 uint256 relayerFee; 48 bool forceSlow; 49 bool receiveLocal; 50 uint256 slippageTol; 51: }
File: contracts/contracts/core/connext/libraries/LibConnextStorage.sol #2 /// @audit Move all of the bools next to eachother and the addresses next to eachother in order to save some storage slots 109 struct AppStorage { 110 bool initialized; 111 // 112 // ConnextHandler 113 // 114 // 0 115 uint256 LIQUIDITY_FEE_NUMERATOR; 116 // 1 117 uint256 LIQUIDITY_FEE_DENOMINATOR; 118 // The local nomad relayer fee router 119 // 2 120 RelayerFeeRouter relayerFeeRouter; 121 // The local nomad promise callback router 122 // 3 123 PromiseRouter promiseRouter; 124 // /** 125 // * @notice The address of the wrapper for the native asset on this domain 126 // * @dev Needed because the nomad only handles ERC20 assets 127 // */ 128 // 4 129 IWrapped wrapper; 130 // /** 131 // * @notice Nonce for the contract, used to keep unique transfer ids. 132 // * @dev Assigned at first interaction (xcall on origin domain); 133 // */ 134 // 5 135 uint256 nonce; 136 // /** 137 // * @notice The external contract that will execute crosschain calldata 138 // */ 139 // 6 140 IExecutor executor; 141 // /** 142 // * @notice The domain this contract exists on 143 // * @dev Must match the nomad domain, which is distinct from the "chainId" 144 // */ 145 // 7 146 uint256 domain; 147 // /** 148 // * @notice The local nomad token registry 149 // */ 150 // 8 151 ITokenRegistry tokenRegistry; 152 // /** 153 // * @notice Mapping holding the AMMs for swapping in and out of local assets 154 // * @dev Swaps for an adopted asset <> nomad local asset (i.e. POS USDC <> madUSDC on polygon) 155 // */ 156 // 9 157 mapping(bytes32 => IStableSwap) adoptedToLocalPools; 158 // /** 159 // * @notice Mapping of whitelisted assets on same domain as contract 160 // * @dev Mapping is keyed on the canonical token identifier matching what is stored in the token 161 // * registry 162 // */ 163 // 10 164 mapping(bytes32 => bool) approvedAssets; 165 // /** 166 // * @notice Mapping of canonical to adopted assets on this domain 167 // * @dev If the adopted asset is the native asset, the keyed address will 168 // * be the wrapped asset address 169 // */ 170 // 11 171 mapping(address => ConnextMessage.TokenId) adoptedToCanonical; 172 // /** 173 // * @notice Mapping of adopted to canonical on this domain 174 // * @dev If the adopted asset is the native asset, the stored address will be the 175 // * wrapped asset address 176 // */ 177 // 12 178 mapping(bytes32 => address) canonicalToAdopted; 179 // /** 180 // * @notice Mapping to determine if transfer is reconciled 181 // */ 182 // 13 183 mapping(bytes32 => bool) reconciledTransfers; 184 // /** 185 // * @notice Mapping holding router address that provided fast liquidity 186 // */ 187 // 14 188 mapping(bytes32 => address[]) routedTransfers; 189 // /** 190 // * @notice Mapping of router to available balance of an asset 191 // * @dev Routers should always store liquidity that they can expect to receive via the bridge on 192 // * this domain (the nomad local asset) 193 // */ 194 // 15 195 mapping(address => mapping(address => uint256)) routerBalances; 196 // /** 197 // * @notice Mapping of approved relayers 198 // * @dev Send relayer fee if msg.sender is approvedRelayer. otherwise revert() 199 // */ 200 // 16 201 mapping(address => bool) approvedRelayers; 202 // /** 203 // * @notice Stores the relayer fee for a transfer. Updated on origin domain when a user calls xcall or bump 204 // * @dev This will track all of the relayer fees assigned to a transfer by id, including any bumps made by the relayer 205 // */ 206 // 17 207 mapping(bytes32 => uint256) relayerFees; 208 // /** 209 // * @notice Stores the relayer of a transfer. Updated on the destination domain when a relayer calls execute 210 // * for transfer 211 // * @dev When relayer claims, must check that the msg.sender has forwarded transfer 212 // */ 213 // 18 214 mapping(bytes32 => address) transferRelayer; 215 // /** 216 // * @notice The max amount of routers a payment can be routed through 217 // */ 218 // 19 219 uint256 maxRoutersPerTransfer; 220 // /** 221 // * @notice The Vault used for sponsoring fees 222 // */ 223 // 20 224 ISponsorVault sponsorVault; 225 // 226 // Router 227 // 228 // 21 229 mapping(uint32 => bytes32) remotes; 230 // 231 // XAppConnectionClient 232 // 233 // 22 234 XAppConnectionManager xAppConnectionManager; 235 // 236 // ProposedOwnable 237 // 238 // 23 239 address _proposed; 240 // 24 241 uint256 _proposedOwnershipTimestamp; 242 // 25 243 bool _routerOwnershipRenounced; 244 // 26 245 uint256 _routerOwnershipTimestamp; 246 // 27 247 bool _assetOwnershipRenounced; 248 // 28 249 uint256 _assetOwnershipTimestamp; 250 // 251 // RouterFacet 252 // 253 // 29 254 RouterPermissionsManagerInfo routerPermissionInfo; 255 // 256 // ReentrancyGuard 257 // 258 // 30 259 uint256 _status; 260 // 261 // StableSwap 262 // 263 /** 264 * @notice Mapping holding the AMM storages for swapping in and out of local assets 265 * @dev Swaps for an adopted asset <> nomad local asset (i.e. POS USDC <> madUSDC on polygon) 266 * Struct storing data responsible for automatic market maker functionalities. In order to 267 * access this data, this contract uses SwapUtils library. For more details, see SwapUtils.sol 268 */ 269 // 31 270 mapping(bytes32 => SwapUtils.Swap) swapStorages; 271 /** 272 * @notice Maps token address to an index in the pool. Used to prevent duplicate tokens in the pool. 273 * @dev getTokenIndex function also relies on this mapping to retrieve token index. 274 */ 275 // 32 276 mapping(bytes32 => mapping(address => uint8)) tokenIndexes; 277 /** 278 * @notice Stores whether or not briding, AMMs, have been paused 279 */ 280 // 33 281 bool _paused; 282 // 283 // AavePortals 284 // 285 /** 286 * @notice Address of Aave Pool contract 287 */ 288 address aavePool; 289 /** 290 * @notice Fee percentage numerator for using Portal liquidity 291 * @dev Assumes the same basis points as the liquidity fee 292 */ 293 uint256 aavePortalFeeNumerator; 294 /** 295 * @notice Mapping to store the transfer liquidity amount provided by Aave Portals 296 */ 297 mapping(bytes32 => uint256) portalDebt; 298 /** 299 * @notice Mapping to store the transfer liquidity amount provided by Aave Portals 300 */ 301 mapping(bytes32 => uint256) portalFeeDebt; 302 // 303 // BridgeFacet (cont.) TODO: can we move this 304 // 305 /** 306 * @notice Stores whether a transfer has had `receiveLocal` overrides forced 307 */ 308 // 34 309 mapping(bytes32 => bool) receiveLocalOverrides; 310: }
calldata instead of memory for read-only arguments in external functions saves gasWhen a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.
If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one
There are 10 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 395: IERC20[] memory _pooledTokens, 396: uint8[] memory decimals, 397: string memory lpTokenName, 398: string memory lpTokenSymbol,
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 393: bytes memory _message
File: contracts/contracts/core/connext/helpers/Executor.sol 113: function execute(ExecutorArgs memory _args) external payable override onlyConnext returns (bool, bytes memory) {
File: contracts/contracts/core/connext/helpers/LPToken.sol 21: function initialize(string memory name, string memory symbol) external initializer returns (bool) { 21: function initialize(string memory name, string memory symbol) external initializer returns (bool) {
File: contracts/contracts/core/promise/PromiseRouter.sol 209: bytes memory _message
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 134: bytes memory _message
storage instead of memory for structs/arrays saves gasWhen fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct
There are 2 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol #1 591: address[] memory routers = s.routedTransfers[transferId];
File: contracts/contracts/core/connext/facets/PortalFacet.sol #2 134: ConnextMessage.TokenId memory canonical = s.adoptedToCanonical[adopted];
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There are 13 instances of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol /// @audit v1PriceOracle on line 93 94: tokenPrice = IPriceOracle(v1PriceOracle).getTokenPrice(tokenAddress);
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol /// @audit _proposed on line 262 265: _setOwner(_proposed); /// @audit _proposed on line 274 286: _setOwner(_proposed); /// @audit _proposed on line 322 323: emit OwnershipProposed(_proposed); /// @audit _proposedOwnershipTimestamp on line 255 258: if ((block.timestamp - _proposedOwnershipTimestamp) <= _delay) /// @audit _routerOwnershipTimestamp on line 175 178: if ((block.timestamp - _routerOwnershipTimestamp) <= _delay) /// @audit _routerOwnershipTimestamp on line 292 293: emit RouterOwnershipRenunciationProposed(_routerOwnershipTimestamp); /// @audit _assetOwnershipTimestamp on line 217 220: if ((block.timestamp - _assetOwnershipTimestamp) <= _delay) /// @audit _assetOwnershipTimestamp on line 303 304: emit AssetOwnershipRenunciationProposed(_assetOwnershipTimestamp);
File: contracts/contracts/core/connext/helpers/SponsorVault.sol /// @audit relayerFeeCap on line 256 256: sponsoredFee = sponsoredFee > relayerFeeCap ? relayerFeeCap : sponsoredFee;
File: contracts/contracts/core/shared/ProposedOwnable.sol /// @audit _proposed on line 132 135: _setOwner(_proposed); /// @audit _proposed on line 171 172: emit OwnershipProposed(_proposed); /// @audit _proposedOwnershipTimestamp on line 125 128: if ((block.timestamp - _proposedOwnershipTimestamp) <= _delay)
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
There are 13 instances of this issue:
File: contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol /// @audit facets_[i] on line 33 34: facets_[i].functionSelectors = ds.facetFunctionSelectors[facetAddress_].functionSelectors;
File: contracts/contracts/core/connext/helpers/SponsorVault.sol /// @audit rates[_originDomain] on line 248 249: den = rates[_originDomain].den;
File: contracts/contracts/core/connext/libraries/SwapUtils.sol /// @audit balances[i] on line 593 595: balances[i] = balances[i].sub(amounts[i], "Cannot withdraw more than available"); /// @audit pooledTokens[i] on line 849 850: pooledTokens[i].safeTransferFrom(msg.sender, address(this), amounts[i]); /// @audit pooledTokens[i] on line 850 853: amounts[i] = pooledTokens[i].balanceOf(address(this)).sub(beforeBalance); /// @audit newBalances[i] on line 873 875: newBalances[i] = newBalances[i].sub(fees[i]); /// @audit balances1[i] on line 1024 1026: balances1[i] = balances1[i].sub(fees[i]);
File: contracts/contracts/core/connext/libraries/LibDiamond.sol /// @audit _diamondCut[facetIndex] on line 105 107: addFunctions(_diamondCut[facetIndex].facetAddress, _diamondCut[facetIndex].functionSelectors); /// @audit _diamondCut[facetIndex] on line 107 107: addFunctions(_diamondCut[facetIndex].facetAddress, _diamondCut[facetIndex].functionSelectors); /// @audit _diamondCut[facetIndex] on line 107 109: replaceFunctions(_diamondCut[facetIndex].facetAddress, _diamondCut[facetIndex].functionSelectors); /// @audit _diamondCut[facetIndex] on line 109 109: replaceFunctions(_diamondCut[facetIndex].facetAddress, _diamondCut[facetIndex].functionSelectors); /// @audit _diamondCut[facetIndex] on line 109 111: removeFunctions(_diamondCut[facetIndex].facetAddress, _diamondCut[facetIndex].functionSelectors); /// @audit _diamondCut[facetIndex] on line 111 111: removeFunctions(_diamondCut[facetIndex].facetAddress, _diamondCut[facetIndex].functionSelectors);
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
There are 9 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 481 function _formatMessage( 482 XCallArgs calldata _args, 483 address _asset, 484 bytes32 _transferId, 485 uint256 _amount 486: ) internal returns (bytes memory) { 541: function _reconcile(uint32 _origin, bytes memory _message) internal { 627: function _recoverSignature(bytes32 _signed, bytes calldata _sig) internal pure returns (address) { 885 function _executePortalTransfer( 886 bytes32 _transferId, 887 uint256 _fastTransferAmount, 888 address _local, 889 address _router 890: ) internal returns (uint256, address) { 1075 function _calculatePortalRepayment( 1076 uint256 _localAmount, 1077 bytes32 _transferId, 1078 address _local 1079 ) 1080 internal 1081 returns ( 1082 uint256, 1083 uint256, 1084: uint256
File: contracts/contracts/core/connext/facets/BaseConnextFacet.sol 114: function _isRemoteRouter(uint32 _domain, bytes32 _router) internal view returns (bool) { 140: function _isReplica(address _potentialReplica) internal view returns (bool) {
File: contracts/contracts/core/promise/PromiseRouter.sol 312: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) {
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 165: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) {
unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statementrequire(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }
There are 4 instances of this issue:
File: contracts/contracts/core/connext/helpers/StableSwap.sol #1 /// @audit require() on line 90 91: precisionMultipliers[i] = 10**uint256(SwapUtils.POOL_PRECISION_DECIMALS - decimals[i]);
File: contracts/contracts/core/connext/facets/BridgeFacet.sol #2 /// @audit if-condition on line 1099 1101: portalFee = availableAmount - backUnbackedAmount;
File: contracts/contracts/core/connext/facets/PortalFacet.sol #3 /// @audit if-condition on line 146 147: uint256 missing = total - amount;
File: contracts/contracts/core/connext/libraries/MathUtils.sol #4 /// @audit if-condition on line 29 30: return a - b;
<array>.length should not be looked up in every loop of a for-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD (3 gas)CALLDATALOAD (3 gas)Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset
There are 19 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 415: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/facets/RelayerFacet.sol 140: for (uint256 i; i < _transferIds.length; ) { 164: for (uint256 i; i < _transferIds.length; ) {
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 176: for (uint256 i = 0; i < tokenAddresses.length; i++) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol 81: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 205: for (uint256 i = 0; i < xp.length; i++) { 558: for (uint256 i = 0; i < balances.length; i++) { 591: for (uint256 i = 0; i < balances.length; i++) { 844: for (uint256 i = 0; i < pooledTokens.length; i++) { 869: for (uint256 i = 0; i < pooledTokens.length; i++) { 924: for (uint256 i = 0; i < amounts.length; i++) { 1014: for (uint256 i = 0; i < pooledTokens.length; i++) { 1019: for (uint256 i = 0; i < pooledTokens.length; i++) { 1039: for (uint256 i = 0; i < pooledTokens.length; i++) { 1055: for (uint256 i = 0; i < pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 104: for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) { 129: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) { 147: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) { 162: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
++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-loopsThe 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
There are 26 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 415: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol 31: for (uint256 i; i < numFacets; i++) {
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 176: for (uint256 i = 0; i < tokenAddresses.length; i++) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol 81: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 205: for (uint256 i = 0; i < xp.length; i++) { 254: for (uint256 i = 0; i < numTokens; i++) { 268: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 289: for (uint256 i = 0; i < numTokens; i++) { 300: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 302: for (uint256 j = 0; j < numTokens; j++) { 344: for (uint256 i = 0; i < numTokens; i++) { 405: for (uint256 i = 0; i < numTokens; i++) { 425: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 558: for (uint256 i = 0; i < balances.length; i++) { 591: for (uint256 i = 0; i < balances.length; i++) { 844: for (uint256 i = 0; i < pooledTokens.length; i++) { 869: for (uint256 i = 0; i < pooledTokens.length; i++) { 924: for (uint256 i = 0; i < amounts.length; i++) { 1014: for (uint256 i = 0; i < pooledTokens.length; i++) { 1019: for (uint256 i = 0; i < pooledTokens.length; i++) { 1039: for (uint256 i = 0; i < pooledTokens.length; i++) { 1055: for (uint256 i = 0; i < pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 104: for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) { 129: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) { 147: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) { 162: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
require()/revert() strings longer than 32 bytes cost extra gasEach extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas
There are 17 instances of this issue:
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 697: require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance"); 784: require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance");
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 66: require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner"); 113: revert("LibDiamondCut: Incorrect FacetCutAction"); 121: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 123: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); 132: require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists"); 139: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 141: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); 150: require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function"); 158: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 161: require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)"); 191: require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist"); 193: require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function"); 224: require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty"); 226: require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)"); 236: revert("LibDiamondCut: _init function reverted");
public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted
There are 3 instances of this issue:
File: contracts/contracts/core/connext/helpers/PriceOracle.sol #1 /// @audit getTokenPrice() 4: abstract contract PriceOracle {
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #2 /// @audit proposed(), proposedTimestamp(), routerOwnershipTimestamp(), assetOwnershipTimestamp(), delay(), isRouterOwnershipRenounced(), proposeRouterOwnershipRenunciation(), renounceRouterOwnership(), isAssetOwnershipRenounced(), proposeAssetOwnershipRenunciation(), renounceAssetOwnership(), renounced(), proposeNewOwner(), acceptProposedOwner() 26: abstract contract ProposedOwnableUpgradeable is Initializable {
File: contracts/contracts/core/shared/ProposedOwnable.sol #3 /// @audit proposed(), proposedTimestamp(), delay(), renounced(), proposeNewOwner(), acceptProposedOwner() 28: abstract contract ProposedOwnable is IProposedOwnable {
bools for storage incurs overhead// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past
There are 3 instances of this issue:
File: contracts/contracts/core/connext/helpers/PriceOracle.sol #1 6: bool public constant isPriceOracle = true;
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #2 54: bool private _routerOwnershipRenounced;
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #3 57: bool private _assetOwnershipRenounced;
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
There is 1 instance of this issue:
File: contracts/contracts/core/connext/facets/upgrade-initializers/DiamondInit.sol #1 2: pragma solidity ^0.8.0;
> 0 costs more gas than != 0 when used on a uint in a require() statementThis change saves 6 gas per instance. The optimization works until solidity version 0.8.13 where there is a regression in gas costs.
There are 2 instances of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol #1 150: require(baseTokenPrice > 0, "invalid base token");
File: contracts/contracts/core/connext/libraries/LibDiamond.sol #2 247: require(contractSize > 0, _errorMessage);
>= costs less gas than >The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas
There are 2 instances of this issue:
File: contracts/contracts/core/connext/helpers/SponsorVault.sol #1 256: sponsoredFee = sponsoredFee > relayerFeeCap ? relayerFeeCap : sponsoredFee;
File: contracts/contracts/core/connext/helpers/SponsorVault.sol #2 258: sponsoredFee = sponsoredFee > address(this).balance ? address(this).balance : sponsoredFee;
constant/non-immutable variables to zero than to let the default of zero be appliedNot overwriting the default for stack variables saves 8 gas. Storage and memory variables have larger savings
There are 22 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 415: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 176: for (uint256 i = 0; i < tokenAddresses.length; i++) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol 81: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 205: for (uint256 i = 0; i < xp.length; i++) { 254: for (uint256 i = 0; i < numTokens; i++) { 268: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 289: for (uint256 i = 0; i < numTokens; i++) { 300: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 302: for (uint256 j = 0; j < numTokens; j++) { 344: for (uint256 i = 0; i < numTokens; i++) { 405: for (uint256 i = 0; i < numTokens; i++) { 425: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 558: for (uint256 i = 0; i < balances.length; i++) { 591: for (uint256 i = 0; i < balances.length; i++) { 844: for (uint256 i = 0; i < pooledTokens.length; i++) { 869: for (uint256 i = 0; i < pooledTokens.length; i++) { 924: for (uint256 i = 0; i < amounts.length; i++) { 1014: for (uint256 i = 0; i < pooledTokens.length; i++) { 1019: for (uint256 i = 0; i < pooledTokens.length; i++) { 1039: for (uint256 i = 0; i < pooledTokens.length; i++) { 1055: for (uint256 i = 0; i < pooledTokens.length; i++) {
File: contracts/contracts/core/relayer-fee/libraries/RelayerFeeMessage.sol 81: for (uint256 i = 0; i < length; ) {
internal functions not called by the contract should be removed to save deployment gasIf the functions are required by an interface, the contract should inherit from that interface and use the override keyword
There are 2 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol #1 917 function _reconcileProcessMessage(bytes memory _message) 918 internal 919 returns ( 920 uint256, 921 address, 922: bytes32
File: contracts/contracts/core/connext/facets/BaseConnextFacet.sol #2 132: function _home() internal view returns (Home) {
++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)Saves 6 gas per loop
There are 26 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 415: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/facets/DiamondLoupeFacet.sol 31: for (uint256 i; i < numFacets; i++) {
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 176: for (uint256 i = 0; i < tokenAddresses.length; i++) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol 81: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 205: for (uint256 i = 0; i < xp.length; i++) { 254: for (uint256 i = 0; i < numTokens; i++) { 268: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 289: for (uint256 i = 0; i < numTokens; i++) { 300: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 302: for (uint256 j = 0; j < numTokens; j++) { 344: for (uint256 i = 0; i < numTokens; i++) { 405: for (uint256 i = 0; i < numTokens; i++) { 425: for (uint256 i = 0; i < MAX_LOOP_LIMIT; i++) { 558: for (uint256 i = 0; i < balances.length; i++) { 591: for (uint256 i = 0; i < balances.length; i++) { 844: for (uint256 i = 0; i < pooledTokens.length; i++) { 869: for (uint256 i = 0; i < pooledTokens.length; i++) { 924: for (uint256 i = 0; i < amounts.length; i++) { 1014: for (uint256 i = 0; i < pooledTokens.length; i++) { 1019: for (uint256 i = 0; i < pooledTokens.length; i++) { 1039: for (uint256 i = 0; i < pooledTokens.length; i++) { 1055: for (uint256 i = 0; i < pooledTokens.length; i++) {
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 104: for (uint256 facetIndex; facetIndex < _diamondCut.length; facetIndex++) { 129: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) { 147: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) { 162: for (uint256 selectorIndex; selectorIndex < _functionSelectors.length; selectorIndex++) {
require() statements that use && saves gasSee this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper
There are 6 instances of this issue:
File: contracts/contracts/core/connext/helpers/StableSwap.sol 84 require( 85 tokenIndexes[address(_pooledTokens[i])] == 0 && _pooledTokens[0] != _pooledTokens[i], 86 "Duplicate tokens" 87: );
File: contracts/contracts/core/connext/libraries/AmplificationUtils.sol 86: require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 397: require(tokenIndexFrom < numTokens && tokenIndexTo < numTokens, "Tokens must be in pool"); 493: require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); 524: require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); 1007: require(maxBurnAmount <= v.lpToken.balanceOf(msg.sender) && maxBurnAmount != 0, ">LP.balanceOf");
uints/ints smaller than 32 bytes (256 bits) incurs overheadWhen 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 Use a larger size then downcast where needed
There are 142 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 107: function getSwapToken(bytes32 canonicalId, uint8 index) public view returns (IERC20) { 119: function getSwapTokenIndex(bytes32 canonicalId, address tokenAddress) public view returns (uint8) { 120: uint8 index = s.tokenIndexes[canonicalId][tokenAddress]; 131: function getSwapTokenBalance(bytes32 canonicalId, uint8 index) external view returns (uint256) { 157: uint8 tokenIndexFrom, 158: uint8 tokenIndexTo, 211: uint8 tokenIndex 239: uint8 tokenIndexFrom, 240: uint8 tokenIndexTo, 347: uint8 tokenIndex, 415: for (uint8 i = 0; i < _pooledTokens.length; i++) {
File: contracts/contracts/core/connext/facets/VersionFacet.sol 16: uint8 internal immutable _version = 0; 20: function VERSION() public pure returns (uint8) {
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 68: uint16 public constant AAVE_REFERRAL_CODE = 0; 95: uint32 indexed origin, 142: uint32 canonicalDomain, 327: (uint32 canonicalDomain, bytes32 canonicalId) = s.tokenRegistry.getTokenId(transactingAssetId); 390: uint32 _origin, 391: uint32 _nonce, 459: uint32 _canonicalDomain, 518: (uint32 canonicalDomain, bytes32 canonicalId) = s.tokenRegistry.getTokenId(_asset); 541: function _reconcile(uint32 _origin, bytes memory _message) internal { 720: (uint32 tokenDomain, bytes32 tokenId) = s.tokenRegistry.getTokenId(_args.local); 733: uint32 _canonicalDomain,
File: contracts/contracts/core/connext/facets/AssetFacet.sol 44: event StableSwapAdded(bytes32 canonicalId, uint32 domain, address swapPool, address caller); 55: event AssetAdded(bytes32 canonicalId, uint32 domain, address adoptedAsset, address supportedAsset, address caller);
File: contracts/contracts/core/connext/facets/RelayerFacet.sol 48: event InitiatedClaim(uint32 indexed domain, address indexed recipient, address caller, bytes32[] transferIds); 131: uint32 _domain,
File: contracts/contracts/core/connext/facets/NomadFacet.sol 15: function remotes(uint32 _domain) public view returns (bytes32) { 34: function enrollRemoteRouter(uint32 _domain, bytes32 _router) external onlyOwner {
File: contracts/contracts/core/connext/facets/BaseConnextFacet.sol 55: modifier onlyRemoteRouter(uint32 _origin, bytes32 _router) { 114: function _isRemoteRouter(uint32 _domain, bytes32 _router) internal view returns (bool) { 123: function _mustHaveRemote(uint32 _domain) internal view returns (bytes32 _remote) { 148: function _localDomain() internal view virtual returns (uint32) {
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 12: function decimals() external view returns (uint8); 21: function getRoundData(uint80 _roundId) 25: uint80 roundId, 29: uint80 answeredInRound 36: uint80 roundId, 40: uint80 answeredInRound
File: contracts/contracts/core/connext/helpers/Executor.sol 43: uint16 public MAX_COPY = 256; 88: function origin() external view override returns (uint32) {
File: contracts/contracts/core/connext/helpers/StableSwap.sol 81: for (uint8 i = 0; i < _pooledTokens.length; i++) { 154: function getToken(uint8 index) public view override returns (IERC20) { 165: function getTokenIndex(address tokenAddress) public view override returns (uint8) { 166: uint8 index = tokenIndexes[tokenAddress]; 176: function getTokenBalance(uint8 index) external view override returns (uint256) { 201: uint8 tokenIndexFrom = getTokenIndex(assetIn); 202: uint8 tokenIndexTo = getTokenIndex(assetOut); 215: uint8 tokenIndexFrom, 216: uint8 tokenIndexTo, 234: uint8 tokenIndexFrom = getTokenIndex(assetIn); 235: uint8 tokenIndexTo = getTokenIndex(assetOut); 247: uint8 tokenIndexFrom, 248: uint8 tokenIndexTo, 291: function calculateRemoveLiquidityOneToken(uint256 tokenAmount, uint8 tokenIndex) 320: uint8 tokenIndexFrom, 321: uint8 tokenIndexTo, 342: uint8 tokenIndexFrom = getTokenIndex(assetIn); 343: uint8 tokenIndexTo = getTokenIndex(assetOut); 360: uint8 tokenIndexFrom = getTokenIndex(assetIn); 361: uint8 tokenIndexTo = getTokenIndex(assetOut); 410: uint8 tokenIndex,
File: contracts/contracts/core/connext/helpers/SponsorVault.sol 78: event RateUpdated(uint32 originDomain, Rate oldRate, Rate newRate, address caller); 147: function setRate(uint32 _originDomain, Rate calldata _rate) external onlyOwner { 235: uint32 _originDomain,
File: contracts/contracts/core/connext/libraries/AssetLogic.sol 45: function getTokenIndexFromStableSwapPool(bytes32 canonicalId, address tokenAddress) internal view returns (uint8) { 47: uint8 index = s.tokenIndexes[canonicalId][tokenAddress]; 328: uint8 tokenIndexIn = getTokenIndexFromStableSwapPool(_canonicalId, _assetIn); 329: uint8 tokenIndexOut = getTokenIndexFromStableSwapPool(_canonicalId, _assetOut); 384: uint8 tokenIndexIn = getTokenIndexFromStableSwapPool(id, _asset); 385: uint8 tokenIndexOut = getTokenIndexFromStableSwapPool(id, adopted); 407: (uint32 domain, bytes32 id) = s.tokenRegistry.getTokenId(_asset); 418: uint8 tokenIndexIn = getTokenIndexFromStableSwapPool(id, _asset); 419: uint8 tokenIndexOut = getTokenIndexFromStableSwapPool(id, local);
File: contracts/contracts/core/connext/libraries/LibConnextStorage.sol 41: uint32 originDomain; 42: uint32 destinationDomain;
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 25: event TokenSwap(address indexed buyer, uint256 tokensSold, uint256 tokensBought, uint128 soldId, uint128 boughtId); 25: event TokenSwap(address indexed buyer, uint256 tokensSold, uint256 tokensBought, uint128 soldId, uint128 boughtId); 100: uint8 internal constant POOL_PRECISION_DECIMALS = 18; 135: uint8 tokenIndex 149: uint8 tokenIndex, 175: uint8 tokenIndex, 243: uint8 tokenIndex, 390: uint8 tokenIndexFrom, 391: uint8 tokenIndexTo, 446: uint8 tokenIndexFrom, 447: uint8 tokenIndexTo, 463: uint8 tokenIndexFrom, 464: uint8 tokenIndexTo, 486: uint8 tokenIndexFrom, 487: uint8 tokenIndexTo, 517: uint8 tokenIndexFrom, 518: uint8 tokenIndexTo, 642: uint8 tokenIndexFrom, 643: uint8 tokenIndexTo, 692: uint8 tokenIndexFrom, 693: uint8 tokenIndexTo, 744: uint8 tokenIndexFrom, 745: uint8 tokenIndexTo, 779: uint8 tokenIndexFrom, 780: uint8 tokenIndexTo, 948: uint8 tokenIndex,
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 20: uint96 functionSelectorPosition; // position in facetFunctionSelectors.functionSelectors array 124: uint96 selectorPosition = uint96(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length); 142: uint96 selectorPosition = uint96(ds.facetFunctionSelectors[_facetAddress].functionSelectors.length); 178: uint96 _selectorPosition,
File: contracts/contracts/core/connext/libraries/ConnextMessage.sol 30: uint32 domain; 156: function formatTokenId(uint32 _domain, bytes32 _id) internal pure returns (bytes29) { 176: uint8 _decimals 220: uint40 _type = uint40(msgType(_message)); 231: function domain(bytes29 _tokenId) internal pure typeAssert(_tokenId, Types.TokenId) returns (uint32) { 262: function msgType(bytes29 _message) internal pure returns (uint8) { 271: function actionType(bytes29 _action) internal pure returns (uint8) {
File: contracts/contracts/core/connext/libraries/LibCrossDomainProperty.sol 29: uint32 domain; 88: function propertyType(bytes29 _property) internal pure returns (uint8) { 128: function domain(bytes29 _property) internal pure typeAssert(_property, Types.DomainAndSender) returns (uint32) { 139: function formatDomainAndSender(uint32 _domain, address _sender) internal pure returns (bytes29) { 149: function formatDomainAndSenderBytes(uint32 _domain, address _sender) internal pure returns (bytes memory) {
File: contracts/contracts/core/promise/PromiseRouter.sol 79: uint32 domain, 100: uint64 indexed originAndNonce, 101: uint32 indexed origin, 171: uint32 _domain, 206: uint32 _origin, 207: uint32 _nonce, 300: function _localDomain() internal view override(XAppConnectionClient) returns (uint32) { 312: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) { 312: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) { 312: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) {
File: contracts/contracts/core/promise/libraries/PromiseMessage.sol 28: uint8 private constant LENGTH_RETURNDATA_LEN = 32;
File: contracts/contracts/core/relayer-fee/libraries/RelayerFeeMessage.sol 29: uint8 private constant LENGTH_ID_LEN = 32; 34: uint8 private constant TRANSFER_ID_LEN = 32;
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 50: event Send(uint32 domain, address recipient, bytes32[] transferIds, bytes32 remote, bytes message); 60: event Receive(uint64 indexed originAndNonce, uint32 indexed origin, address indexed recipient, bytes32[] transferIds); 60: event Receive(uint64 indexed originAndNonce, uint32 indexed origin, address indexed recipient, bytes32[] transferIds); 103: uint32 _domain, 131: uint32 _origin, 132: uint32 _nonce, 153: function _localDomain() internal view override(XAppConnectionClient) returns (uint32) { 165: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) { 165: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) { 165: function _originAndNonce(uint32 _origin, uint32 _nonce) internal pure returns (uint64) {
private rather than public for constants, saves gasIf needed, the value can be read from the verified contract source code. 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
There are 6 instances of this issue:
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 68: uint16 public constant AAVE_REFERRAL_CODE = 0;
File: contracts/contracts/core/connext/helpers/PriceOracle.sol 6: bool public constant isPriceOracle = true;
File: contracts/contracts/core/connext/libraries/AmplificationUtils.sol 21: uint256 public constant A_PRECISION = 100; 22: uint256 public constant MAX_A = 10**6;
File: contracts/contracts/core/connext/libraries/LibCrossDomainProperty.sol 37: bytes29 public constant EMPTY = hex"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; 38: bytes public constant EMPTY_BYTES = hex"ffffffffffffffffffffffffffffffffffffffffffffffffffffffffff";
SafeMath once the solidity version is 0.8.0 or greaterVersion 0.8.0 introduces internal overflow checks, so using SafeMath is redundant and adds overhead
There are 3 instances of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol #1 4: import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
File: contracts/contracts/core/connext/libraries/AmplificationUtils.sol #2 5: import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
File: contracts/contracts/core/connext/libraries/SwapUtils.sol #3 4: import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
require()/revert() checks should be refactored to a modifier or functionSaves deployment costs
There are 7 instances of this issue:
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 432: revert("Approximation did not converge"); 524: require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); 717: require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); 756: require(dy >= minDy, "Swap didn't result in min tokens"); 784: require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance");
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 139: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 141: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)");
if-statements with mutually-exclusive conditions should be changed to if-else statementsIf two conditions are the same, their blocks should be combined
There is 1 instance of this issue:
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol #1 87 if (tokenPrice == 0) { 88 tokenPrice = getPriceFromOracle(tokenAddress); 89 } 90 if (tokenPrice == 0) { 91 tokenPrice = getPriceFromDex(tokenAddress); 92: }
require() or revert() statements that check input arguments should be at the top of the functionChecks that involve constants should come before checks that involve state variables
There are 6 instances of this issue:
File: contracts/contracts/core/connext/helpers/LPToken.sol 50: require(to != address(this), "LPToken: cannot send to itself");
File: contracts/contracts/core/connext/helpers/StableSwap.sol 167: require(address(getToken(index)) == tokenAddress, "Token does not exist");
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 396: require(tokenIndexFrom != tokenIndexTo, "Can't compare token to itself");
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 123: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); 141: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); 161: require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)");
The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})
There is 1 instance of this issue:
File: contracts/contracts/core/promise/PromiseRouter.sol #1 132: receive() external payable {}
block.timestamp and block.number are added to event information by default so adding them manually wastes gas
There are 4 instances of this issue:
File: contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol #1 52: event RouterOwnershipRenunciationProposed(uint256 timestamp);
File: contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol #2 56: event AssetOwnershipRenunciationProposed(uint256 timestamp);
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #3 62: event RouterOwnershipRenunciationProposed(uint256 timestamp);
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol #4 66: event AssetOwnershipRenunciationProposed(uint256 timestamp);
revert()/require() strings to save gasCustom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas
There are 79 instances of this issue:
File: contracts/contracts/core/connext/facets/BaseConnextFacet.sol 38: require(s._status != _ENTERED, "ReentrancyGuard: reentrant call"); 125: require(_remote != bytes32(0), "!remote");
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 72: require(msg.sender == admin, "caller is not the admin"); 150: require(baseTokenPrice > 0, "invalid base token");
File: contracts/contracts/core/connext/helpers/Executor.sol 57: require(msg.sender == connext, "#OC:027");
File: contracts/contracts/core/connext/helpers/LPToken.sol 35: require(amount != 0, "LPToken: cannot mint 0"); 50: require(to != address(this), "LPToken: cannot send to itself");
File: contracts/contracts/core/connext/helpers/StableSwap.sol 75: require(_pooledTokens.length > 1, "_pooledTokens.length <= 1"); 76: require(_pooledTokens.length <= 32, "_pooledTokens.length > 32"); 77: require(_pooledTokens.length == decimals.length, "_pooledTokens decimals mismatch"); 84 require( 85 tokenIndexes[address(_pooledTokens[i])] == 0 && _pooledTokens[0] != _pooledTokens[i], 86 "Duplicate tokens" 87: ); 89: require(address(_pooledTokens[i]) != address(0), "The 0 address isn't an ERC-20"); 90: require(decimals[i] <= SwapUtils.POOL_PRECISION_DECIMALS, "Token decimals exceeds max"); 96: require(_a < AmplificationUtils.MAX_A, "_a exceeds maximum"); 97: require(_fee < SwapUtils.MAX_SWAP_FEE, "_fee exceeds maximum"); 98: require(_adminFee < SwapUtils.MAX_ADMIN_FEE, "_adminFee exceeds maximum"); 102: require(lpToken.initialize(lpTokenName, lpTokenSymbol), "could not init lpToken clone"); 125: require(block.timestamp <= deadline, "Deadline not met"); 155: require(index < swapStorage.pooledTokens.length, "Out of range"); 167: require(address(getToken(index)) == tokenAddress, "Token does not exist"); 177: require(index < swapStorage.pooledTokens.length, "Index out of range");
File: contracts/contracts/core/connext/libraries/AmplificationUtils.sol 84: require(block.timestamp >= self.initialATime.add(1 days), "Wait 1 day before starting ramp"); 85: require(futureTime_ >= block.timestamp.add(MIN_RAMP_TIME), "Insufficient ramp time"); 86: require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A"); 92: require(futureAPrecise.mul(MAX_A_CHANGE) >= initialAPrecise, "futureA_ is too small"); 94: require(futureAPrecise <= initialAPrecise.mul(MAX_A_CHANGE), "futureA_ is too large"); 111: require(self.futureATime > block.timestamp, "Ramp is already stopped");
File: contracts/contracts/core/connext/libraries/SwapUtils.sol 191: require(tokenIndex < xp.length, "Token index out of range"); 198: require(tokenAmount <= xp[tokenIndex], "Withdraw exceeds available"); 248: require(tokenIndex < numTokens, "Token not found"); 342: require(numTokens == precisionMultipliers.length, "Balances must match multipliers"); 396: require(tokenIndexFrom != tokenIndexTo, "Can't compare token to itself"); 397: require(tokenIndexFrom < numTokens && tokenIndexTo < numTokens, "Tokens must be in pool"); 493: require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); 524: require(tokenIndexFrom < xp.length && tokenIndexTo < xp.length, "Token index out of range"); 554: require(amount <= totalSupply, "Cannot exceed total supply"); 615: require(index < self.pooledTokens.length, "Token index out of range"); 649: require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); 662: require(dy >= minDy, "Swap didn't result in min tokens"); 697: require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance"); 703: require(dx <= maxDx, "Swap needs more than max tokens"); 717: require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); 723: require(dx == tokenFrom.balanceOf(address(this)).sub(beforeBalance), "not support fee token"); 750: require(dx <= tokenFrom.balanceOf(msg.sender), "Cannot swap more than you own"); 756: require(dy >= minDy, "Swap didn't result in min tokens"); 784: require(dy <= self.balances[tokenIndexTo], "Cannot get more than pool balance"); 790: require(dx <= maxDx, "Swap didn't result in min tokens"); 823: require(amounts.length == pooledTokens.length, "Amounts must match pooled tokens"); 845: require(v.totalSupply != 0 || amounts[i] > 0, "Must supply all tokens in pool"); 861: require(v.d1 > v.d0, "D should increase"); 890: require(toMint >= minToMint, "Couldn't mint min requested"); 916: require(amount <= lpToken.balanceOf(msg.sender), ">LP.balanceOf"); 917: require(minAmounts.length == pooledTokens.length, "minAmounts must match poolTokens"); 925: require(amounts[i] >= minAmounts[i], "amounts[i] < minAmounts[i]"); 954: require(tokenAmount <= lpToken.balanceOf(msg.sender), ">LP.balanceOf"); 955: require(tokenIndex < pooledTokens.length, "Token not found"); 961: require(dy >= minAmount, "dy < minAmount"); 1005: require(amounts.length == pooledTokens.length, "Amounts should match pool tokens"); 1007: require(maxBurnAmount <= v.lpToken.balanceOf(msg.sender) && maxBurnAmount != 0, ">LP.balanceOf"); 1032: require(tokenAmount != 0, "Burnt amount cannot be zero"); 1035: require(tokenAmount <= maxBurnAmount, "tokenAmount > maxBurnAmount"); 1071: require(newAdminFee <= MAX_ADMIN_FEE, "Fee is too high"); 1084: require(newSwapFee <= MAX_SWAP_FEE, "Fee is too high");
File: contracts/contracts/core/connext/libraries/LibDiamond.sol 66: require(msg.sender == diamondStorage().contractOwner, "LibDiamond: Must be contract owner"); 100 require( 101 diamondStorage().acceptanceTimes[keccak256(abi.encode(_diamondCut))] < block.timestamp, 102 "LibDiamond: delay not elapsed" 103: ); 121: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 123: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); 132: require(oldFacetAddress == address(0), "LibDiamondCut: Can't add function that already exists"); 139: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 141: require(_facetAddress != address(0), "LibDiamondCut: Add facet can't be address(0)"); 150: require(oldFacetAddress != _facetAddress, "LibDiamondCut: Can't replace function with same function"); 158: require(_functionSelectors.length > 0, "LibDiamondCut: No selectors in facet to cut"); 161: require(_facetAddress == address(0), "LibDiamondCut: Remove facet address must be address(0)"); 191: require(_facetAddress != address(0), "LibDiamondCut: Can't remove function that doesn't exist"); 193: require(_facetAddress != address(this), "LibDiamondCut: Can't remove immutable function"); 224: require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty"); 226: require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)"); 247: require(contractSize > 0, _errorMessage);
File: contracts/contracts/core/connext/libraries/ConnextMessage.sol 116: require(isValidAction(_action), "!action");
payableIf 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. 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
There are 85 instances of this issue:
File: contracts/contracts/core/connext/facets/StableSwapFacet.sol 393 function initializeSwap( 394 bytes32 _canonicalId, 395 IERC20[] memory _pooledTokens, 396 uint8[] memory decimals, 397 string memory lpTokenName, 398 string memory lpTokenSymbol, 399 uint256 _a, 400 uint256 _fee, 401 uint256 _adminFee, 402 address lpTokenTargetAddress 403: ) external onlyOwner { 460: function withdrawSwapAdminFees(bytes32 canonicalId) external onlyOwner { 469: function setSwapAdminFee(bytes32 canonicalId, uint256 newAdminFee) external onlyOwner { 478: function setSwapFee(bytes32 canonicalId, uint256 newSwapFee) external onlyOwner { 490 function rampA( 491 bytes32 canonicalId, 492 uint256 futureA, 493 uint256 futureTime 494: ) external onlyOwner { 502: function stopRampA(bytes32 canonicalId) external onlyOwner {
File: contracts/contracts/core/connext/facets/BridgeFacet.sol 233: function setPromiseRouter(address payable _promiseRouter) external onlyOwner { 242: function setExecutor(address _executor) external onlyOwner { 250: function setSponsorVault(address _sponsorVault) external onlyOwner { 389 function handle( 390 uint32 _origin, 391 uint32 _nonce, 392 bytes32 _sender, 393 bytes memory _message 394: ) external onlyReplica onlyRemoteRouter(_origin, _sender) { 389 function handle( 390 uint32 _origin, 391 uint32 _nonce, 392 bytes32 _sender, 393 bytes memory _message 394: ) external onlyReplica onlyRemoteRouter(_origin, _sender) {
File: contracts/contracts/core/connext/facets/AssetFacet.sol 100: function setWrapper(address _wrapper) external onlyOwner { 112: function setTokenRegistry(address _tokenRegistry) external onlyOwner { 132 function setupAsset( 133 ConnextMessage.TokenId calldata _canonical, 134 address _adoptedAssetId, 135 address _stableSwapPool 136: ) external onlyOwner { 162: function addStableSwapPool(ConnextMessage.TokenId calldata _canonical, address _stableSwapPool) external onlyOwner { 171: function removeAssetId(bytes32 _canonicalId, address _adoptedAssetId) external onlyOwner {
File: contracts/contracts/core/connext/facets/PortalFacet.sol 57: function setAavePool(address _aavePool) external onlyOwner { 65: function setAavePortalFee(uint256 _aavePortalFeeNumerator) external onlyOwner {
File: contracts/contracts/core/connext/facets/RelayerFacet.sol 88: function setRelayerFeeRouter(address _relayerFeeRouter) external onlyOwner { 101: function addRelayer(address _relayer) external onlyOwner { 112: function removeRelayer(address _relayer) external onlyOwner { 161: function claim(address _recipient, bytes32[] calldata _transferIds) external onlyRelayerFeeRouter {
File: contracts/contracts/core/connext/facets/ProposedOwnableFacet.sol 128: function proposeRouterOwnershipRenunciation() public onlyOwner { 142: function renounceRouterOwnership() public onlyOwner { 162: function proposeAssetOwnershipRenunciation() public onlyOwner { 175: function renounceAssetOwnership() public onlyOwner { 203: function proposeNewOwner(address newlyProposed) public onlyOwner { 217: function renounceOwnership() public onlyOwner { 236: function acceptProposedOwner() public onlyProposed { 253: function pause() public onlyOwner { 258: function unpause() public onlyOwner {
File: contracts/contracts/core/connext/facets/NomadFacet.sol 25: function setXAppConnectionManager(address _xAppConnectionManager) external onlyOwner { 34: function enrollRemoteRouter(uint32 _domain, bytes32 _router) external onlyOwner {
File: contracts/contracts/core/connext/facets/RoutersFacet.sol 259 function setupRouter( 260 address router, 261 address owner, 262 address recipient 263: ) external onlyOwner { 293: function removeRouter(address router) external onlyOwner { 331: function setMaxRoutersPerTransfer(uint256 _newMaxRouters) external onlyOwner { 345: function setLiquidityFeeNumerator(uint256 _numerator) external onlyOwner { 361: function approveRouterForPortal(address _router) external onlyOwner { 375: function unapproveRouterForPortal(address _router) external onlyOwner { 393: function setRouterRecipient(address router, address recipient) external onlyRouterOwner(router) { 410: function proposeRouterOwner(address router, address proposed) external onlyRouterOwner(router) { 430: function acceptProposedRouterOwner(address router) external onlyProposedRouterOwner(router) {
File: contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol 142 function setDexPriceInfo( 143 address _token, 144 address _baseToken, 145 address _lpToken, 146 bool _active 147: ) external onlyAdmin { 158: function setDirectPrice(address _token, uint256 _price) external onlyAdmin { 163: function setV1PriceOracle(address _v1PriceOracle) external onlyAdmin { 168: function setAdmin(address newAdmin) external onlyAdmin { 175: function setAggregators(address[] calldata tokenAddresses, address[] calldata sources) external onlyAdmin {
File: contracts/contracts/core/connext/helpers/Executor.sol 113: function execute(ExecutorArgs memory _args) external payable override onlyConnext returns (bool, bytes memory) {
File: contracts/contracts/core/connext/helpers/LPToken.sol 34: function mint(address recipient, uint256 amount) external onlyOwner {
File: contracts/contracts/core/connext/helpers/StableSwap.sol 440: function withdrawAdminFees() external onlyOwner { 448: function setAdminFee(uint256 newAdminFee) external onlyOwner { 456: function setSwapFee(uint256 newSwapFee) external onlyOwner { 467: function rampA(uint256 futureA, uint256 futureTime) external onlyOwner { 474: function stopRampA() external onlyOwner {
File: contracts/contracts/core/connext/helpers/ProposedOwnableUpgradeable.sol 77: function __ProposedOwnable_init() internal onlyInitializing { 81: function __ProposedOwnable_init_unchained() internal onlyInitializing { 155: function proposeRouterOwnershipRenunciation() public virtual onlyOwner { 169: function renounceRouterOwnership() public virtual onlyOwner { 197: function proposeAssetOwnershipRenunciation() public virtual onlyOwner { 211: function renounceAssetOwnership() public virtual onlyOwner { 239: function proposeNewOwner(address newlyProposed) public virtual onlyOwner { 253: function renounceOwnership() public virtual onlyOwner { 272: function acceptProposedOwner() public virtual onlyProposed {
File: contracts/contracts/core/connext/helpers/SponsorVault.sol 138: function setConnext(address _connext) external onlyOwner { 147: function setRate(uint32 _originDomain, Rate calldata _rate) external onlyOwner { 159: function setRelayerFeeCap(uint256 _relayerFeeCap) external onlyOwner { 168: function setGasTokenOracle(address _gasTokenOracle) external onlyOwner { 178: function setTokenExchange(address _token, address payable _tokenExchange) external onlyOwner { 196 function reimburseLiquidityFees( 197 address _token, 198 uint256 _liquidityFee, 199 address _receiver 200: ) external override onlyConnext returns (uint256) { 234 function reimburseRelayerFees( 235 uint32 _originDomain, 236 address payable _to, 237 uint256 _originRelayerFee 238: ) external override onlyConnext { 286 function withdraw( 287 address _token, 288 address _receiver, 289 uint256 _amount 290: ) external onlyOwner {
File: contracts/contracts/core/promise/PromiseRouter.sol 155: function setConnext(address _connext) external onlyOwner { 170 function send( 171 uint32 _domain, 172 bytes32 _transferId, 173 address _callbackAddress, 174 bool _returnSuccess, 175 bytes calldata _returnData 176: ) external onlyConnext { 205 function handle( 206 uint32 _origin, 207 uint32 _nonce, 208 bytes32 _sender, 209 bytes memory _message 210: ) external override onlyReplica onlyRemoteRouter(_origin, _sender) { 205 function handle( 206 uint32 _origin, 207 uint32 _nonce, 208 bytes32 _sender, 209 bytes memory _message 210: ) external override onlyReplica onlyRemoteRouter(_origin, _sender) { 268: function initCallbackFee(bytes32 _transferId) external payable onlyConnext {
File: contracts/contracts/core/relayer-fee/RelayerFeeRouter.sol 89: function setConnext(address _connext) external onlyOwner { 102 function send( 103 uint32 _domain, 104 address _recipient, 105 bytes32[] calldata _transferIds 106: ) external onlyConnext { 130 function handle( 131 uint32 _origin, 132 uint32 _nonce, 133 bytes32 _sender, 134 bytes memory _message 135: ) external override onlyReplica onlyRemoteRouter(_origin, _sender) { 130 function handle( 131 uint32 _origin, 132 uint32 _nonce, 133 bytes32 _sender, 134 bytes memory _message 135: ) external override onlyReplica onlyRemoteRouter(_origin, _sender) {
File: contracts/contracts/core/shared/ProposedOwnable.sol 109: function proposeNewOwner(address newlyProposed) public virtual onlyOwner { 123: function renounceOwnership() public virtual onlyOwner { 142: function acceptProposedOwner() public virtual onlyProposed { 180: function __ProposedOwnable_init() internal onlyInitializing { 184: function __ProposedOwnable_init_unchained() internal onlyInitializing {
#0 - itsmetechjay
2022-06-21T13:43:23Z
Warden created this issue as a placeholder, because their submission was too large for the contest form. They then emailed their md file to our team on 06/19/2022 at 10:16am central. I've updated this issue with their md file content.