Shell Protocol - Sathish9098's results

GAS OPTIMIZATION

[G-1] Optimizing state variables for minimal storage usage

The EVM uses 32-byte words, allowing variables smaller than 32 bytes to be packed together in one storage slot if their combined size is ≤32 bytes. Accessing these packed variables saves roughly 2000 gas per SLOAD, costing only 100 gas ('Gwarmaccess') compared to 2100 gas ('Gcoldsload') for separate slots.

[G-1.1] _ERC1155InteractionStatus , _ERC721InteractionStatus can be packed with same SLOT : Saves 2000 GAS , 1 SLOT

The value of _ERC1155InteractionStatus and _ERC721InteractionStatus values always interchanged between NOT_INTERACTION , INTERACTION constant values . So uint128 perfectly safe instead of uint256. This will not harm protocol and perfectly safe conversion.

FILE: 2023-11-shellprotocol/src/ocean/Ocean.sol

106: uint256 constant NOT_INTERACTION = 1;
107: uint256 constant INTERACTION = 2;
- 108: uint256 _ERC1155InteractionStatus;
- 109: uint256 _ERC721InteractionStatus;
+ 108: uint128 _ERC1155InteractionStatus;
+ 109: uint128 _ERC721InteractionStatus;

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L108-L109

[G-2] Don't cache state variable only used once

There are a few instances where state variables are assigned to local variables but are only used once. These can be optimized by directly using the state variable in the expression.

FILE: 2023-11-shellprotocol/src/adapters/Curve2PoolAdapter.sol

function wrapToken(uint256 tokenId, uint256 amount) internal override {
-        address tokenAddress = underlying[tokenId];

        Interaction memory interaction = Interaction({
-            interactionTypeAndAddress: _fetchInteractionId(tokenAddress, 
+            interactionTypeAndAddress: _fetchInteractionId(underlying[tokenId], uint256(InteractionType.WrapErc20)),
            inputToken: 0,
            outputToken: 0,
            specifiedAmount: amount,
            metadata: bytes32(0)
        });

        IOceanInteractions(ocean).doInteraction(interaction);
    }

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/Curve2PoolAdapter.sol#L102-L114

[G-3] Modulus operations that could be unchecked

Modulus operations should be unchecked to save gas since they cannot overflow or underflow. Execution of modulus operations outside unchecked blocks adds nothing but overhead. Saves about 30 gas.

FILE: 2023-11-shellprotocol/src/ocean/Ocean.sol

1145:  truncatedAmount = amountToConvert % shift;

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/ocean/Ocean.sol#L1145

[G-4] Use assembly to validate msg.sender

We can use assembly to efficiently validate msg.sender with the least amount of opcodes necessary. For more details check the following report Here

FILE : 2023-11-shellprotocol/src/adapters/OceanAdapter.sol

39: require(msg.sender == ocean);

https://github.com/code-423n4/2023-11-shellprotocol/blob/485de7383cdf88284ee6bcf2926fb7c19e9fb257/src/adapters/OceanAdapter.sol#L39

// SPDX-License-Identifier: MIT

pragma solidity 0.8.20;



contract ControlContract{


    function control(uint a, uint b) public returns (uint256) {
        return a % b;
    }

    
}

contract TestContract{


    

    function control(uint a, uint b) public returns (uint256) {
        unchecked{
            return a % b;
        }
    }

    
}

Analysis Shell Protocol

Overview

Shell Protocol is a decentralized finance (DeFi) platform designed to simplify and enhance the DeFi experience for both users and developers

One-Click Transactions: Shell Protocol aims to streamline the user experience by offering powerful, one-click transactions. This feature is particularly beneficial for those new to DeFi or those seeking a more efficient and less complex interaction with DeFi services.

Capital-Efficient Automated Market Makers (AMMs): At the heart of Shell Protocol is its set of Automated Market Makers, which are touted for their exceptional capital efficiency. This means that liquidity providers can expect better returns on their assets, and traders can enjoy lower slippage and more stable prices. The AMMs are designed to be robust and flexible, catering to a wide range of financial assets.

Modular Developer Experience : Recognizing the diverse needs of DeFi developers, Shell Protocol offers a modular design. This approach allows developers to easily integrate Shell's features into their projects or to build on top of the platform. The modular design also facilitates customization and scalability, enabling developers to create tailored DeFi solutions.

Project’s risk model

Admin abuse risks

196:     function changeUnwrapFee(uint256 nextUnwrapFeeDivisor) external override onlyOwner {

onlyOwner (Function: changeUnwrapFee()):

This modifier restricts the execution of the function to the owner of the contract.

Risk: If the owner account is compromised, the attacker could manipulate the unwrap fee, potentially causing financial loss to users or destabilizing the protocol. Additionally, even without malicious intent, the owner has significant power to alter contract parameters, which can lead to a lack of trust if changes are made arbitrarily or without community consensus.

289:         onlyApprovedForwarder(userAddress)

onlyApprovedForwarder(userAddress) (Functions: forwardedDoInteraction(), forwardedDoMultipleInteractions()):

This modifier likely allows only an approved forwarder address, linked to the userAddress, to execute the function.

Risk: The implementation details of how a forwarder is approved are crucial. If the process of approving forwarders is not decentralized or transparent, it can lead to centralization risks where the administrator could favor certain forwarders. Also, if the mechanism for approval is not secure, it might be susceptible to manipulation, allowing unauthorized entities to act as forwarders.

64:         onlyOcean

onlyOcean (Functions: computeOutputAmount(), computeInputAmount()):

This modifier restricts function execution to addresses authorized by the Ocean protocol.

Risk : Similar to onlyOwner, this creates a single point of failure. If the Ocean authority is compromised or acts maliciously, they could potentially manipulate these functions for personal gain or disrupt the protocol's operations. This risk is compounded if the Ocean authority has a broad range of powers over many aspects of the protocol.

Mitigation Strategies

Decentralized Governance : Transitioning from a single owner model to a decentralized governance structure can mitigate risks associated with the onlyOwner modifier. This would involve community members or token holders voting on critical decisions, thus distributing power.

Transparent Approval Process : For the onlyApprovedForwarder modifier, ensuring a transparent and secure process for approving forwarders is essential. This might include community oversight or stringent security checks.

Role Rotation and Multi-Signature Controls : Implementing multi-signature wallets and regularly rotating the roles responsible for critical functions can reduce risks associated with the onlyOcean modifier. This ensures that no single entity has prolonged, unchecked control.

Systemic risks

Token Wrapping and Unwrapping Risk in Fee Management

The contract manages fees for wrapping and unwrapping tokens, particularly with ERC-20 and ERC-1155 tokens. Any miscalculation or manipulation in fee management can lead to economic vulnerabilities, affecting user incentives and the protocol's sustainability.

Risks in Forwarded Interactions

The forwardedDoInteraction and forwardedDoMultipleInteractions functions, which use onlyApprovedForwarder, introduce additional risks. If the logic for approving forwarders is flawed or if a forwarder is compromised, it could lead to unauthorized or malicious transactions.

Risk of Slippage and Price Impact

The contract checks for slippage limits (SLIPPAGE_LIMIT_EXCEEDED). If the actual output amount of a swap, deposit, or withdrawal is lower than the user's expected minimum, the transaction is reverted. This mechanism can protect users from high slippage, but if not calibrated correctly, it could also lead to failed transactions during high volatility, impacting the user experience and trust in the protocol.

ERC20 Token Approval Risks

The contract grants maximum (type(uint256).max) approval to the ocean and primitive addresses for ERC20 tokens. While this is a common pattern to reduce transaction costs, it also means that if either of these addresses is compromised, it could lead to the loss of user funds.

Conversion and Rounding Errors

The contract uses _convertDecimals for normalizing token amounts to the Ocean's 18-decimal standard. Inaccuracies or rounding errors in these conversions could lead to financial discrepancies, especially for tokens with different decimal standards.

Techinical Risks

Complex Token Wrapping/Unwrapping Logic

The logic for wrapping and unwrapping tokens is intricate, with multiple points of interaction with the contract's state and external tokens. Bugs or flaws here could lead to incorrect token balances or loss of funds.

Non-upgradeability Concerns

The contract's apparent lack of upgradeability means that addressing any discovered vulnerabilities or design issues could be challenging, posing long-term systemic risks in a rapidly evolving DeFi landscape.

Gas Usage and Network Load

The complex operations of the contract may lead to high gas costs, affecting its usability and performance, especially during times of network congestion.

Complexity in Handling Multiple Token Standards

The contract interacts with multiple token standards, each with its own set of rules and potential edge cases. This complexity increases the risk of bugs or security vulnerabilities, especially when dealing with the intricacies of each standard, like ERC721 and ERC1155's safe transfer mechanisms.

Complexity in Compute Type Determination

The _determineComputeType function determines the action (swap, deposit, withdraw) based on input and output tokens. Incorrect implementation or manipulation of this logic could lead to unintended contract behaviors, affecting liquidity operations and user funds.

Unanticipated Interactions with Other Protocols

Given the interconnected nature of DeFi, changes or updates in other protocols that interact with the Curve 2pool could inadvertently affect the adapter's functionality, leading to systemic risks in the broader ecosystem.

Dependency on Accurate Token Indexing

The contract uses indexOf mapping to associate Ocean IDs with their corresponding Curve pool indices. Any incorrect mapping or updates to these indices could result in failed transactions or incorrect swaps, impacting liquidity and user funds.

Integration risks

Protocol Interoperability

Protocol interoperability, especially in the context of the Curve2PoolAdapter and Ocean contracts within the DeFi ecosystem, refers to the ability of these systems to work together seamlessly. Given that DeFi protocols often rely on each other's functionalities and data, interoperability is crucial for their collective success and user trust.

Technical Aspects of Protocol Interoperability

Smart Contract Interface Compatibility:

• The Curve2PoolAdapter and Ocean contracts must adhere to specific interfaces to interact with Curve Finance's pools and other external protocols.
• These interfaces involve function signatures, return types, and event definitions. Any mismatch in these interfaces can lead to failed transactions or incorrect data interpretation.

Data Format and Standardization:

• Interacting protocols must agree on data formats, such as the representation of token amounts, addresses, and transaction data.
• For instance, token amounts might need normalization to a standard decimal representation to ensure accurate calculations during swaps or liquidity provisioning.

Handling Protocol-Specific Logic:

• Curve Finance pools, like the 2pool, have unique mechanics, such as variable fees, slippage calculations, and liquidity dynamics.

• The Curve2PoolAdapter must accurately incorporate these mechanics into its logic to ensure that operations like token swaps or liquidity management reflect the underlying Curve pool's state.

• State Synchronization and Updates:

• The state of one protocol (like liquidity levels in Curve pools) can affect the decisions and operations in another (like optimal routing in the Ocean contract).

• Ensuring real-time or near-real-time synchronization of state information across protocols is vital for maintaining operational accuracy.

Test Suite

• The report highlights a significant discrepancy in test coverage, with certain areas like mock and ocean directories being well-tested, while others like adapters, proteus, and test/fork directories have inadequate testing.
• The lack of testing in key areas such as the adapters and parts of the Proteus directory is a major concern. These untested sections could harbor undetected bugs or vulnerabilities, posing a risk to the protocol's overall security and functionality.
• The high coverage in certain areas demonstrates a capability for thorough testing, which needs to be extended to the under-tested sections of the codebase.

--------------------------------------|----------|----------|----------|----------|----------------|

Recommendations

• Prioritize increasing test coverage in the adapters and proteus directories.
• Review and enhance the tests in the test/fork directory to ensure they are effectively assessing the code.
• Maintain the high level of testing in areas where coverage is already strong, ensuring ongoing robustness and reliability.

Architecture assessment of business logic

The Ocean contract, as provided, is a comprehensive Solidity contract designed for DeFi applications, primarily focusing on the management and interaction of various token standards

Core Functionalities:

Token Handling:

The contract supports interactions with multiple token types including ERC-20, ERC-721, and ERC-1155. It provides functionalities for wrapping and unwrapping these tokens. Wrapping converts tokens into a standard format within the contract, while unwrapping allows users to extract their original tokens.

DeFi Interaction Execution:

Functions like doInteraction and doMultipleInteractions allow the contract to process both individual and batch interactions. These interactions involve executing calls to external contracts and updating the contract's state (like minting and burning tokens) based on these interactions.

Fee Management for Unwrapping:

The contract includes a mechanism to handle fees, particularly for unwrapping tokens. This is managed through the unwrapFeeDivisor, which calculates the fee for unwrapping operations.

Design and Structure:

The contract inherits from OceanERC1155 and implements interfaces such as IOceanInteractions, IOceanFeeChange, IERC721Receiver, and IERC1155Receiver. This indicates a modular design approach and compliance with ERC standards. It uses OpenZeppelin contracts for standard functionalities related to ERC token interfaces, indicating a reliance on established and tested implementations.

Security Mechanisms:

The contract appears to incorporate reentrancy guards, as indicated by the use of constants like NOT_INTERACTION and INTERACTION and related state variables. It includes access control mechanisms, though the specific implementation details are not fully provided in the snippet.

Events and Modifiers:

Several events are declared, which are crucial for logging and tracking transactions within the contract. Custom modifiers, like onlyApprovedForwarder, are used for controlling access to certain functions.

Software engineering considerations

Modularity and Reusability:

The contract exhibits modularity by inheriting from several interfaces and contracts, suggesting components are designed for reuse and extension. This approach aligns with solid software engineering principles, promoting maintainability and scalability.

Design Patterns and Best Practices:

Utilization of established design patterns, such as interface-based programming and reentrancy guards, indicates adherence to industry best practices. These patterns enhance the contract’s robustness and security.

Code Readability and Maintainability:

The contract's structure, naming conventions, and comments suggest an emphasis on readability and maintainability, crucial for long-term management and updates. Security:

Security is a critical aspect, especially given the DeFi context. The contract shows awareness of common vulnerabilities (like reentrancy attacks) and includes mechanisms to mitigate them. Continuous security audits and reviews are essential due to the evolving nature of smart contract exploits. Efficiency and Optimization:

Functions like doMultipleInteractions suggest a focus on optimizing transactions, likely to reduce gas costs. However, given the contract's complexity, there's a need for thorough analysis and testing to ensure efficiency, especially in gas usage.

Error Handling and Input Validation:

The use of custom error messages and checks indicates a proactive approach to error handling and input validation, critical for robustness and user trust.

Interoperability and Standards Compliance:

Compliance with ERC standards and the ability to interact with multiple token types demonstrate a high level of interoperability, a significant factor in the Ethereum ecosystem.

Testing and Documentation:

Given the complexity, comprehensive testing (unit tests, integration tests) and detailed documentation are vital for ensuring the contract works as intended and to facilitate future development.

Upgradeability:

The potential for future upgrades and modifications should be considered, especially in a rapidly evolving domain like DeFi. However, the contract doesn’t explicitly mention upgrade mechanisms.

Dependency Management:

Reliance on external libraries like OpenZeppelin means the contract's security and functionality are partly dependent on these external components. Keeping these dependencies updated and secure is crucial.

Time Spend

20 Hours

Time spent:

20 hours