Ethena Labs - K42's results

Enabling The Internet Bond

General Information

Platform: Code4rena

Start Date: 24/10/2023

Pot Size: $36,500 USDC

Total HM: 4

Participants: 147

Period: 6 days

Judge: 0xDjango

Id: 299

League: ETH

Ethena Labs

Findings Distribution

Researcher Performance

Rank: 70/147

Findings: 2

Award: $20.70

Gas:
grade-b
Analysis:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

6.4563 USDC - $6.46

Labels

bug
G (Gas Optimization)
grade-b
sufficient quality report
G-36

External Links

Gas Optimization Report for Ethena-Labs by K42

Possible Optimizations in EthenaMinting.sol

Possible Optimization 1 =

After Optimization:

// Before
bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");

// After
uint256 private constant MINTER_ROLE = 1 << 1;
  • Estimated Gas Saved = This would reduce the gas cost for SSTORE and SLOAD operations related to role management. The bitwise operation uses fewer opcodes compared to keccak256.
  • Safety = This is safe as long as the bitwise constants are managed carefully to avoid overlap.

Possible Optimization 2 =

  • The contract has multiple revert conditions in functions like mint() and redeem(). Using short-circuiting can save gas when the first condition is not met.

After Optimization:

// Before
if (order.order_type != OrderType.MINT) revert InvalidOrder();
verifyOrder(order, signature);

// After
if (order.order_type != OrderType.MINT || !verifyOrder(order, signature)) revert InvalidOrder();
  • Estimated Gas Saved = Minimal but every bit counts. Short-circuiting avoids the execution of subsequent conditions if the first one is not met. Fewer opcodes due to reduced conditional checks.
  • Safety = This is safe as long as the conditions do not have side effects that are expected to be executed.

Possible Optimization 3 =

Here is the optimized code snippet:

// Before
uint256 totalTransferred = 0;
for (uint256 i = 0; i < addresses.length; ++i) {
    uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
    // ...
}

// After
for (uint256 i = 0; i < addresses.length; ++i) {
    uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
    uint256 totalTransferred = 0;
    // ...
}
  • Estimated gas saved = Minimal but improves readability and stack management. This is safe and improves the maintainability of the contract.

Possible Optimizations in StakedUSDe.sol

Possible Optimization 1 =

  • This contract uses custom modifiers notZero and notOwner for simple checks. Using require directly in the function can be more gas-efficient.

After Optimization:

// Before
modifier notZero(uint256 amount) {
  if (amount == 0) revert InvalidAmount();
  _;
}

// After
// Use require directly in the function
require(amount != 0, "InvalidAmount");
  • Estimated Gas Saved = Minimal but every bit counts. Removes the overhead of an extra function call.
  • Safety = This is safe as long as the conditions are checked properly.

Possible Optimization 2 =

  • Use calldata for function parameters that are not modified within the function to save gas.

After Optimization:

// Before
function addToBlacklist(address target, bool isFullBlacklisting) external ...

// After
function addToBlacklist(address calldata target, bool isFullBlacklisting) external ...
  • Estimated Gas Saved = Minimal but every bit counts. calldata is cheaper than memory or storage.
  • Safety = This is safe as long as the parameters are not modified within the function.

Possible Optimization 3 =

Optimized Code Snippet:

// Before
bytes32 private constant FULL_RESTRICTED_STAKER_ROLE = keccak256("FULL_RESTRICTED_STAKER_ROLE");
bytes32 private constant SOFT_RESTRICTED_STAKER_ROLE = keccak256("SOFT_RESTRICTED_STAKER_ROLE");

// After
uint256 private constant FULL_RESTRICTED_STAKER_ROLE = 1 << 1;
uint256 private constant SOFT_RESTRICTED_STAKER_ROLE = 1 << 2;
  • Estimated Gas Saved = This would reduce the gas cost for SSTORE and SLOAD operations related to role management.
  • Safety = This is safe as long as the bitwise constants are managed carefully to avoid overlap.
  • Extra Context = The bot report list mentions the use of bitwise operations and assembly for various optimizations, however it doesn't specifically address the concept of using bitwise operations for role management. Therefore this is unique in relation to the bot report.

Possible Optimizations in StakedUSDeV2.sol

Possible Optimization 1 =

  • Instead of having two separate functions (cooldownAssets() and cooldownShares()) that essentially do the same thing but with different parameters, use function overloading to reduce code duplication.

After Optimization:

function cooldown(uint256 amount, address owner, bool isAsset) external ensureCooldownOn returns (uint256) {
    uint256 limit = isAsset ? maxWithdraw(owner) : maxRedeem(owner);

    if (amount > limit) revert ExcessiveAmount();

    uint256 otherAmount = isAsset ? previewWithdraw(amount) : previewRedeem(amount);
    cooldowns[owner].cooldownEnd = uint104(block.timestamp) + cooldownDuration;
    cooldowns[owner].underlyingAmount += amount;
    _withdraw(_msgSender(), address(silo), owner, amount, otherAmount);

    return otherAmount;
}
  • Estimated Gas Saved = This would save gas on contract deployment due to reduced bytecode size.
  • Safety = This is safe as long as the function is called correctly with the isAsset parameter to distinguish between assets and shares.

Possible Optimization 2 =

  • Instead of using revert statements, emit events for errors. This can save gas because revert consumes all remaining gas, while emitting an event will not.

After Optimization:

event Error(string reason);

modifier ensureCooldownOff() {
    if (cooldownDuration != 0) emit Error("OperationNotAllowed");
    else _;
}

modifier ensureCooldownOn() {
    if (cooldownDuration == 0) emit Error("OperationNotAllowed");
    else _;
}
  • Estimated Gas Saved = Minimal, but it will save the remaining gas when an operation is not allowed.

Possible Optimizations in SingleAdminAccessControl.sol

Possible Optimization 1 =

  • Instead of emitting separate events for admin transfer, role grant, and role revoke, use a single event to indicate any role change.

After Optimization:

event RoleChanged(address indexed account, bytes32 role, bool isGranted);

function _grantRole(bytes32 role, address account) internal override {
    // ... existing code
    emit RoleChanged(account, role, true);
}

function _revokeRole(bytes32 role, address account) internal override {
    // ... existing code
    emit RoleChanged(account, role, false);
}
  • Estimated gas saved = This would save gas on contract deployment due to reduced bytecode size.
  • Safety = This is safe as long as the event is properly documented to indicate what isGranted means.

Possible Optimization 2 =

  • Instead of using multiple modifiers like onlyRole(DEFAULT_ADMIN_ROLE) and notAdmin(role), use bitfields to combine multiple checks into a single modifier.

After Optimization:

modifier roleChecks(bytes32 role, uint8 checks) {
    if ((checks & 0x01) == 0x01 && !hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) revert InvalidAdminChange();
    if ((checks & 0x02) == 0x02 && role == DEFAULT_ADMIN_ROLE) revert InvalidAdminChange();
    _;
}

function grantRole(bytes32 role, address account) public override roleChecks(role, 0x03) {
    _grantRole(role, account);
}
  • Estimated gas saved = This would save gas by reducing the number of modifiers and therefore the number of jumps in the EVM. Reduces the number of JUMP and JUMPI opcodes.
  • Safety = This is safe as long as the bitfield is properly managed.

#0 - c4-pre-sort

2023-11-01T15:40:16Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-11-10T20:09:56Z

fatherGoose1 marked the issue as grade-b

Awards

14.2357 USDC - $14.24

Labels

analysis-advanced
grade-b
sufficient quality report
A-13

External Links

Advanced Analysis Report for Ethena-Labs

Overview
Understanding the Ecosystem:
Codebase Quality Analysis:

1. Structures and Libraries

  • Mapping Structures: The contracts use mappings for state variables. While gas-efficient, they lack the ability to iterate, making certain types of logic hard to implement.
  • SafeMath Library: Given that Solidity 0.8.x and above have built-in overflow and underflow checks, the explicit use of SafeMath can be omitted to reduce gas costs.

2. Modifiers and Access Control

  • Role-Based Access Control: The contracts use OpenZeppelin's AccessControl, which is robust but could be replaced with a more gas-efficient, in-house solution.
  • Custom Modifiers: The names ensureCooldownOff and ensureCooldownOn could be more descriptive. Consider renaming to requireCooldownInactive and requireCooldownActive.

3. State Variables

  • Immutable vs Mutable: Mutable state variables like cooldownDuration in StakedUSDeV2.sol should be better encapsulated to prevent unauthorized changes.
  • Visibility: The choice of visibility for state variables like cooldowns in StakedUSDeV2.sol should be justified in inline comments.

4. Events and Logging

  • Event Emitting: Events are emitted for significant state changes, but more could be added for better off-chain tracking.
  • Logging: No explicit logging mechanisms are in place, which could be useful for debugging and monitoring.

