Badger-Vested-Aura contest - MiloTruck's results

Bringing BTC to DeFi

General Information

Platform: Code4rena

Start Date: 15/06/2022

Pot Size: $30,000 USDC

Total HM: 5

Participants: 55

Period: 3 days

Judge: Jack the Pug

Id: 138

League: ETH

BadgerDAO

Findings Distribution

Researcher Performance

Rank: 52/55

Findings: 1

Award: $31.91

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Awards

31.9127 USDC - $31.91

Labels

bug
G (Gas Optimization)
sponsor acknowledged
valid

External Links

Gas Optimizations Report

For-loops: Index initialized with default value

Uninitialized uint variables are assigned with a default value of 0.

Thus, in for-loops, explicitly initializing an index with 0 costs unnecesary gas. For example, the following code:

for (uint256 i = 0; i < length; ++i) {

can be changed to:

for (uint256 i; i < length; ++i) {

Consider declaring the following lines without explicitly setting the index to 0:

contracts/MyStrategy.sol:
 118:        for(uint i = 0; i < length; i++){
 300:        for (uint256 i = 0; i < _claims.length; i++) {
 317:        for (uint256 i = 0; i < _claims.length; i++) {

For-Loops: Cache array length outside of loops

Reading an array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

For example:

for (uint256 i; i < arr.length; ++i) {}

can be changed to:

uint256 len = arr.length;
for (uint256 i; i < len; ++i) {}

Consider making the following change to these lines:

contracts/MyStrategy.sol:
 300:        for (uint256 i = 0; i < _claims.length; i++) {

Arithmetics: ++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2  

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable  

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2, thus it costs more gas.

The same logic applies for --i and i--.

Consider using ++i instead of i++ or i += 1 in the following instances:

contracts/MyStrategy.sol:
 118:        for(uint i = 0; i < length; i++){
 300:        for (uint256 i = 0; i < _claims.length; i++) {
 317:        for (uint256 i = 0; i < _claims.length; i++) {

Arithmetics: Use != 0 instead of > 0 for unsigned integers

uint will never go below 0. Thus, > 0 is gas inefficient in comparisons as checking if != 0 is sufficient and costs less gas.

Consider changing > 0 to != 0 in these lines:

contracts/MyStrategy.sol:
 230:        if (auraBalEarned > 0) {
 279:        if (harvested[0].amount > 0) {
 323:        if (difference > 0) {
 330:        if (difference > 0) {
 387:        upkeepNeeded = unlockable > 0;

Visibility: Consider declaring constants as non-public to save gas

If a constant is not used outside of its contract, declaring it as private or internal instead of public can save gas.

Consider changing the visibility of the following from public to internal or private:

contracts/MyStrategy.sol:
  32:        IBalancerVault public constant BALANCER_VAULT = IBalancerVault(0xBA12222222228d8Ba445958a75a0704d566BF2C8);
  34:        address public constant BADGER = 0x3472A5A71965499acd81997a54BBA8D852C6E53d;
  35:        address public constant BADGER_TREE = 0x660802Fc641b154aBA66a62137e71f331B6d787A;
  37:        IAuraLocker public constant LOCKER = IAuraLocker(0x3Fa73f1E5d8A792C80F426fc8F84FBF7Ce9bBCAC);
  39:        IERC20Upgradeable public constant BAL = IERC20Upgradeable(0xba100000625a3754423978a60c9317c58a424e3D);
  40:        IERC20Upgradeable public constant WETH = IERC20Upgradeable(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
  41:        IERC20Upgradeable public constant AURA = IERC20Upgradeable(0xC0c293ce456fF0ED870ADd98a0828Dd4d2903DBF);
  42:        IERC20Upgradeable public constant AURABAL = IERC20Upgradeable(0x616e8BfA43F920657B3497DBf40D6b1A02D4608d);
  43:        IERC20Upgradeable public constant BALETH_BPT = IERC20Upgradeable(0x5c6Ee304399DBdB9C8Ef030aB642B10820DB8F56);
  45:        bytes32 public constant AURABAL_BALETH_BPT_POOL_ID = 0x3dd0843a028c86e0b760b1a76929d1c5ef93a2dd000200000000000000000249;
  46:        bytes32 public constant BAL_ETH_POOL_ID = 0x5c6ee304399dbdb9c8ef030ab642b10820db8f56000200000000000000000014;
  47:        bytes32 public constant AURA_ETH_POOL_ID = 0xc29562b045d80fd77c69bec09541f5c16fe20d9d000200000000000000000251;

Errors: Reduce the length of error messages (long revert strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

In these instances, consider shortening the revert strings to fit within 32 bytes:

contracts/MyStrategy.sol:
 184:        require(
 185:            balanceOfPool() == 0 && LOCKER.balanceOf(address(this)) == 0,
 186:            "You have to wait for unlock or have to manually rebalance out of it"
 187:        );

Variables declared as constant are expressions, not constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

If the variable was immutable instead: the calculation would only be done once at deploy time (in the constructor), and then the result would be saved and read directly at runtime rather than being recalculated.

See: ethereum/solidity#9232:

Consequences: each usage of a β€œconstant” costs ~100 gas more on each access (it is still a little better than storing the result in storage, but not much). since these are not real constants, they can’t be referenced from a real constant environment (e.g. from assembly, or from another library)

contracts/MyStrategy.sol:
  32:        IBalancerVault public constant BALANCER_VAULT = IBalancerVault(0xBA12222222228d8Ba445958a75a0704d566BF2C8);
  37:        IAuraLocker public constant LOCKER = IAuraLocker(0x3Fa73f1E5d8A792C80F426fc8F84FBF7Ce9bBCAC);
  39:        IERC20Upgradeable public constant BAL = IERC20Upgradeable(0xba100000625a3754423978a60c9317c58a424e3D);
  40:        IERC20Upgradeable public constant WETH = IERC20Upgradeable(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
  41:        IERC20Upgradeable public constant AURA = IERC20Upgradeable(0xC0c293ce456fF0ED870ADd98a0828Dd4d2903DBF);
  42:        IERC20Upgradeable public constant AURABAL = IERC20Upgradeable(0x616e8BfA43F920657B3497DBf40D6b1A02D4608d);
  43:        IERC20Upgradeable public constant BALETH_BPT = IERC20Upgradeable(0x5c6Ee304399DBdB9C8Ef030aB642B10820DB8F56);

Change these expressions from constant to immutable and implement the calculation in the constructor. Alternatively, hardcode these values in the constants and add a comment to say how the value was calculated.

#0 - GalloDaSballo

2022-06-19T01:40:04Z

Variables declared as constant are expressions, not constants

False, Tom was wrong!!!

Rest I ack

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter