Platform: Code4rena
Start Date: 30/05/2023
Pot Size: $300,500 USDC
Total HM: 79
Participants: 101
Period: about 1 month
Judge: Trust
Total Solo HM: 36
Id: 242
League: ETH
Rank: 29/101
Findings: 4
Award: $1,600.94
๐ Selected for report: 0
๐ Solo Findings: 0
๐ Selected for report: Madalad
Also found by: IllIllI, MohammedRizwan, ihtishamsudo, jasonxiale
172.8238 USDC - $172.82
Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions on L1 do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert (see this link for a test case).
File: src/talos/base/TalosBaseStrategy.sol 409: if (amount0 > 0) _token0.transfer(msg.sender, amount0); 410: if (amount1 > 0) _token1.transfer(msg.sender, amount1);
IllIllI-bot
Use OpenZeppelinโs SafeERC20's safeTransfer()/safeTransferFrom() instead
ERC20
#0 - c4-judge
2023-07-09T12:56:02Z
trust1995 marked the issue as primary issue
#1 - c4-judge
2023-07-09T12:56:06Z
trust1995 marked the issue as satisfactory
#2 - c4-sponsor
2023-07-12T16:58:36Z
0xLightt marked the issue as sponsor confirmed
#3 - c4-judge
2023-07-25T13:17:19Z
trust1995 marked the issue as selected for report
#4 - c4-judge
2023-07-26T14:24:25Z
trust1995 marked the issue as not selected for report
#5 - c4-judge
2023-07-26T14:24:43Z
trust1995 marked the issue as duplicate of #583
๐ Selected for report: Madalad
Also found by: IllIllI, MohammedRizwan, ihtishamsudo, jasonxiale
172.8238 USDC - $172.82
Not all IERC20 implementations revert() when there's a failure in transfer()/transferFrom(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment
File: src/talos/base/TalosBaseStrategy.sol 409: if (amount0 > 0) _token0.transfer(msg.sender, amount0); 410: if (amount1 > 0) _token1.transfer(msg.sender, amount1);
IllIllI-bot
revert if the return value does not indicate success
ERC20
#0 - c4-judge
2023-07-09T12:55:46Z
trust1995 marked the issue as primary issue
#1 - c4-judge
2023-07-09T12:55:53Z
trust1995 marked the issue as satisfactory
#2 - c4-judge
2023-07-09T12:56:45Z
trust1995 marked the issue as duplicate of #516
#3 - c4-judge
2023-07-09T14:19:57Z
trust1995 marked the issue as not a duplicate
#4 - trust1995
2023-07-09T14:21:16Z
Issue is deemed separate from #516.
The semantics around transfer()
call are not respected by the callee, as defined by the ERC20 specification.
#5 - c4-judge
2023-07-09T14:26:50Z
trust1995 marked the issue as primary issue
#6 - c4-sponsor
2023-07-12T16:59:24Z
0xLightt marked the issue as sponsor confirmed
#7 - c4-judge
2023-07-25T13:16:41Z
trust1995 marked issue #583 as primary and marked this issue as a duplicate of 583
๐ Selected for report: 0xnev
Also found by: 0xSmartContract, Breeje, BugBusters, ByteBandits, IllIllI, Kaiziron, Madalad, MohammedRizwan, SpicyMeatball, T1MOH, kutugu, nadin, peanuts, said, shealtielanz, tsvetanovv
14.356 USDC - $14.36
Passing block.timestamp as the expiry/deadline of an operation does not mean "require immediate execution" - it means "whatever block this transaction appears in, I'm comfortable with that block's timestamp". Providing this value means that a malicious miner can hold the transaction for as long as they like (think the flashbots mempool for bundling transactions), which may be until they are able to cause the transaction to incur the maximum amount of slippage allowed by the slippage parameter, or until conditions become unfavorable enough that other orders, e.g. liquidations, are triggered. Timestamps should be chosen off-chain, and should be specified by the caller to avoid unnecessary MEV.
There are 6 instances of this issue:
File: src/talos/TalosStrategyVanilla.sol 143 tokenId: _tokenId, 144 amount0Desired: balance0, 145 amount1Desired: balance1, 146 amount0Min: 0, 147 amount1Min: 0, 148 deadline: block.timestamp 149 }) 150 ); 151: liquidity += liquidityDifference;
File: src/talos/base/TalosBaseStrategy.sol 142 amount0Desired: amount0Desired, 143 amount1Desired: amount1Desired, 144 amount0Min: 0, 145 amount1Min: 0, 146 recipient: address(this), 147 deadline: block.timestamp 148 }) 149 ); 150: 203 tokenId: _tokenId, 204 amount0Desired: amount0Desired, 205 amount1Desired: amount1Desired, 206 amount0Min: 0, 207 amount1Min: 0, 208 deadline: block.timestamp 209 }) 210 ); 211: 264 INonfungiblePositionManager.DecreaseLiquidityParams({ 265 tokenId: _tokenId, 266 liquidity: liquidityToDecrease, 267 amount0Min: amount0Min, 268 amount1Min: amount1Min, 269 deadline: block.timestamp 270 }) 271 ); 272: 354 INonfungiblePositionManager.DecreaseLiquidityParams({ 355 tokenId: _tokenId, 356 liquidity: _liquidity, 357 amount0Min: 0, 358 amount1Min: 0, 359 deadline: block.timestamp 360 }) 361 ); 362: liquidity = 0;
File: src/talos/libraries/PoolActions.sol 80 amount0Desired: balance0, 81 amount1Desired: balance1, 82 amount0Min: 0, 83 amount1Min: 0, 84 recipient: address(this), 85 deadline: block.timestamp 86 }) 87 ); 88: }
IllIllI-bot
Require the user to pass in a timestamp
MEV
#0 - c4-judge
2023-07-11T07:30:50Z
trust1995 marked the issue as duplicate of #171
#1 - c4-judge
2023-07-11T07:30:56Z
trust1995 marked the issue as satisfactory
๐ Selected for report: 0xSmartContract
Also found by: 3kus-iosiro, Audit_Avengers, ByteBandits, IllIllI, Kamil-Chmielewski, Madalad, RED-LOTUS-REACH, Rolezn, Sathish9098, Stormreckson, Udsen, bin2chen, brgltd, ihtishamsudo, kaveyjoe, kodyvim, lukejohn, matrix_0wl, mgf15, nadin
803.432 USDC - $803.43
Issue | Instances | |
---|---|---|
[Lโ01] | Initialization can be front-run | 10 |
[Lโ02] | Tokens may be minted to address(0x0) | 10 |
[Lโ03] | decimals() is not a part of the ERC-20 standard | 20 |
[Lโ04] | name() is not a part of the ERC-20 standard | 4 |
[Lโ05] | symbol() is not a part of the ERC-20 standard | 4 |
[Lโ06] | File allows a version of solidity that is susceptible to an assembly optimizer bug | 1 |
[Lโ07] | Solidity version 0.8.20 may not work on other chains due to PUSH0 | 153 |
Total: 202 instances over 7 issues
Issue | Instances | |
---|---|---|
[Nโ01] | Duplicate import statements | 7 |
[Nโ02] | override function arguments that are unused should have the variable name removed or commented out to avoid compiler warnings | 5 |
[Nโ03] | Non-assembly method available | 1 |
[Nโ04] | Inconsistent method of specifying a floating pragma | 11 |
[Nโ05] | Using > />= without specifying an upper bound is unsafe | 11 |
[Nโ06] | NatSpec @param is listed multiple times | 2 |
[Nโ07] | Avoid the use of sensitive terms | 33 |
[Nโ08] | Large assembly blocks should have extensive comments | 2 |
[Nโ09] | Visibility should be set explicitly rather than defaulting to internal | 2 |
[Nโ10] | Control structures do not follow the Solidity Style Guide | 13 |
[Nโ11] | Expressions for constant values such as a call to keccak256() , should use immutable rather than constant | 3 |
[Nโ12] | Numeric values having to do with time should use time units for readability | 11 |
[Nโ13] | Consider using delete rather than assigning zero/false to clear values | 19 |
[Nโ14] | Contracts should have full test coverage | 1 |
[Nโ15] | internal functions not called by the contract should be removed | 1 |
Total: 122 instances over 15 issues
The initialize()
functions below are not called by another contract atomically after the contract is deployed, so it's possible for a malicious user to call initialize()
which, if it's noticed in time, would require the project to re-deploy the contract in order to properly initialize. Consider creating a factory contract, which will new
and initialize()
each contract atomically.
There are 10 instances of this issue:
<details> <summary>see instances</summary>File: src/governance/GovernorBravoDelegateMaia.sol 56 function initialize( 57 address timelock_, 58 address govToken_, 59 uint256 votingPeriod_, 60 uint256 votingDelay_, 61 uint256 proposalThreshold_ 62: ) public virtual {
File: src/hermes/minters/BaseV2Minter.sol 78: function initialize(FlywheelGaugeRewards _flywheelGaugeRewards) external {
File: src/ulysses-omnichain/BaseBranchRouter.sol 37: function initialize(address _localBridgeAgentAddress) external onlyOwner {
File: src/ulysses-omnichain/BranchPort.sol 99: function initialize(address _coreBranchRouter, address _bridgeAgentFactory) external virtual onlyOwner {
File: src/ulysses-omnichain/CoreRootRouter.sol 63: function initialize(address _bridgeAgentAddress, address _hTokenFactory) external onlyOwner {
File: src/ulysses-omnichain/MulticallRootRouter.sol 74: function initialize(address _bridgeAgentAddress) external onlyOwner {
File: src/ulysses-omnichain/RootPort.sol 128: function initialize(address _bridgeAgentFactory, address _coreRootRouter) external onlyOwner {
File: src/ulysses-omnichain/factories/ArbitrumBranchBridgeAgentFactory.sol 54: function initialize(address _coreRootBridgeAgent) external override onlyOwner {
File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol 35: function initialize(address _wrappedNativeTokenAddress, address _coreRouter) external onlyOwner {
</details>File: src/ulysses-omnichain/factories/ERC20hTokenRootFactory.sol 40: function initialize(address _coreRouter) external onlyOwner {
address(0x0)
Neither the listed functions, nor _mint()
prevent minting to address(0x0)
There are 10 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-4626/ERC4626.sol 47 function mint(uint256 shares, address receiver) public virtual returns (uint256 assets) { 48 assets = previewMint(shares); // No need to check for rounding error, previewMint rounds up. 49 50 // Need to transfer before minting or ERC777s could reenter. 51 address(asset).safeTransferFrom(msg.sender, address(this), assets); 52 53 _mint(receiver, shares); 54 55 emit Deposit(msg.sender, receiver, assets, shares); 56 57 afterDeposit(assets, shares); 58: }
File: src/erc-4626/ERC4626DepositOnly.sol 47 function mint(uint256 shares, address receiver) public virtual returns (uint256 assets) { 48 assets = previewMint(shares); // No need to check for rounding error, previewMint rounds up. 49 50 // Need to transfer before minting or ERC777s could reenter. 51 address(asset).safeTransferFrom(msg.sender, address(this), assets); 52 53 _mint(receiver, shares); 54 55 emit Deposit(msg.sender, receiver, assets, shares); 56 57 afterDeposit(assets, shares); 58: }
File: src/erc-4626/ERC4626MultiToken.sol 113 function mint(uint256 shares, address receiver) 114 public 115 virtual 116 nonReentrant 117 returns (uint256[] memory assetsAmounts) 118 { 119 assetsAmounts = previewMint(shares); // No need to check for rounding error, previewMint rounds up. 120 121 // Need to transfer before minting or ERC777s could reenter. 122 receiveAssets(assetsAmounts); 123 124 _mint(receiver, shares); 125 126 emit Deposit(msg.sender, receiver, assetsAmounts, shares); 127 128 afterDeposit(assetsAmounts, shares); 129: }
File: src/hermes/tokens/HERMES.sol 62 function mint(address account, uint256 amount) external onlyOwner { 63 _mint(account, amount); 64: }
File: src/hermes/tokens/bHermesBoost.sol 28 function mint(address to, uint256 amount) external onlybHermes { 29 _mint(to, amount); 30: }
File: src/hermes/tokens/bHermesGauges.sol 35 function mint(address to, uint256 amount) external onlybHermes { 36 _mint(to, amount); 37: }
File: src/hermes/tokens/bHermesVotes.sol 26 function mint(address to, uint256 amount) external onlybHermes { 27 _mint(to, amount); 28: }
File: src/maia/tokens/Maia.sol 55 function mint(address account, uint256 amount) external onlyOwner { 56 _mint(account, amount); 57: }
File: src/ulysses-omnichain/token/ERC20hTokenBranch.sol 23 function mint(address account, uint256 amount) external override onlyOwner returns (bool) { 24 _mint(account, amount); 25 return true; 26: }
</details>File: src/ulysses-omnichain/token/ERC20hTokenRoot.sol 72 function mint(address to, uint256 amount, uint256 chainId) external requiresPort returns (bool) { 73 getTokenBalance[chainId] += amount; 74 _mint(to, amount); 75 return true; 76: }
decimals()
is not a part of the ERC-20 standardThe decimals()
function is not a part of the ERC-20 standard, and was added later as an optional extension. As such, some valid ERC20 tokens do not support this interface, so it is unsafe to blindly cast all tokens to this interface, and then call this function.
There are 20 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-4626/ERC4626.sol 23: constructor(ERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, _asset.decimals()) {
File: src/erc-4626/ERC4626DepositOnly.sol 23: constructor(ERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol, _asset.decimals()) {
File: src/erc-4626/ERC4626MultiToken.sol 51: require(ERC20(_assets[i]).decimals() == 18);
File: src/erc-4626/UlyssesERC4626.sol 27: if (ERC20(_asset).decimals() != 18) revert InvalidAssetDecimals();
File: src/ulysses-amm/UlyssesToken.sol 46: require(ERC20(asset).decimals() == 18);
File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol 104: msg.sender, msg.sender, underlyingAddress, _normalizeDecimals(amount, ERC20(underlyingAddress).decimals())
File: src/ulysses-omnichain/ArbitrumBranchPort.sol 72: underlyingAddress.safeTransfer(_recipient, _denormalizeDecimals(_deposit, ERC20(underlyingAddress).decimals())); 82: _recipient, _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals()) 120: _depositor, address(this), _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals()) 141: _denormalizeDecimals(_deposits[i], ERC20(_underlyingAddresses[i]).decimals())
File: src/ulysses-omnichain/BranchBridgeAgent.sol 252: _normalizeDecimals(_dParams.deposit, ERC20(_dParams.token).decimals()), 284: _deposits[i] = _normalizeDecimals(_dParams.deposits[i], ERC20(_dParams.tokens[i]).decimals()); 357: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() 342: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() 687: _normalizeDecimals(_dParams.deposit, ERC20(_dParams.token).decimals()), 720: deposits[i] = _normalizeDecimals(_dParams.deposits[i], ERC20(_dParams.tokens[i]).decimals()); 1355: deposits[i] = _normalizeDecimals(_deposits[i], ERC20(_tokens[i]).decimals());
</details>File: src/ulysses-omnichain/BranchPort.sol 212: _recipient, _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals()) 254: _depositor, address(this), _denormalizeDecimals(_deposit, ERC20(_underlyingAddress).decimals()) 272: _denormalizeDecimals(_deposits[i], ERC20(_underlyingAddresses[i]).decimals())
name()
is not a part of the ERC-20 standardThe name()
function is not a part of the ERC-20 standard, and was added later as an optional extension. As such, some valid ERC20 tokens do not support this interface, so it is unsafe to blindly cast all tokens to this interface, and then call this function.
There are 4 instances of this issue:
File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol 49: string memory name = ERC20(_underlyingAddress).name();
File: src/ulysses-omnichain/CoreBranchRouter.sol 65: string memory name = ERC20(_underlyingAddress).name();
File: src/ulysses-omnichain/CoreRootRouter.sol 159: _globalAddress, ERC20(_globalAddress).name(), ERC20(_globalAddress).symbol(), _remoteExecutionGas
File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol 39: ERC20(_wrappedNativeTokenAddress).name(),
symbol()
is not a part of the ERC-20 standardThe symbol()
function is not a part of the ERC-20 standard, and was added later as an optional extension. As such, some valid ERC20 tokens do not support this interface, so it is unsafe to blindly cast all tokens to this interface, and then call this function.
There are 4 instances of this issue:
File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol 50: string memory symbol = ERC20(_underlyingAddress).symbol();
File: src/ulysses-omnichain/CoreBranchRouter.sol 66: string memory symbol = ERC20(_underlyingAddress).symbol();
File: src/ulysses-omnichain/CoreRootRouter.sol 159: _globalAddress, ERC20(_globalAddress).name(), ERC20(_globalAddress).symbol(), _remoteExecutionGas
File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol 40: ERC20(_wrappedNativeTokenAddress).symbol(),
In solidity versions 0.8.13 and 0.8.14, there is an optimizer bug where, if the use of a variable is in a separate assembly
block from the block in which it was stored, the mstore
operation is optimized out, leading to uninitialized memory. The code currently does not have such a pattern of execution, but it does use mstore
s in assembly
blocks, so it is a risk for future changes. The affected solidity versions should be avoided if at all possible.
There is one instance of this issue:
File: src/ulysses-amm/UlyssesPool.sol 380 assembly { 381 // Store bandwidth state slot in memory 382 mstore(0x00, bandwidthStateList.slot) 383 // Hash the bandwidth state slot to get the bandwidth state list start 384 let bandwidthStateListStart := keccak256(0x00, 0x20) 385 386 // Total difference from target bandwidth of all bandwidth states 387 let totalDiff 388 // Total difference from target bandwidth of all bandwidth states 389 let transfered 390 // Total amount to be distributed according to each bandwidth weights 391 let transferedChange 392 393 for { let i := 1 } lt(i, length) { i := add(i, 1) } { 394 // Load bandwidth and weight from storage 395 // Each bandwidth state occupies two storage slots 396 let slot := sload(add(bandwidthStateListStart, mul(i, 2))) 397 // Bandwidth is the first 248 bits of the slot 398 let bandwidth := and(slot, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff) 399 // Weight is the last 8 bits of the slot 400 let weight := shr(248, slot) 401 402 // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`. 403 if mul(weight, gt(_totalSupply, div(not(0), weight))) { 404 // Store the function selector of `MulDivFailed()`. 405 mstore(0x00, 0xad251c27) 406 // Revert with (offset, size). 407 revert(0x1c, 0x04) 408 } 409 410 // Calculate the target bandwidth 411 let targetBandwidth := div(mul(_totalSupply, weight), _totalWeights) 412 413 // Calculate the difference from the target bandwidth 414 switch positiveTransfer 415 // If the transfer is positive, calculate deficit from target bandwidth 416 case true { 417 // If there is a deficit, store the difference 418 if gt(targetBandwidth, bandwidth) { 419 // Calculate the difference 420 let diff := sub(targetBandwidth, bandwidth) 421 // Add the difference to the total difference 422 totalDiff := add(totalDiff, diff) 423 // Store the difference in the diffs array 424 mstore(add(diffs, add(mul(i, 0x20), 0x20)), diff) 425 } 426 } 427 // If the transfer is negative, calculate surplus from target bandwidth 428 default { 429 // If there is a surplus, store the difference 430 if gt(bandwidth, targetBandwidth) { 431 // Calculate the difference 432 let diff := sub(bandwidth, targetBandwidth) 433 // Add the difference to the total difference 434 totalDiff := add(totalDiff, diff) 435 // Store the difference in the diffs array 436 mstore(add(diffs, add(mul(i, 0x20), 0x20)), diff) 437 } 438 } 439 } 440 441 // Calculate the amount to be distributed according deficit/surplus 442 // and/or the amount to be distributed according to each bandwidth weights 443 switch gt(amount, totalDiff) 444 // If the amount is greater than the total deficit/surplus 445 case true { 446 // Total deficit/surplus is distributed 447 transfered := totalDiff 448 // Set rest to be distributed according to each bandwidth weights 449 transferedChange := sub(amount, totalDiff) 450 } 451 // If the amount is less than the total deficit/surplus 452 default { 453 // Amount will be distributed according to deficit/surplus 454 transfered := amount 455 } 456 457 for { let i := 1 } lt(i, length) { i := add(i, 1) } { 458 // Increase/decrease amount of bandwidth for each bandwidth state 459 let bandwidthUpdate 460 461 // If there is a deficit/surplus, calculate the amount to be distributed 462 if gt(transfered, 0) { 463 // Load the difference from the diffs array 464 let diff := mload(add(diffs, add(mul(i, 0x20), 0x20))) 465 466 // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`. 467 if mul(diff, gt(transfered, div(not(0), diff))) { 468 // Store the function selector of `MulDivFailed()`. 469 mstore(0x00, 0xad251c27) 470 // Revert with (offset, size). 471 revert(0x1c, 0x04) 472 } 473 474 // Calculate the amount to be distributed according to deficit/surplus 475 switch roundUp 476 // If round up then do mulDivUp(transfered, diff, totalDiff) 477 case true { 478 bandwidthUpdate := 479 add( 480 iszero(iszero(mod(mul(transfered, diff), totalDiff))), div(mul(transfered, diff), totalDiff) 481 ) 482 } 483 // If round down then do mulDiv(transfered, diff, totalDiff) 484 default { bandwidthUpdate := div(mul(transfered, diff), totalDiff) } 485 } 486 487 // If there is a rest, calculate the amount to be distributed according to each bandwidth weights 488 if gt(transferedChange, 0) { 489 // Load weight from storage 490 let weight := shr(248, sload(add(bandwidthStateListStart, mul(i, 2)))) 491 492 // Equivalent to `require(y == 0 || x <= type(uint256).max / y)`. 493 if mul(weight, gt(transferedChange, div(not(0), weight))) { 494 // Store the function selector of `MulDivFailed()`. 495 mstore(0x00, 0xad251c27) 496 // Revert with (offset, size). 497 revert(0x1c, 0x04) 498 } 499 500 // Calculate the amount to be distributed according to each bandwidth weights 501 switch roundUp 502 // If round up then do mulDivUp(transferedChange, weight, _totalWeights) 503 case true { 504 bandwidthUpdate := 505 add( 506 bandwidthUpdate, 507 add( 508 iszero(iszero(mod(mul(transferedChange, weight), _totalWeights))), 509 div(mul(transferedChange, weight), _totalWeights) 510 ) 511 ) 512 } 513 // If round down then do mulDiv(transferedChange, weight, _totalWeights) 514 default { 515 bandwidthUpdate := add(bandwidthUpdate, div(mul(transferedChange, weight), _totalWeights)) 516 } 517 } 518 519 // If there is an update in bandwidth 520 if gt(bandwidthUpdate, 0) { 521 // Store the amount to be updated in the bandwidthUpdateAmounts array 522 mstore(add(bandwidthUpdateAmounts, add(mul(i, 0x20), 0x20)), bandwidthUpdate) 523 } 524 } 525: }
PUSH0
The compiler for Solidity 0.8.20 switches the default target EVM version to Shanghai, which includes the new PUSH0
op code. This op code may not yet be implemented on all L2s, so deployment on these chains will fail. To work around this issue, use an earlier EVM version. While the project itself may or may not compile with 0.8.20, other projects with which it integrates, or which extend this project may, and those projects will have problems deploying these contracts/libraries.
There are 153 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-20/ERC20Boost.sol 3: pragma solidity ^0.8.0;
File: src/erc-20/ERC20Gauges.sol 3: pragma solidity ^0.8.0;
File: src/erc-20/ERC20MultiVotes.sol 4: pragma solidity ^0.8.0;
File: src/erc-20/interfaces/Errors.sol 2: pragma solidity ^0.8.0;
File: src/erc-20/interfaces/IERC20Boost.sol 2: pragma solidity ^0.8.0;
File: src/erc-20/interfaces/IERC20Gauges.sol 3: pragma solidity ^0.8.0;
File: src/erc-20/interfaces/IERC20MultiVotes.sol 3: pragma solidity ^0.8.0;
File: src/erc-4626/ERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/ERC4626DepositOnly.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/ERC4626MultiToken.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/UlyssesERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/interfaces/IERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/interfaces/IERC4626DepositOnly.sol 2: pragma solidity ^0.8.0;
File: src/erc-4626/interfaces/IERC4626MultiToken.sol 2: pragma solidity ^0.8.0;
File: src/erc-4626/interfaces/IUlyssesERC4626.sol 2: pragma solidity ^0.8.0;
File: src/gauges/BaseV2Gauge.sol 2: pragma solidity ^0.8.0;
File: src/gauges/UniswapV3Gauge.sol 2: pragma solidity ^0.8.0;
File: src/gauges/factories/BaseV2GaugeFactory.sol 3: pragma solidity ^0.8.0;
File: src/gauges/factories/BaseV2GaugeManager.sol 3: pragma solidity ^0.8.0;
File: src/gauges/factories/BribesFactory.sol 2: pragma solidity ^0.8.0;
File: src/gauges/factories/UniswapV3GaugeFactory.sol 3: pragma solidity ^0.8.0;
File: src/gauges/interfaces/IBaseV2Gauge.sol 2: pragma solidity ^0.8.0;
File: src/gauges/interfaces/IBaseV2GaugeFactory.sol 3: pragma solidity ^0.8.0;
File: src/gauges/interfaces/IBaseV2GaugeManager.sol 3: pragma solidity ^0.8.0;
File: src/gauges/interfaces/IBribesFactory.sol 2: pragma solidity ^0.8.0;
File: src/gauges/interfaces/IUniswapV3Gauge.sol 2: pragma solidity ^0.8.0;
File: src/gauges/interfaces/IUniswapV3GaugeFactory.sol 3: pragma solidity ^0.8.0;
File: src/governance/GovernorBravoDelegateMaia.sol 2: pragma solidity ^0.8.10;
File: src/governance/GovernorBravoDelegator.sol 2: pragma solidity ^0.8.10;
File: src/governance/GovernorBravoInterfaces.sol 2: pragma solidity ^0.8.10;
File: src/hermes/UtilityManager.sol 3: pragma solidity ^0.8.0;
File: src/hermes/bHermes.sol 2: pragma solidity ^0.8.0;
File: src/hermes/interfaces/IBaseV2Minter.sol 2: pragma solidity ^0.8.0;
File: src/hermes/interfaces/IUtilityManager.sol 2: pragma solidity ^0.8.0;
File: src/hermes/interfaces/IbHermesUnderlying.sol 2: pragma solidity ^0.8.0;
File: src/hermes/minters/BaseV2Minter.sol 2: pragma solidity ^0.8.0;
File: src/hermes/tokens/HERMES.sol 2: pragma solidity ^0.8.0;
File: src/hermes/tokens/bHermesBoost.sol 2: pragma solidity ^0.8.0;
File: src/hermes/tokens/bHermesGauges.sol 2: pragma solidity ^0.8.0;
File: src/hermes/tokens/bHermesVotes.sol 2: pragma solidity ^0.8.0;
File: src/maia/PartnerUtilityManager.sol 2: pragma solidity ^0.8.0;
File: src/maia/factories/PartnerManagerFactory.sol 2: pragma solidity ^0.8.0;
File: src/maia/interfaces/IBaseVault.sol 3: pragma solidity ^0.8.0;
File: src/maia/interfaces/IERC4626PartnerManager.sol 2: pragma solidity ^0.8.0;
File: src/maia/interfaces/IPartnerManagerFactory.sol 2: pragma solidity ^0.8.0;
File: src/maia/interfaces/IPartnerUtilityManager.sol 2: pragma solidity ^0.8.0;
File: src/maia/libraries/DateTimeLib.sol 2: pragma solidity ^0.8.4;
File: src/maia/tokens/ERC4626PartnerManager.sol 2: pragma solidity ^0.8.0;
File: src/maia/tokens/Maia.sol 2: pragma solidity ^0.8.0;
File: src/maia/vMaia.sol 3: pragma solidity ^0.8.0;
File: src/rewards/FlywheelCoreInstant.sol 2: pragma solidity ^0.8.0;
File: src/rewards/FlywheelCoreStrategy.sol 2: pragma solidity ^0.8.0;
File: src/rewards/base/BaseFlywheelRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/base/FlywheelCore.sol 3: pragma solidity ^0.8.0;
File: src/rewards/booster/FlywheelBoosterGaugeWeight.sol 3: pragma solidity ^0.8.0;
File: src/rewards/depots/MultiRewardsDepot.sol 2: pragma solidity ^0.8.0;
File: src/rewards/depots/RewardsDepot.sol 2: pragma solidity ^0.8.0;
File: src/rewards/depots/SingleRewardsDepot.sol 2: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelAcummulatedRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelBooster.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelBribeRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelCore.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelGaugeRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelInstantRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IFlywheelRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IMultiRewardsDepot.sol 2: pragma solidity ^0.8.0;
File: src/rewards/interfaces/IRewardsDepot.sol 2: pragma solidity ^0.8.0;
File: src/rewards/rewards/FlywheelAcummulatedRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/rewards/FlywheelBribeRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/rewards/FlywheelGaugeRewards.sol 3: pragma solidity ^0.8.0;
File: src/rewards/rewards/FlywheelInstantRewards.sol 3: pragma solidity ^0.8.0;
File: src/talos/TalosManager.sol 2: pragma solidity ^0.8.0;
File: src/talos/TalosOptimizer.sol 3: pragma solidity ^0.8.0;
File: src/talos/TalosStrategyStaked.sol 2: pragma solidity >=0.8.0;
File: src/talos/TalosStrategyVanilla.sol 3: pragma solidity >=0.8.0;
File: src/talos/base/TalosBaseStrategy.sol 3: pragma solidity >=0.8.0;
File: src/talos/boost-aggregator/BoostAggregator.sol 2: pragma solidity >=0.8.0;
File: src/talos/factories/BoostAggregatorFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/factories/OptimizerFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/factories/TalosBaseStrategyFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/factories/TalosStrategyStakedFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/factories/TalosStrategyVanillaFactory.sol 3: pragma solidity ^0.8.0;
File: src/talos/interfaces/AutomationCompatibleInterface.sol 2: pragma solidity ^0.8.0;
File: src/talos/interfaces/IBoostAggregator.sol 2: pragma solidity >=0.8.0;
File: src/talos/interfaces/IBoostAggregatorFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/interfaces/IOptimizerFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/interfaces/ITalosBaseStrategy.sol 3: pragma solidity ^0.8.0;
File: src/talos/interfaces/ITalosBaseStrategyFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/interfaces/ITalosManager.sol 2: pragma solidity ^0.8.0;
File: src/talos/interfaces/ITalosOptimizer.sol 3: pragma solidity ^0.8.0;
File: src/talos/interfaces/ITalosStrategyStaked.sol 2: pragma solidity ^0.8.0;
File: src/talos/interfaces/ITalosStrategyStakedFactory.sol 2: pragma solidity ^0.8.0;
File: src/talos/libraries/PoolActions.sol 3: pragma solidity ^0.8.0;
File: src/talos/libraries/PoolVariables.sol 3: pragma solidity ^0.8.0;
File: src/talos/strategies/TalosStrategySimple.sol 2: pragma solidity >=0.8.0;
File: src/ulysses-amm/UlyssesPool.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/UlyssesRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/UlyssesToken.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/factories/UlyssesFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/interfaces/IUlyssesFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/interfaces/IUlyssesPool.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/interfaces/IUlyssesRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-amm/interfaces/IUlyssesToken.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/ArbitrumBranchPort.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/BaseBranchRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/BranchBridgeAgent.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/BranchBridgeAgentExecutor.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/BranchPort.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/CoreBranchRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/CoreRootRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/MulticallRootRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/RootBridgeAgent.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/RootPort.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/VirtualAccount.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/factories/ArbitrumBranchBridgeAgentFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/factories/BranchBridgeAgentFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/factories/ERC20hTokenRootFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/factories/RootBridgeAgentFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IAnycallConfig.sol 3: pragma solidity ^0.8.10;
File: src/ulysses-omnichain/interfaces/IAnycallExecutor.sol 3: pragma solidity ^0.8.10;
File: src/ulysses-omnichain/interfaces/IAnycallProxy.sol 3: pragma solidity ^0.8.10;
File: src/ulysses-omnichain/interfaces/IApp.sol 3: pragma solidity ^0.8.10;
File: src/ulysses-omnichain/interfaces/IArbitrumBranchPort.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IBranchBridgeAgent.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IBranchBridgeAgentFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IBranchPort.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IBranchRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/ICoreBranchRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IERC20hTokenBranch.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IERC20hTokenBranchFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IERC20hTokenRoot.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IERC20hTokenRootFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IMulticall2.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IPortStrategy.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IRootBridgeAgent.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IRootBridgeAgentFactory.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IRootPort.sol 3: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IRootRouter.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IVirtualAccount.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/interfaces/IWETH9.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/lib/AnycallFlags.sol 3: pragma solidity ^0.8.10;
File: src/ulysses-omnichain/token/ERC20hTokenBranch.sol 2: pragma solidity ^0.8.0;
File: src/ulysses-omnichain/token/ERC20hTokenRoot.sol 2: pragma solidity ^0.8.0;
File: src/uni-v3-staker/UniswapV3Staker.sol 3: pragma solidity ^0.8.0;
File: src/uni-v3-staker/interfaces/IUniswapV3Staker.sol 3: pragma solidity ^0.8.0;
File: src/uni-v3-staker/libraries/IncentiveId.sol 2: pragma solidity ^0.8.0;
File: src/uni-v3-staker/libraries/IncentiveTime.sol 2: pragma solidity ^0.8.0;
File: src/uni-v3-staker/libraries/NFTPositionInfo.sol 3: pragma solidity ^0.8.0;
</details> ## Non-critical IssuesFile: src/uni-v3-staker/libraries/RewardMath.sol 2: pragma solidity ^0.8.0;
There are 7 instances of this issue:
<details> <summary>see instances</summary>File: src/talos/TalosManager.sol 11: import {ITalosManager, AutomationCompatibleInterface} from "./interfaces/ITalosManager.sol";
File: src/ulysses-omnichain/BranchBridgeAgent.sol 21 import { 22 Deposit, 23 DepositStatus, 24 DepositInput, 25 DepositMultipleInput, 26 DepositParams, 27 DepositMultipleParams, 28 SettlementParams, 29 SettlementMultipleParams 30: } from "./interfaces/IBranchBridgeAgent.sol";
File: src/ulysses-omnichain/CoreRootRouter.sol 16: import {DepositParams, DepositMultipleParams} from "./interfaces/IRootBridgeAgent.sol";
File: src/ulysses-omnichain/MulticallRootRouter.sol 13: import {DepositParams, DepositMultipleParams, Settlement} from "./interfaces/IRootBridgeAgent.sol";
File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol 9: import {DepositParams, DepositMultipleParams} from "./interfaces/IRootBridgeAgent.sol";
File: src/ulysses-omnichain/RootBridgeAgent.sol 26 import { 27 IRootBridgeAgent, 28 DepositParams, 29 DepositMultipleParams, 30 Settlement, 31 SettlementStatus, 32 SettlementParams, 33 SettlementMultipleParams, 34 UserFeeInfo, 35 SwapCallbackData 36: } from "./interfaces/IRootBridgeAgent.sol";
</details>File: src/ulysses-omnichain/RootPort.sol 17: import {VirtualAccount, GasPoolInfo} from "./interfaces/IRootPort.sol";
override
function arguments that are unused should have the variable name removed or commented out to avoid compiler warningsThere are 5 instances of this issue:
File: src/maia/vMaia.sol 91: function claimBoost(uint256 amount) public override {}
File: src/talos/TalosStrategyVanilla.sol 79: function afterRedeem(uint256 _tokenId) internal override {} 90: function afterDeposit(uint256 _tokenId) internal override {} 100: function afterRerange(uint256 _tokenId) internal override {}
File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol 149: function _gasSwapIn(bytes memory gasData) internal override returns (uint256 gasAmount) {
assembly{ id := chainid() }
=> uint256 id = block.chainid
, assembly { size := extcodesize() }
=> uint256 size = address().code.length
, assembly { hash := extcodehash() }
=> bytes32 hash = address().codehash
There are some automated tools that will flag a project as having higher complexity if there is inline-assembly, so it's best to avoid using it where it's not necessary
There is one instance of this issue:
File: src/governance/GovernorBravoDelegateMaia.sol 539: chainId := chainid()
Some files use >=
, some use ^
. The instances below are examples of the method that has the fewest instances for a specific version. Note that using >=
without also specifying <=
will lead to failures to compile, or external project incompatability, when the major version changes and there are breaking-changes, so ^
should be preferred regardless of the instance counts
There are 11 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-4626/ERC4626DepositOnly.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/ERC4626MultiToken.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/ERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/interfaces/IERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/UlyssesERC4626.sol 2: pragma solidity >=0.8.0;
File: src/talos/base/TalosBaseStrategy.sol 3: pragma solidity >=0.8.0;
File: src/talos/boost-aggregator/BoostAggregator.sol 2: pragma solidity >=0.8.0;
File: src/talos/interfaces/IBoostAggregator.sol 2: pragma solidity >=0.8.0;
File: src/talos/strategies/TalosStrategySimple.sol 2: pragma solidity >=0.8.0;
File: src/talos/TalosStrategyStaked.sol 2: pragma solidity >=0.8.0;
</details>File: src/talos/TalosStrategyVanilla.sol 3: pragma solidity >=0.8.0;
>
/>=
without specifying an upper bound is unsafeThere will be breaking changes in future versions of solidity, and at that point your code will no longer be compatable. While you may have the specific version to use in a configuration file, others that include your source files may not.
There are 11 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-4626/ERC4626DepositOnly.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/ERC4626MultiToken.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/ERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/interfaces/IERC4626.sol 2: pragma solidity >=0.8.0;
File: src/erc-4626/UlyssesERC4626.sol 2: pragma solidity >=0.8.0;
File: src/talos/base/TalosBaseStrategy.sol 3: pragma solidity >=0.8.0;
File: src/talos/boost-aggregator/BoostAggregator.sol 2: pragma solidity >=0.8.0;
File: src/talos/interfaces/IBoostAggregator.sol 2: pragma solidity >=0.8.0;
File: src/talos/strategies/TalosStrategySimple.sol 2: pragma solidity >=0.8.0;
File: src/talos/TalosStrategyStaked.sol 2: pragma solidity >=0.8.0;
</details>File: src/talos/TalosStrategyVanilla.sol 3: pragma solidity >=0.8.0;
@param
is listed multiple timesThere are 2 instances of this issue:
File: src/rewards/interfaces/IFlywheelCore.sol /// @audit user 67 /** 68 * @notice accrue rewards for a two users on a strategy 69 * @param strategy the strategy to accrue a user's rewards on 70 * @param user the first user to be accrued 71 * @param user the second user to be accrued 72 * @return the cumulative amount of rewards accrued to the first user (including prior) 73 * @return the cumulative amount of rewards accrued to the second user (including prior) 74 */ 75: function accrue(ERC20 strategy, address user, address secondUser) external returns (uint256, uint256);
File: src/ulysses-omnichain/interfaces/IBranchBridgeAgent.sol /// @audit depositor 313 /** 314 * @notice Internal function performs call to AnycallProxy Contract for cross-chain messaging. 315 * @param depositor address of user depositing assets. 316 * @param params calldata for omnichain execution. 317 * @param depositor address of user depositing assets. 318 * @param gasToBridgeOut gas allocated for the cross-chain call. 319 * @param remoteExecutionGas gas allocated for omnichain execution. 320 * @dev DEPOSIT ID: 1 (Call without Deposit) 321 * 322 */ 323: function performCallOut(address depositor, bytes memory params, uint128 gasToBridgeOut, uint128 remoteExecutionGas)
Use alternative variants, e.g. allowlist/denylist instead of whitelist/blacklist
There are 33 instances of this issue:
<details> <summary>see instances</summary>File: src/gauges/interfaces/IBaseV2Gauge.sol 58: /// @notice mapping of whitelisted bribe tokens.
File: src/governance/GovernorBravoDelegateMaia.sol 389: function isWhitelisted(address account) public view returns (bool) { 447: function _setWhitelistAccountExpiration(address account, uint256 expiration) external { 461: function _setWhitelistGuardian(address account) external { 113: // Allow addresses above proposal threshold and whitelisted addresses to propose 235: // Whitelisted proposers can't be canceled for falling below proposal threshold 385: * @notice View function which returns if an account is whitelisted 387: * @return If the account is whitelisted 443: * @notice Admin function for setting the whitelist expiration as a timestamp for an account. Whitelist status allows accounts to propose without meeting threshold 444: * @param account Account address to set whitelist expiration for 445: * @param expiration Expiration for account whitelist status as timestamp (if now < expiration, whitelisted) 458: * @notice Admin function for setting the whitelistGuardian. WhitelistGuardian can cancel proposals from whitelisted addresses 459: * @param account Account to set whitelistGuardian to (0x0 to remove whitelistGuardian)
File: src/governance/GovernorBravoInterfaces.sol 163: mapping(address => uint256) public whitelistAccountExpirations; 166: address public whitelistGuardian; 53: /// @notice Emitted when whitelist account expiration is set 56: /// @notice Emitted when the whitelistGuardian is set 162: /// @notice Stores the expiration of account whitelist status as a timestamp 165: /// @notice Address which manages whitelisted proposals and whitelist accounts
File: src/rewards/rewards/FlywheelBribeRewards.sol 38: /// @dev Anyone can call this, whitelisting is handled in FlywheelCore
File: src/talos/boost-aggregator/BoostAggregator.sol 48: mapping(address => bool) public whitelistedAddresses; 143: function addWhitelistedAddress(address user) external onlyOwner { 148: function removeWhitelistedAddress(address user) external onlyOwner { 78: /// whitelisted addresses to retrieve NFTs incorrectly sent to this contract 188: /// @dev Only whitelisted addresses
File: src/talos/interfaces/IBoostAggregator.sol 48: function whitelistedAddresses(address) external view returns (bool); 84: function addWhitelistedAddress(address user) external; 90: function removeWhitelistedAddress(address user) external; 47: /// @notice mapping of whitelisted addresses 81: * @notice add whitelisted address to stake using this contract 87: * @notice remove whitelisted address from staking using this contract
File: src/ulysses-omnichain/BranchPort.sol 56: /// @notice List of Tokens whitelisted for usage in Port Strategies.
</details>File: src/ulysses-omnichain/interfaces/IPortStrategy.sol 9: * whitelisted by the chain's Branch Port to manage a limited amount
Assembly blocks are take a lot more time to audit than normal Solidity code, and often have gotchas and side-effects that the Solidity versions of the same code do not. Consider adding more comments explaining what is being done in every step of the assembly code
There are 2 instances of this issue:
File: src/governance/GovernorBravoDelegator.sol 75 assembly { 76 let free_mem_ptr := mload(0x40) 77 returndatacopy(free_mem_ptr, 0, returndatasize()) 78 79 switch success 80 case 0 { revert(free_mem_ptr, returndatasize()) } 81 default { return(free_mem_ptr, returndatasize()) } 82: }
File: src/maia/libraries/DateTimeLib.sol 43 assembly { 44 epochDay := add(epochDay, 719468) 45 let doe := mod(epochDay, 146097) 46 let yoe := div(sub(sub(add(doe, div(doe, 36524)), div(doe, 1460)), eq(doe, 146096)), 365) 47 let doy := sub(doe, sub(add(mul(365, yoe), shr(2, yoe)), div(yoe, 100))) 48 let mp := div(add(mul(5, doy), 2), 153) 49 month := sub(add(mp, 3), mul(gt(mp, 9), 12)) 50: }
internal
There are 2 instances of this issue:
File: src/ulysses-omnichain/factories/ERC20hTokenBranchFactory.sol 16: address immutable localPortAddress; 19: address localCoreRouterAddress;
See the control structures section of the Solidity Style Guide
There are 13 instances of this issue:
<details> <summary>see instances</summary>File: src/hermes/bHermes.sol 143 if ( 144: userBalance - userClaimedWeight[msg.sender] < amount || userBalance - userClaimedBoost[msg.sender] < amount 161 if ( 162: userBalance - userClaimedWeight[from] < amount || userBalance - userClaimedBoost[from] < amount
File: src/maia/tokens/ERC4626PartnerManager.sol 328 if ( 329: userBalance - userClaimedWeight[from] < amount || userBalance - userClaimedBoost[from] < amount
File: src/talos/base/TalosBaseStrategy.sol 108 if (initialized) revert AlreadyInitialized(); 109 110: {
File: src/ulysses-omnichain/BranchBridgeAgentExecutor.sol 141 if ( 142: _data.length - PARAMS_GAS_OUT
File: src/ulysses-omnichain/BranchBridgeAgent.sol 813 if ( 814: _hTokens.length != _tokens.length || _tokens.length != _amounts.length
File: src/ulysses-omnichain/CoreRootRouter.sol 186 if ( 187: IPort(rootPortAddress).isGlobalAddress(_underlyingAddress)
File: src/ulysses-omnichain/RootBridgeAgentExecutor.sol 172 if ( 173: length - PARAMS_GAS_IN 277 if ( 278: _data.length - PARAMS_GAS_IN
File: src/ulysses-omnichain/RootBridgeAgent.sol 55 if ( 56: (_dParams.amount < _dParams.deposit) //Deposit can't be greater than amount. 262 } else if ( 263: msg.sender != depositOwner && msg.sender != address(IPort(localPortAddress).getUserAccount(depositOwner))
File: src/ulysses-omnichain/RootPort.sol 493 if ( 494: getUnderlyingTokenFromLocal[_ecoTokenGlobalAddress][localChainId] != address(0)
</details>File: src/uni-v3-staker/UniswapV3Staker.sol 374 if ((isNotRestake || block.timestamp < endTime) && owner != msg.sender) revert NotCalledByOwner(); 375 376: {
keccak256()
, should use immutable
rather than constant
While it doesn't save any gas because the compiler knows that developers often make this mistake, it's still best to use the right tool for the task at hand. There is a difference between constant
variables and immutable
variables, and they should each be used in their appropriate contexts. constants
should be used for literal values written into the code, and immutable
variables should be used for expressions, or values calculated in, or passed into the constructor.
There are 3 instances of this issue:
File: src/erc-20/ERC20MultiVotes.sol 360 bytes32 public constant DELEGATION_TYPEHASH = 361: keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
File: src/governance/GovernorBravoDelegateMaia.sol 42 bytes32 public constant DOMAIN_TYPEHASH = 43: keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); 46: bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used
There are 11 instances of this issue:
File: src/governance/GovernorBravoDelegateMaia.sol /// @audit 80640 17 /// @notice The minimum setable voting period 18: uint256 public constant MIN_VOTING_PERIOD = 80640; // About 2 weeks /// @audit 161280 20 /// @notice The max setable voting period 21: uint256 public constant MAX_VOTING_PERIOD = 161280; // About 4 weeks /// @audit 40320 23 /// @notice The min setable voting delay 24: uint256 public constant MIN_VOTING_DELAY = 40320; // About 1 weeks /// @audit 80640 26 /// @notice The max setable voting delay 27: uint256 public constant MAX_VOTING_DELAY = 80640; // About 2 weeks
File: src/hermes/minters/BaseV2Minter.sol /// @audit 86400 23 /// @dev allows minting once per week (reset every Thursday 00:00 UTC) 24: uint256 internal constant week = 86400 * 7; /// @audit 300 29: uint256 internal constant max_dao_share = 300;
File: src/maia/libraries/DateTimeLib.sol /// @audit 86400 35 /// @dev Returns (`month`) from the number of days since 1970-01-01. 36 /// See: https://howardhinnant.github.io/date_algorithms.html 37 /// Note: Inputs outside the supported ranges result in undefined behavior. 38 /// Use {isSupportedDays} to check if the inputs is supported. 39 function getMonth(uint256 timestamp) internal pure returns (uint256 month) { 40: uint256 epochDay = timestamp / 86400; /// @audit 86400 57: uint256 day = timestamp / 86400; /// @audit 86400 58: startOfDay = day * 86400;
File: src/ulysses-amm/UlyssesPool.sol /// @audit 4980e14 /// @audit 6000e14 69 /// @notice The current rebalancing fees 70: Fees public fees = Fees({lambda1: 20e14, lambda2: 4980e14, sigma1: 6000e14, sigma2: 500e14});
delete
rather than assigning zero/false to clear valuesThe delete
keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic
There are 19 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-20/ERC20Boost.sol 249: getUserBoost[msg.sender] = 0;
File: src/erc-20/ERC20MultiVotes.sol 340: _delegatesVotesCount[user][delegatee] = 0;
File: src/governance/GovernorBravoDelegateMaia.sol 149: newProposal.eta = 0; 156: newProposal.forVotes = 0; 157: newProposal.againstVotes = 0; 158: newProposal.abstainVotes = 0; 159: newProposal.canceled = false; 160: newProposal.executed = false;
File: src/rewards/base/FlywheelCore.sol 98: rewardsAccrued[user] = 0;
File: src/talos/libraries/PoolVariables.sol 211: secondsAgo[1] = 0;
File: src/ulysses-amm/UlyssesToken.sol 78: assetId[asset] = 0;
</details>File: src/uni-v3-staker/UniswapV3Staker.sol 202: incentive.totalRewardUnclaimed = 0; 268: rewards[msg.sender] = 0; 279: rewards[msg.sender] = 0; 406: _userAttachements[owner][key.pool] = 0; 426: deposit.stakedTimestamp = 0; 454: stake.secondsPerLiquidityInsideInitialX128 = 0; 455: stake.liquidityNoOverflow = 0; 456: if (liquidity >= type(uint96).max) stake.liquidityIfOverflow = 0;
While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.
There is one instance of this issue:
File: Various Files
internal
functions not called by the contract should be removedAll unused code should be removed
There is one instance of this issue:
File: src/ulysses-omnichain/BranchBridgeAgent.sol 1109: function _getContext() internal view returns (address from, uint256 fromChainId) {
#0 - c4-judge
2023-07-11T07:57:31Z
trust1995 marked the issue as grade-a
#1 - c4-sponsor
2023-07-12T21:50:26Z
0xBugsy marked the issue as sponsor confirmed
#2 - 0xLightt
2023-09-07T11:09:59Z
๐ Selected for report: Raihan
Also found by: 0x11singh99, 0xAnah, 0xSmartContract, 0xn006e7, Aymen0909, DavidGiladi, IllIllI, JCN, Jorgect, MohammedRizwan, Rageur, ReyAdmirado, Rickard, Rolezn, SAQ, SM3_SS, Sathish9098, TheSavageTeddy, hunter_w3b, kaveyjoe, lsaudit, matrix_0wl, naman1778, petrichor, shamsulhaq123, wahedtalash77
610.3258 USDC - $610.33
Issue | Instances | Total Gas Saved | |
---|---|---|---|
[Gโ01] | Consider using bytes32 rather than a string | 1 | - |
[Gโ02] | Remove or replace unused state variables | 2 | - |
[Gโ03] | State variables can be packed into fewer storage slots | 3 | 6000 |
[Gโ04] | Structs can be packed into fewer storage slots | 5 | 10000 |
[Gโ05] | Using storage instead of memory for structs/arrays saves gas | 2 | 8400 |
[Gโ06] | State variables should be cached in stack variables rather than re-reading them from storage | 94 | 9118 |
[Gโ07] | Multiple accesses of a mapping/array should use a local variable cache | 54 | 2268 |
Total: 161 instances over 7 issues with 35786 gas saved
Gas totals are estimates based on data from the Ethereum Yellowpaper. The estimates use the lower bounds of ranges and count two iterations of each for
-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.
The issues below may be reported by other bots/wardens, but can be ignored since either the rule or the specified instances are invalid
Issue | Instances | |
---|---|---|
[Iโ01] | constant variables, which will save gas | 3 |
[Iโ02] | 24 | |
[Iโ03] | abi.encode() is less efficient than abi.encodepacked() | 15 |
[Iโ04] | 8 | |
[Iโ05] | public function visibility to external to save gas | 5 |
[Iโ06] | 3 | |
[Iโ07] | 2 |
Total: 60 instances over 7 issues
bytes32
rather than a string
Using the bytes
types for fixed-length strings is more efficient than having the EVM have to incur the overhead of string processing. Consider whether the value needs to be a string
. A good reason to keep it as a string
would be if the variable is defined in an interface that this project does not own.
There is one instance of this issue:
File: src/governance/GovernorBravoDelegateMaia.sol 9: string public constant name = "vMaia Governor Bravo";
Saves a storage slot. If the variable is assigned a non-zero value, saves Gsset (20000 gas). If it's assigned a zero value, saves Gsreset (2900 gas). If the variable remains unassigned, there is no gas savings unless the variable is public
, in which case the compiler-generated non-payable getter deployment cost is saved. If the state variable is overriding an interface's public function, mark the variable as constant
or immutable
so that it does not use a storage slot
There are 2 instances of this issue:
File: src/ulysses-omnichain/factories/RootBridgeAgentFactory.sol 38: mapping(address => address) public getBridgeAgentManager;
File: src/ulysses-omnichain/token/ERC20hTokenRoot.sol 22: address public localBranchPortAddress;
If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper
There are 3 instances of this issue:
File: src/rewards/rewards/FlywheelGaugeRewards.sol /// @audit Variable ordering with 2 slots instead of the current 3: /// mapping(32):gaugeQueuedRewards, uint112(14):nextCycleQueuedRewards, uint32(4):gaugeCycle, uint32(4):nextCycle, uint32(4):paginationOffset 36: uint32 public override gaugeCycle;
File: src/talos/base/TalosBaseStrategy.sol /// @audit Variable ordering with 4 slots instead of the current 5: /// uint256(32):tokenId, uint256(32):protocolFees0, uint256(32):protocolFees1, uint128(16):liquidity, int24(3):tickLower, int24(3):tickUpper, bool(1):initialized 37: uint256 public override tokenId;
File: src/ulysses-omnichain/RootPort.sol /// @audit Variable ordering with 20 slots instead of the current 21: /// mapping(32):getUserAccount, mapping(32):isRouterApproved, mapping(32):isChainId, mapping(32):isBridgeAgent, address[](32):bridgeAgents, uint256(32):bridgeAgentsLenght, mapping(32):getBridgeAgentManager, mapping(32):isBridgeAgentFactory, address[](32):bridgeAgentFactories, uint256(32):bridgeAgentFactoriesLenght, mapping(32):isGlobalAddress, mapping(32):getGlobalTokenFromLocal, mapping(32):getLocalTokenFromGlobal, mapping(32):getLocalTokenFromUnder, mapping(32):getUnderlyingTokenFromLocal, mapping(32):getWrappedNativeToken, mapping(32):getGasPoolInfo, address(20):localBranchPortAddress, bool(1):_setup, address(20):coreRootRouterAddress, address(20):coreRootBridgeAgentAddress 34: address public localBranchPortAddress;
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 5 instances of this issue:
File: src/governance/GovernorBravoInterfaces.sol /// @audit Variable ordering with 13 slots instead of the current 14: /// uint256(32):id, uint256(32):eta, address[](32):targets, uint256[](32):values, string[](32):signatures, bytes[](32):calldatas, uint256(32):startBlock, uint256(32):endBlock, uint256(32):forVotes, uint256(32):againstVotes, uint256(32):abstainVotes, mapping(32):receipts, address(20):proposer, bool(1):canceled, bool(1):executed 105 struct Proposal { 106 /// @notice Unique id for looking up a proposal 107 uint256 id; 108 /// @notice Creator of the proposal 109 address proposer; 110 /// @notice The timestamp that the proposal will be available for execution, set once the vote succeeds 111 uint256 eta; 112 /// @notice the ordered list of target addresses for calls to be made 113 address[] targets; 114 /// @notice The ordered list of values (i.e. msg.value) to be passed to the calls to be made 115 uint256[] values; 116 /// @notice The ordered list of function signatures to be called 117 string[] signatures; 118 /// @notice The ordered list of calldata to be passed to each call 119 bytes[] calldatas; 120 /// @notice The block at which voting begins: holders must delegate their votes prior to this block 121 uint256 startBlock; 122 /// @notice The block at which voting ends: votes must be cast prior to this block 123 uint256 endBlock; 124 /// @notice Current number of votes in favor of this proposal 125 uint256 forVotes; 126 /// @notice Current number of votes in opposition to this proposal 127 uint256 againstVotes; 128 /// @notice Current number of votes for abstaining for this proposal 129 uint256 abstainVotes; 130 /// @notice Flag marking whether the proposal has been canceled 131 bool canceled; 132 /// @notice Flag marking whether the proposal has been executed 133 bool executed; 134 /// @notice Receipts of ballots for the entire set of voters 135 mapping(address => Receipt) receipts; 136: }
File: src/ulysses-omnichain/interfaces/IBranchBridgeAgent.sol /// @audit Variable ordering with 4 slots instead of the current 5: /// uint256(32):amount, uint256(32):deposit, address(20):hToken, uint24(3):toChain, address(20):token 26 struct DepositInput { 27 //Deposit Info 28 address hToken; //Input Local hTokens Address. 29 address token; //Input Native / underlying Token Address. 30 uint256 amount; //Amount of Local hTokens deposited for interaction. 31 uint256 deposit; //Amount of native tokens deposited for interaction. 32 uint24 toChain; //Destination chain for interaction. 33: } /// @audit Variable ordering with 5 slots instead of the current 6: /// address[](32):hTokens, address[](32):tokens, uint256[](32):amounts, uint256[](32):deposits, uint128(16):depositedGas, uint32(4):depositNonce, uint24(3):toChain, uint8(1):numberOfAssets 55 struct DepositMultipleParams { 56 //Deposit Info 57 uint8 numberOfAssets; //Number of assets to deposit. 58 uint32 depositNonce; //Deposit nonce. 59 address[] hTokens; //Input Local hTokens Address. 60 address[] tokens; //Input Native / underlying Token Address. 61 uint256[] amounts; //Amount of Local hTokens deposited for interaction. 62 uint256[] deposits; //Amount of native tokens deposited for interaction. 63 uint24 toChain; //Destination chain for interaction. 64 uint128 depositedGas; //BRanch chain gas token amount sent with request. 65: }
File: src/ulysses-omnichain/interfaces/IRootBridgeAgent.sol /// @audit Variable ordering with 4 slots instead of the current 5: /// uint256(32):amount, uint256(32):deposit, address(20):hToken, uint32(4):depositNonce, uint24(3):toChain, address(20):token 63 struct DepositParams { 64 //Deposit Info 65 uint32 depositNonce; //Deposit nonce. 66 address hToken; //Input Local hTokens Address. 67 address token; //Input Native / underlying Token Address. 68 uint256 amount; //Amount of Local hTokens deposited for interaction. 69 uint256 deposit; //Amount of native tokens deposited for interaction. 70 uint24 toChain; //Destination chain for interaction. 71: } /// @audit Variable ordering with 5 slots instead of the current 6: /// address[](32):hTokens, address[](32):tokens, uint256[](32):amounts, uint256[](32):deposits, uint32(4):depositNonce, uint24(3):toChain, uint8(1):numberOfAssets 73 struct DepositMultipleParams { 74 //Deposit Info 75 uint8 numberOfAssets; //Number of assets to deposit. 76 uint32 depositNonce; //Deposit nonce. 77 address[] hTokens; //Input Local hTokens Address. 78 address[] tokens; //Input Native / underlying Token Address. 79 uint256[] amounts; //Amount of Local hTokens deposited for interaction. 80 uint256[] deposits; //Amount of native tokens deposited for interaction. 81 uint24 toChain; //Destination chain for interaction. 82: }
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: src/rewards/rewards/FlywheelGaugeRewards.sol 179: QueuedRewards memory queuedRewards = gaugeQueuedRewards[gauge]; 204: QueuedRewards memory queuedRewards = gaugeQueuedRewards[ERC20(msg.sender)];
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces 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 94 instances of this issue:
<details> <summary>see instances</summary>File: src/gauges/BaseV2Gauge.sol /// @audit multiRewardsDepot on line 133 135: multiRewardsDepot.addAsset(flyWheelRewards, bribeFlywheel.rewardToken());
File: src/governance/GovernorBravoDelegateMaia.sol /// @audit whitelistGuardian on line 464 466: emit WhitelistGuardianSet(oldGuardian, whitelistGuardian);
File: src/governance/GovernorBravoDelegator.sol /// @audit implementation on line 48 50: emit NewImplementation(oldImplementation, implementation);
File: src/hermes/minters/BaseV2Minter.sol /// @audit dao on line 145 145: if (dao != address(0)) underlying.safeTransfer(dao, share);
File: src/maia/factories/PartnerManagerFactory.sol /// @audit partners on line 59 60: partners.push(newPartnerManager);
File: src/maia/PartnerUtilityManager.sol /// @audit partnerVault on line 75 76: IBaseVault(partnerVault).applyWeight(); /// @audit partnerVault on line 85 86: IBaseVault(partnerVault).applyBoost(); /// @audit partnerVault on line 95 96: IBaseVault(partnerVault).applyGovernance();
File: src/maia/tokens/ERC4626PartnerManager.sol /// @audit partnerVault on line 74 65: partnerVault
File: src/rewards/base/FlywheelCore.sol /// @audit flywheelRewards on line 126 128: rewardToken.safeTransferFrom(address(flywheelRewards), address(newFlywheelRewards), oldRewardBalance); /// @audit flywheelBooster on line 164 165: ? flywheelBooster.boostedTotalSupply(strategy) /// @audit flywheelBooster on line 196 197: ? flywheelBooster.boostedBalanceOf(strategy, user)
File: src/rewards/depots/MultiRewardsDepot.sol /// @audit _assets on line 60 62: delete _isAsset[_assets[rewardsContract]];
File: src/rewards/rewards/FlywheelGaugeRewards.sol /// @audit nextCycleQueuedRewards on line 134 137: uint112 queued = nextCycleQueuedRewards; /// @audit paginationOffset on line 122 125: uint32 offset = paginationOffset;
File: src/talos/base/TalosBaseStrategy.sol /// @audit tokenId on line 155 160: emit Initialize(tokenId, msg.sender, receiver, amount0, amount1, shares); /// @audit tokenId on line 190 203: tokenId: _tokenId, /// @audit tokenId on line 255 265: tokenId: _tokenId, /// @audit tokenId on line 265 283: tokenId: _tokenId, /// @audit tokenId on line 299 305: emit Rerange(tokenId, tickLower, tickUpper, amount0, amount1); /// @audit tokenId on line 305 307: afterRerange(tokenId); // tokenId changed in doRerange /// @audit tokenId on line 312 318: emit Rerange(tokenId, tickLower, tickUpper, amount0, amount1); /// @audit tokenId on line 318 320: afterRerange(tokenId); // tokenId changed in doRerange /// @audit tokenId on line 355 365: tokenId: _tokenId, /// @audit liquidity on line 261 266: liquidity: liquidityToDecrease, /// @audit liquidity on line 350 356: liquidity: _liquidity, /// @audit tickLower on line 120 /// @audit tickLower on line 140 140: tickLower: tickLower, /// @audit tickUpper on line 121 /// @audit tickUpper on line 141 141: tickUpper: tickUpper,
File: src/ulysses-amm/UlyssesPool.sol /// @audit bandwidthStateList on line 169 176: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit bandwidthStateList on line 249 253: for (uint256 i = 1; i < bandwidthStateList.length;) { /// @audit bandwidthStateList on line 260 278: for (uint256 i = 1; i < bandwidthStateList.length;) { /// @audit bandwidthStateList on line 278 280: if (i == bandwidthStateList.length - 1) { /// @audit bandwidthStateList on line 284 296: for (uint256 i = 1; i < bandwidthStateList.length; i++) { /// @audit totalWeights on line 176 181: uint256 oldTotalWeights = totalWeights; /// @audit totalWeights on line 183 212: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit totalWeights on line 233 238: uint256 oldTotalWeights = totalWeights; /// @audit totalWeights on line 241 243: if (totalWeights > MAX_TOTAL_WEIGHT || oldTotalWeights == newTotalWeights) { /// @audit totalWeights on line 243 297: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit bandwidthStateList[i].bandwidth on line 133 135: assets += bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i].weight on line 176 212: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit bandwidthStateList[i].bandwidth on line 178 190: uint256 oldBandwidth = bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i].bandwidth on line 192 194: newBandwidth += oldBandwidth - bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i].bandwidth on line 194 214: newRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false); /// @audit bandwidthStateList[i].weight on line 233 284: leftOverBandwidth.mulDiv(bandwidthStateList[i].weight, weightsWithoutPool).toUint248(); /// @audit bandwidthStateList[i].weight on line 284 297: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit bandwidthStateList[i].bandwidth on line 283 299: newRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false);
File: src/ulysses-amm/UlyssesToken.sol /// @audit assets on line 89 95: for (uint256 i = 0; i < assets.length; i++) {
File: src/ulysses-omnichain/ArbitrumBranchPort.sol /// @audit localChainId on line 62 66: address underlyingAddress = IRootPort(rootPortAddress).getUnderlyingTokenFromLocal(_globalAddress, localChainId); /// @audit rootPortAddress on line 49 54: IRootPort(rootPortAddress).mintToLocalBranch(_recipient, globalToken, _deposit); /// @audit rootPortAddress on line 62 66: address underlyingAddress = IRootPort(rootPortAddress).getUnderlyingTokenFromLocal(_globalAddress, localChainId); /// @audit rootPortAddress on line 66 70: IRootPort(rootPortAddress).burnFromLocalBranch(_depositor, _globalAddress, _deposit);
File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol /// @audit localPortAddress on line 83 93: if (!IPort(localPortAddress).isBridgeAgent(newBridgeAgent)) {
File: src/ulysses-omnichain/BaseBranchRouter.sol /// @audit localBridgeAgentAddress on line 39 40: bridgeAgentExecutorAddress = IBridgeAgent(localBridgeAgentAddress).bridgeAgentExecutorAddress();
File: src/ulysses-omnichain/BranchBridgeAgent.sol /// @audit bridgeAgentExecutorAddress on line 1154 1177: try BranchBridgeAgentExecutor(bridgeAgentExecutorAddress).executeWithSettlement( /// @audit bridgeAgentExecutorAddress on line 1177 1201: try BranchBridgeAgentExecutor(bridgeAgentExecutorAddress).executeWithSettlementMultiple( /// @audit getDeposit[_depositNonce].hTokens on line 338 353: getDeposit[_depositNonce].hTokens[0], /// @audit getDeposit[_depositNonce].hTokens on line 353 365: } else if (uint8(getDeposit[_depositNonce].hTokens.length) > 1) { /// @audit getDeposit[_depositNonce].hTokens on line 365 373: uint8(getDeposit[_depositNonce].hTokens.length), /// @audit getDeposit[_depositNonce].tokens on line 339 342: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() /// @audit getDeposit[_depositNonce].tokens on line 342 354: getDeposit[_depositNonce].tokens[0], /// @audit getDeposit[_depositNonce].tokens on line 354 357: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() /// @audit getDeposit[_depositNonce].amounts on line 340 355: getDeposit[_depositNonce].amounts[0], /// @audit getDeposit[_depositNonce].deposits on line 342 357: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() /// @audit getDeposit[nonce].hTokens on line 375 387: uint8(getDeposit[nonce].hTokens.length), /// @audit getDeposit[nonce].hTokens on line 387 389: getDeposit[nonce].hTokens, /// @audit getDeposit[nonce].tokens on line 376 378: _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens), /// @audit getDeposit[nonce].tokens on line 378 390: getDeposit[nonce].tokens, /// @audit getDeposit[nonce].tokens on line 390 392: _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens), /// @audit getDeposit[nonce].amounts on line 377 391: getDeposit[nonce].amounts, /// @audit getDeposit[nonce].deposits on line 378 392: _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens),
File: src/ulysses-omnichain/CoreBranchRouter.sol /// @audit localPortAddress on line 133 143: if (!IPort(localPortAddress).isBridgeAgent(newBridgeAgent)) { /// @audit localPortAddress on line 164 165: IPort(localPortAddress).addBridgeAgentFactory(_newBridgeAgentFactoryAddress); /// @audit localPortAddress on line 165 167: IPort(localPortAddress).toggleBridgeAgentFactory(_newBridgeAgentFactoryAddress); /// @audit localPortAddress on line 178 179: IPort(localPortAddress).toggleBridgeAgent(_branchBridgeAgent); /// @audit localPortAddress on line 190 191: IPort(localPortAddress).addStrategyToken(_underlyingToken, _minimumReservesRatio); /// @audit localPortAddress on line 191 193: IPort(localPortAddress).toggleStrategyToken(_underlyingToken); /// @audit localPortAddress on line 212 214: IPort(localPortAddress).addPortStrategy(_portStrategy, _underlyingToken, _dailyManagementLimit); /// @audit localPortAddress on line 214 217: IPort(localPortAddress).updatePortStrategy(_portStrategy, _underlyingToken, _dailyManagementLimit); /// @audit localPortAddress on line 217 220: IPort(localPortAddress).togglePortStrategy(_portStrategy, _underlyingToken);
File: src/ulysses-omnichain/MulticallRootRouter.sol /// @audit bridgeAgentAddress on line 120 123: IBridgeAgent(bridgeAgentAddress).callOutAndBridge{value: msg.value}( /// @audit bridgeAgentAddress on line 148 155: IBridgeAgent(bridgeAgentAddress).callOutAndBridgeMultiple{value: msg.value}(
File: src/ulysses-omnichain/RootBridgeAgent.sol /// @audit initialGas on line 875 1161: if (initialGas > 0) { /// @audit initialGas on line 1161 1171: if (initialGas > 0) { /// @audit userFeeInfo on line 247 248: userFeeInfo.gasToBridgeOut = _remoteExecutionGas; /// @audit userFeeInfo on line 751 754: return uint128(userFeeInfo.gasToBridgeOut); /// @audit userFeeInfo on line 754 758: if (userFeeInfo.gasToBridgeOut <= MIN_FALLBACK_RESERVE * tx.gasprice) revert InsufficientGasForFees(); /// @audit userFeeInfo on line 758 759: (amountOut, gasToken) = _gasSwapOut(userFeeInfo.gasToBridgeOut, _toChain); /// @audit userFeeInfo on line 908 /// @audit userFeeInfo on line 1162 1162: _payExecutionGas(userFeeInfo.depositedGas, userFeeInfo.gasToBridgeOut, _initialGas, fromChainId); /// @audit userFeeInfo on line 1162 /// @audit userFeeInfo on line 1172 1172: _payExecutionGas(userFeeInfo.depositedGas, userFeeInfo.gasToBridgeOut, _initialGas, fromChainId);
</details>File: src/ulysses-omnichain/RootPort.sol /// @audit coreRootRouterAddress on line 435 442: IBridgeAgent(ICoreRootRouter(coreRootRouterAddress).bridgeAgentAddress()).syncBranchBridgeAgent(
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 54 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-20/ERC20Boost.sol /// @audit _userGauges[<etc>] on line 204 213: require(_userGauges[msg.sender].remove(gauge)); // Remove from set. Should never fail. /// @audit _userGauges[<etc>] on line 232 239: require(_userGauges[msg.sender].remove(gauge)); // Remove from set. Should never fail.
File: src/erc-20/ERC20Gauges.sol /// @audit _userGauges[user] on line 210 211: if (added && _userGauges[user].length() > maxGauges && !canContractExceedMaxGauges[user]) {
File: src/erc-20/ERC20MultiVotes.sol /// @audit _delegates[user] on line 324 339: require(_delegates[user].remove(delegatee)); // Remove from set. Should never fail.
File: src/ulysses-amm/factories/UlyssesFactory.sol /// @audit pools[<etc>] on line 111 124: pools[poolIds[i]].transferOwnership(owner);
File: src/ulysses-amm/UlyssesPool.sol /// @audit bandwidthStateList[i] on line 131 133: assets += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false); /// @audit bandwidthStateList[i] on line 133 135: assets += bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i] on line 176 178: oldRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false); /// @audit bandwidthStateList[i] on line 178 190: uint256 oldBandwidth = bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i] on line 190 192: bandwidthStateList[i].bandwidth = oldBandwidth.mulDivUp(oldTotalWeights, newTotalWeights).toUint248(); /// @audit bandwidthStateList[i] on line 192 194: newBandwidth += oldBandwidth - bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i] on line 194 212: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit bandwidthStateList[i] on line 212 214: newRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false); /// @audit bandwidthStateList[i] on line 233 235: oldRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false); /// @audit bandwidthStateList[i] on line 235 255: uint256 oldBandwidth = bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i] on line 255 257: bandwidthStateList[i].bandwidth = /// @audit bandwidthStateList[i] on line 257 260: leftOverBandwidth += oldBandwidth - bandwidthStateList[i].bandwidth; /// @audit bandwidthStateList[i] on line 260 281: bandwidthStateList[i].bandwidth += leftOverBandwidth.toUint248(); /// @audit bandwidthStateList[i] on line 281 283: bandwidthStateList[i].bandwidth += /// @audit bandwidthStateList[i] on line 283 284: leftOverBandwidth.mulDiv(bandwidthStateList[i].weight, weightsWithoutPool).toUint248(); /// @audit bandwidthStateList[i] on line 284 297: uint256 targetBandwidth = totalSupply.mulDiv(bandwidthStateList[i].weight, totalWeights); /// @audit bandwidthStateList[i] on line 297 299: newRebalancingFee += _calculateRebalancingFee(bandwidthStateList[i].bandwidth, targetBandwidth, false);
File: src/ulysses-amm/UlyssesToken.sol /// @audit assets[i] on line 112 116: assets[i].safeTransfer(msg.sender, assetBalance - newAssetBalance); /// @audit assets[i] on line 116 118: assets[i].safeTransferFrom(msg.sender, address(this), newAssetBalance - assetBalance);
File: src/ulysses-omnichain/BranchBridgeAgent.sol /// @audit getDeposit[_depositNonce] on line 327 332: if (uint8(getDeposit[_depositNonce].hTokens.length) == 1) { /// @audit getDeposit[_depositNonce] on line 332 338: getDeposit[_depositNonce].hTokens[0], /// @audit getDeposit[_depositNonce] on line 338 339: getDeposit[_depositNonce].tokens[0], /// @audit getDeposit[_depositNonce] on line 339 340: getDeposit[_depositNonce].amounts[0], /// @audit getDeposit[_depositNonce] on line 340 /// @audit getDeposit[_depositNonce] on line 342 342: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() /// @audit getDeposit[_depositNonce] on line 342 353: getDeposit[_depositNonce].hTokens[0], /// @audit getDeposit[_depositNonce] on line 353 354: getDeposit[_depositNonce].tokens[0], /// @audit getDeposit[_depositNonce] on line 354 355: getDeposit[_depositNonce].amounts[0], /// @audit getDeposit[_depositNonce] on line 355 /// @audit getDeposit[_depositNonce] on line 357 357: getDeposit[_depositNonce].deposits[0], ERC20(getDeposit[_depositNonce].tokens[0]).decimals() /// @audit getDeposit[_depositNonce] on line 357 365: } else if (uint8(getDeposit[_depositNonce].hTokens.length) > 1) { /// @audit getDeposit[_depositNonce] on line 365 373: uint8(getDeposit[_depositNonce].hTokens.length), /// @audit getDeposit[nonce] on line 375 376: getDeposit[nonce].tokens, /// @audit getDeposit[nonce] on line 376 377: getDeposit[nonce].amounts, /// @audit getDeposit[nonce] on line 377 /// @audit getDeposit[nonce] on line 378 378: _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens), /// @audit getDeposit[nonce] on line 378 387: uint8(getDeposit[nonce].hTokens.length), /// @audit getDeposit[nonce] on line 387 389: getDeposit[nonce].hTokens, /// @audit getDeposit[nonce] on line 389 390: getDeposit[nonce].tokens, /// @audit getDeposit[nonce] on line 390 391: getDeposit[nonce].amounts, /// @audit getDeposit[nonce] on line 391 /// @audit getDeposit[nonce] on line 392 392: _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens), /// @audit getDeposit[_depositNonce] on line 373 408: getDeposit[_depositNonce].status = DepositStatus.Success; /// @audit getDeposit[_depositNonce] on line 408 411: getDeposit[_depositNonce].depositedGas = msg.value.toUint128(); /// @audit getDeposit[_depositNonce] on line 1069 1075: getDeposit[_depositNonce].depositedGas -= minExecCost.toUint128();
File: src/ulysses-omnichain/RootBridgeAgent.sol /// @audit getSettlement[_depositNonce] on line 257 260: if (getSettlement[_depositNonce].status != SettlementStatus.Failed || depositOwner == address(0)) { /// @audit getSettlement[_settlementNonce] on line 839 845: getSettlement[_settlementNonce].gasToBridgeOut -= minExecCost.toUint128();
</details>File: src/uni-v3-staker/UniswapV3Staker.sol /// @audit deposits[tokenId] on line 491 501: deposits[tokenId].stakedTimestamp = uint40(block.timestamp); /// @audit incentives[incentiveId] on line 483 502: incentives[incentiveId].numberOfStakes++;
The issues below may be reported by other bots/wardens, but can be ignored since either the rule or the specified instances are invalid
constant
variables, which will save gasThe compiler automatically expands simple expressions into their literal values, so manually doing the expansion doesn't save gas, and this finding is invalid
There are 3 instances of this issue:
File: src/erc-20/ERC20MultiVotes.sol 360 bytes32 public constant DELEGATION_TYPEHASH = 361: keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
File: src/governance/GovernorBravoDelegateMaia.sol 42 bytes32 public constant DOMAIN_TYPEHASH = 43: keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)"); 46: bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
Using delete instead of assigning zero to state variables does not save any extra gas - they both refund the same amount, so this finding is invalid
There are 24 instances of this issue:
<details> <summary>see instances</summary>File: src/erc-20/ERC20Boost.sol 249: getUserBoost[msg.sender] = 0;
File: src/erc-20/ERC20MultiVotes.sol 340: _delegatesVotesCount[user][delegatee] = 0;
File: src/governance/GovernorBravoDelegateMaia.sol 149: newProposal.eta = 0; 156: newProposal.forVotes = 0; 157: newProposal.againstVotes = 0; 158: newProposal.abstainVotes = 0;
File: src/hermes/minters/BaseV2Minter.sol 165: weekly = 0;
File: src/rewards/base/FlywheelCore.sol 98: rewardsAccrued[user] = 0;
File: src/rewards/rewards/FlywheelGaugeRewards.sol 100: nextCycleQueuedRewards = 0; 101: paginationOffset = 0; 122: paginationOffset = 0; 145: nextCycleQueuedRewards = 0; 146: paginationOffset = 0;
File: src/talos/base/TalosBaseStrategy.sol 362: liquidity = 0;
File: src/talos/libraries/PoolVariables.sol 211: secondsAgo[1] = 0;
File: src/ulysses-amm/UlyssesToken.sol 78: assetId[asset] = 0;
</details>File: src/uni-v3-staker/UniswapV3Staker.sol 202: incentive.totalRewardUnclaimed = 0; 268: rewards[msg.sender] = 0; 279: rewards[msg.sender] = 0; 406: _userAttachements[owner][key.pool] = 0; 426: deposit.stakedTimestamp = 0; 454: stake.secondsPerLiquidityInsideInitialX128 = 0; 455: stake.liquidityNoOverflow = 0; 456: if (liquidity >= type(uint96).max) stake.liquidityIfOverflow = 0;
abi.encode()
is less efficient than abi.encodepacked()
abi.encodePacked()
does not always save gas over abi.encode()
and in fact often costs more gas. The comparison sometimes linked to itself even shows that when addresses are involved, the packed flavor costs more gas. Furthermore, the test was done on Solidity 0.4.24, and there have been a lot of changes since then. Without more specific tests or explanations, it's not correct to say that the change always saves gas.
There are 15 instances of this issue:
File: src/erc-20/ERC20MultiVotes.sol 368: "\x19\x01", DOMAIN_SEPARATOR(), keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry))
File: src/governance/GovernorBravoDelegateMaia.sol 198: !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 346: keccak256(abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), getChainIdInternal(), address(this)));
File: src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol 53: bytes memory data = abi.encode(_underlyingAddress, address(0), name, symbol); 98: bytes memory data = abi.encode(newBridgeAgent, _rootBridgeAgent);
File: src/ulysses-omnichain/CoreBranchRouter.sol 48: bytes memory data = abi.encode(msg.sender, _globalAddress, _toChain, _rootExecutionGas); 72: bytes memory data = abi.encode(_underlyingAddress, newToken, name, symbol); 105: bytes memory data = abi.encode(_globalAddress, newToken); 148: bytes memory data = abi.encode(newBridgeAgent, _rootBridgeAgent);
File: src/ulysses-omnichain/CoreRootRouter.sol 107: bytes memory data = abi.encode( 158: bytes memory data = abi.encode( 238: bytes memory data = abi.encode(_branchBridgeAgentFactory); 259: bytes memory data = abi.encode(_branchBridgeAgent); 282: bytes memory data = abi.encode(_underlyingToken, _minimumReservesRatio); 309: bytes memory data = abi.encode(_portStrategy, _underlyingToken, _dailyManagementLimit, _isUpdateDailyLimit);
The rule is true only when the contract being defined is upgradeable, which isn't the case for these invalid examples
There are 8 instances of this issue:
<details> <summary>see instances</summary>File: src/talos/base/TalosBaseStrategy.sol 13: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
File: src/talos/boost-aggregator/BoostAggregator.sol 9: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
File: src/talos/interfaces/IBoostAggregator.sol 6: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
File: src/talos/interfaces/ITalosBaseStrategy.sol 7: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
File: src/ulysses-omnichain/VirtualAccount.sol 9: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
File: src/ulysses-omnichain/interfaces/IVirtualAccount.sol 4: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
File: src/uni-v3-staker/UniswapV3Staker.sol 12: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
</details>File: src/uni-v3-staker/interfaces/IUniswapV3Staker.sol 5: import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
public
function visibility to external
to save gasBoth public
and external
functions use the same amount of gas (both deployment and runtime gas), so this finding is invalid
There are 5 instances of this issue:
File: src/erc-20/ERC20Boost.sol 175: function decrementGaugeBoost(address gauge, uint256 boost) public {
File: src/erc-20/ERC20MultiVotes.sol 131: function delegates(address delegator) public view returns (address[] memory) { 363: function delegateBySig(address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s) public {
File: src/governance/GovernorBravoDelegateMaia.sol 104 function propose( 105 address[] memory targets, 106 uint256[] memory values, 107 string[] memory signatures, 108 bytes[] memory calldatas, 109 string memory description 110: ) public returns (uint256) {
File: src/rewards/base/FlywheelCore.sol 84: function accrue(ERC20 strategy, address user, address secondUser) public returns (uint256, uint256) {
safeTransferETH()
does not call functions on a contract and instead directly sends funds to the contract, so contract existence checks are not needed for these library calls.
There are 3 instances of this issue:
File: src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol 165: SafeTransferLib.safeTransferETH(_recipient, gasRemaining);
File: src/ulysses-omnichain/BranchBridgeAgent.sol 1044: SafeTransferLib.safeTransferETH(_recipient, gasRemaining - minExecCost);
File: src/ulysses-omnichain/RootBridgeAgent.sol 1263: SafeTransferLib.safeTransferETH(daoAddress, _accumulatedFees);
Importing whole files rather than specific identifiers does not waste gas, so this finding is invalid
There are 2 instances of this issue:
File: src/governance/GovernorBravoDelegateMaia.sol 4: import "./GovernorBravoInterfaces.sol";
File: src/governance/GovernorBravoDelegator.sol 4: import "./GovernorBravoInterfaces.sol";
#0 - c4-judge
2023-07-11T08:00:51Z
trust1995 marked the issue as grade-a
#1 - c4-sponsor
2023-07-12T18:26:46Z
0xBugsy marked the issue as sponsor confirmed
#2 - 0xLightt
2023-09-07T11:28:23Z