5. Function Complexity

  • Single Responsibility: Functions like mint() and redeem() in EthenaMinting.sol perform multiple tasks, making them more complex and harder to audit.
  • Gas Costs: Functions like transferInRewards() in StakedUSDe.sol could be optimized for gas by reducing state changes.

6. Upgradability

  • Proxy Patterns: No upgradability patterns like proxy contracts are implemented, which could be a limitation for future upgrades.
  • Immutable Code: Given the absence of upgradability, the code is essentially immutable once deployed, making thorough auditing crucial.

7. Error Handling

  • Revert Statements: The contracts use revert() statements with error messages, which is a good practice for debugging.
Architecture Recommendations:
  • Implement a DAO governance mechanism for critical parameters and role assignments.
  • Consider using Chainlink oracles for any off-chain data requirements.
  • Implement circuit breakers for emergency stops.
  • Use a rate limiter to prevent abuse of minting and redeeming functions.
Centralization Risks:
  • Possible Risk: Single admin control over key contract functionalities.
  • Potential Impact: Admin could maliciously alter contract behavior.
  • Mitigation Strategy: Implement multi-signature requirements or DAO-based governance.
Mechanism Review:
  • The minting and redeeming mechanisms are well-implemented but could benefit from additional checks and balances.
  • The staking mechanism in StakedUSDe.sol and StakedUSDeV2.sol is robust but should include more events for state changes.
Systemic Risks:
  • Centralized control could lead to malicious activities.
Areas of Concern:
  • Lack of upgradability could be a limitation for future improvements.
  • The complexity of some functions could make them more error-prone.
Codebase Analysis:

EthenaMinting Contract

Functions and Risks

mint()
  • Specific Risk: The function uses nonReentrant but still performs multiple state changes after external calls.
  • Recommendation: Move all state changes above external calls to adhere to the Checks-Effects-Interactions pattern.
redeem()
  • Specific Risk: The function uses a nonce and expiry for orders but doesn't account for a user sending two identical orders with different signatures.
  • Recommendation: Include the signature in the mapping to ensure uniqueness.
setMaxMintPerBlock() and setMaxRedeemPerBlock()
  • Specific Risk: These functions can be called by the admin role, potentially disrupting the contract's economics.
  • Recommendation: Implement a timelock for sensitive admin actions.

StakedUSDe Contract

Functions and Risks

transferInRewards()
  • Specific Risk: The function allows for the transfer of rewards but does not validate the reward amount against some maximum.
  • Recommendation: Implement a maximum reward amount check.
addToBlacklist() and removeFromBlacklist()
  • Specific Risk: The function allows blacklisting addresses, which can be abused.
  • Recommendation: Implement a community governance mechanism for these sensitive actions.
rescueTokens()
  • Specific Risk: This function allows the admin to withdraw any ERC20 tokens from the contract.
  • Recommendation: Restrict this function to only allow the withdrawal of non-core tokens.

StakedUSDeV2 Contract

Functions and Risks

withdraw() and redeem()
  • Specific Risk: These functions are only callable when cooldown is off, but there's no mechanism to notify or auto-toggle the cooldown.
  • Recommendation: Implement an auto-toggle for cooldown based on certain conditions.
unstake()
  • Specific Risk: The function allows unstaking only when the cooldown ends but doesn't account for slashing conditions.
  • Recommendation: Implement a slashing mechanism for malicious actors.
cooldownAssets() and cooldownShares()
  • Specific Risk: These functions allow for assets and shares to be put on cooldown but do not limit the number of times this can be done.
  • Recommendation: Implement a rate limiter or maximum cooldown periods per user.
Recommendations:
  • Error Handling: Implement more granular error messages for better debugging.
  • Testing: Conduct extensive unit and integration tests, including edge cases for all functions.
  • Use Tenderly and Defender for continued monitoring to prevent un-foreseen risks or actions.
  • Implement a time-locked admin control for sensitive operations.
  • Conduct a formal verification of the contracts.
  • Implement automated testing for all contract functions.
Contract Details:
Conclusion:
  • Ethena-Labs has a well-structured and robust set of contracts, but there are areas for improvement, particularly in terms of decentralization and upgradability.

Time spent:

15 hours

#0 - c4-pre-sort

2023-11-01T14:24:25Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-11-10T19:35:57Z

fatherGoose1 marked the issue as grade-b

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