Decent - hunter_w3b's results

Decent enables one-click transactions using any token across chains.

General Information

Platform: Code4rena

Start Date: 19/01/2024

Pot Size: $36,500 USDC

Total HM: 9

Participants: 113

Period: 3 days

Judge: 0xsomeone

Id: 322

League: ETH

Decent

Findings Distribution

Researcher Performance

Rank: 44/113

Findings: 2

Award: $62.25

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: c3phas

Also found by: 0x11singh99, Raihan, dharma09, hunter_w3b, slvDev

Labels

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

Awards

38.8402 USDC - $38.84

External Links

Codebase Optimization Report

Table Of Contents

The bot did an amazing job on this audit,but still missed some findings

[G-01]Using immutable on variables that are only set in the constructor and never after (Save 8400 Gas)

NOTE: This instance was missed by the bot. Use immutable if you want to assign a permanent value at construction. Use constants if you already know the permanent value. Both get directly embedded in bytecode, saving SLOAD.

  • Gas per instance: 2.1K
  • Total Instances: 4
  • Total Gas Saved: 2.1 * 4 = 8400 Gas

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/DecentBridgeAdapter.sol#L19

File: src/bridge_adapters/DecentBridgeAdapter.sol

19    address bridgeToken;
-     address bridgeToken;
+     address  immutable bridgeToken;

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L34

File: src/DecentEthRouter.sol

15    IDecentBridgeExecutor public executor;
File: src/DecentEthRouter.sol

-    IDecentBridgeExecutor public executor;
+    IDecentBridgeExecutor public immutable executor;

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L13

File: src/DecentBridgeExecutor.sol

9    IWETH weth;
10    bool public gasCurrencyIsEth; // for chains that use ETH as gas currency
-    IWETH weth;
+    IWETH  immutable weth;

-    bool public gasCurrencyIsEth;
+    bool public immutable gasCurrencyIsEth;

[G-02]State variables should be cached in stack variables rather than re reading them from storage (Save 1552 Gas)

This instance was missed by the bot.

  • Gas per instance: 97
  • Total Instances: 16
  • Total Gas Saved: 97 * 16 = 1552 Gas
File: main/src/UTB.sol

76            wrapped.deposit{value: swapParams.amountIn}();
77            swapParams.tokenIn = address(wrapped);
98            wrapped.withdraw(amountOut);


143            executor.execute{value: amountOut}(
152            IERC20(tokenOut).approve(address(executor), amountOut);
153            executor.execute(


// @audit   cache   feeCollector  in local variable

233        if (address(feeCollector) != address(0)) {
242                    address(feeCollector),
248            feeCollector.collectFees{value: value}(fees, packedInfo, signature);

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L76

File: src/DecentBridgeExecutor.sol

// @audit   cache   weth  in local variable
30         uint256 balanceBefore = weth.balanceOf(address(this));
41            (balanceBefore - weth.balanceOf(address(this)));
44        weth.transfer(from, remainingAfterCall);

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L30

File: src/DecentEthRouter.sol


266        if (weth.balanceOf(address(this)) < _amount) {

273                weth.transfer(_to, _amount);

275                weth.withdraw(_amount);

279            weth.approve(address(executor), _amount);

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L266

[G-03]Assigning STATE VARIABLES directly with named struct constructors wastes gas

Using named arguments for struct means that the compiler needs to organize the fields in memory before doing the assignment, which wastes gas. Set each field directly in storage (use dot-notation), or use the unnamed version of the constructor.

File: src/DecentEthRouter.sol

170        ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({
171            refundAddress: payable(msg.sender),
172            zroPaymentAddress: address(0x0),
173            adapterParams: adapterParams
174        });

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L170

File: src/swappers/UniSwapper.

130            .ExactInputParams({
131                path: swapParams.path,
132                recipient: address(this),
133                amountIn: swapParams.amountIn,
134                amountOutMinimum: swapParams.amountOut
135            });



150            .ExactOutputParams({
151                path: swapParams.path,
152                recipient: address(this),
153                //deadline: block.timestamp,
154                amountOut: swapParams.amountOut,
155                amountInMaximum: swapParams.amountIn
156            });

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L130-L135

[G-04]Using STORAGE instead of MEMORY for structs saves gas

When 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

File:  src/bridge_adapters/DecentBridgeAdapter.sol

51        SwapParams memory swapParams = abi.decode(

103        SwapParams memory swapParams = abi.decode(

134        SwapParams memory swapParams = abi.decode(

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/DecentBridgeAdapter.sol#L51

File: src/DecentEthRouter.sol

170        ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L170

File: src/bridge_adapters/StargateBridgeAdapter.sol

202        SwapParams memory swapParams = abi.decode(

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/StargateBridgeAdapter.sol#L202

File: src/swappers/UniSwapper.sol

129        IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter

149        IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L129

File: src/UTB.sol

69        SwapParams memory swapParams = abi.decode(

181        SwapParams memory newPostSwapParams = abi.decode(

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L69

[G-05]Using CALLDATA instead of MEMORY for read only arguments in external functions

NOTE: This instance was missed by the bot.

When a function with a memory array is called externally, the abi.decode ()  step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.

File: src/DecentBridgeExecutor.sol

73        bytes memory callPayload

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L73

File: src/DecentEthRouter.sol

120        bytes memory payload


203        bytes memory additionalPayload

243        bytes memory _payload

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L120

File: src/bridge_adapters/StargateBridgeAdapter.sol

71        SwapInstructions memory postBridge,

75        bytes memory payload,

189        bytes memory payload

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/StargateBridgeAdapter.sol#L71

File: src/swappers/UniSwapper.sol

33        SwapParams memory newSwapParams,

34        bytes memory payload

59        bytes memory swapPayload

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L33

File: src/UTB.sol

312        SwapInstructions memory postBridge,

315        bytes memory payload,

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L312

File: src/UTBExecutor.sol


22        bytes memory payload,

44        bytes memory payload,

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBExecutor.sol#L22

File: src/UTBFeeCollector.sol

46        bytes memory packedInfo,

47        bytes memory signature

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L46

[G-06]Using constants directly, rather than caching the value, saves gas

File: src/bridge_adapters/DecentBridgeAdapter.sol

30    function getId() public pure returns (uint8) {
31        return BRIDGE_ID;
32    }

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/DecentBridgeAdapter.sol#L30

File: src/bridge_adapters/StargateBridgeAdapter.sol

41    function getId() public pure returns (uint8) {
42        return BRIDGE_ID;
43    }

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/StargateBridgeAdapter.sol#L41

File: src/swappers/UniSwapper.sol

28    function getId() public pure returns (uint8) {
29        return SWAPPER_ID;
30    }

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L28-L30

[G-07]Avoid unnecessary public variables

A public storage variable has an implicit public function of the same name. A public function increases the size of the jump table and adds bytecode to read the variable in question. That makes the contract larger.

File:/home/x0w3r/progress-ON/Decent/scope/BaseAdapter.sol

7    address public bridgeExecutor;

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/BaseAdapter.sol#L7

File: /home/x0w3r/progress-ON/Decent/scope/DcntEth.sol

6    address public router;

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L6

File: /home/x0w3r/progress-ON/Decent/scope/DecentBridgeAdapter.sol

15    IDecentEthRouter public router;

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/DecentBridgeAdapter.sol#L15

File: /home/x0w3r/progress-ON/Decent/scope/DecentBridgeExecutor.sol

10    bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L10

File: /home/x0w3r/progress-ON/Decent/scope/DecentEthRouter.sol

13    IWETH public weth;
14    IDcntEth public dcntEth;
15    IDecentBridgeExecutor public executor;
22    bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L13

File:/home/x0w3r/progress-ON/Decent/scope/StargateBridgeAdapter.sol

24    address public stargateEth;

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/StargateBridgeAdapter.sol#L24

File: /home/x0w3r/progress-ON/Decent/scope/UniSwapper.sol

17    address public uniswap_router;
18    address payable public wrapped;

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L17

[G-08]We can save an entire SLOAD (2100 Gas) by short circuiting the operations

  • Gas per instance: 2100
  • Total Instances: 2
  • Total Gas Saved: 2 * 2100 = 4200 Gas

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L77

File: src/DecentBridgeExecutor.sol

77        if (!gasCurrencyIsEth || !deliverEth) {

The if statement has two checks !gasCurrencyIsEth and !deliverEth where the first check involves making a state read while the second check only compares a variable to a function parameter.

According to the rules of short circuit, if the first check is true, we do not have to do the second check thus in this case, we should make sure the first check is the cheapest to do.

By reordering as shown below, we can avoid making the state read if !gasCurrencyIsEth which would save us ~2100 Gas.


-        if (!gasCurrencyIsEth || !deliverEth) {
+        if (!deliverEth || !gasCurrencyIsEth ) {
File: src/DecentEthRouter.sol

272            if (!gasCurrencyIsEth || !deliverEth) {

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L272

[G-09]Using fixed BYTES CONSTANTS is cheaper than using string

If data can fit into 32 bytes, then you should use bytes32 datatype rather than bytes or strings as it is cheaper in solidity.

File: main/src/UTBFeeCollector.sol

10    string constant BANNER = "\x19Ethereum Signed Message:\n32";

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L10

[G-10]Using assembly to revert with an error message

When reverting in solidity code, it is common practice to use a require or revert statement to revert execution with an error message. This can in most cases be further optimized by using assembly to revert with the error message.

File: src/bridge_adapters/BaseAdapter.sol

12        require(

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/BaseAdapter.sol#L12

File: src/DcntEth.sol

9        require(msg.sender == router);

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L9

File: src/bridge_adapters/DecentBridgeAdapter.sol

91        require(

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/DecentBridgeAdapter.sol#L19

File: src/DecentEthRouter.sol

38        require(gasCurrencyIsEth, "Gas currency is not ETH");

43        require(

51        require(weth.balanceOf(address(this)) > amount, "not enough reserves");

62        require(balance >= amount, "not enough balance");

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L38

File: src/bridge_adapters/StargateBridgeAdapter.sol

157        require(

https://github.com/code-423n4/2024-01-decent/blob/main/src/bridge_adapters/StargateBridgeAdapter.sol#L81

File: src/swappers/UniSwapper.sol

96        require(uniswap_router != address(0), "router not set");

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L137

File: src/UTB.sol

75            require(msg.value >= swapParams.amountIn, "not enough native");

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L76

File: src/UTBFeeCollector.sol

29        require(signature.length == 65, "Invalid signature length");

54        require(recovered == signer, "Wrong signature");

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L10

[G-11]Use ASSEMBLY to hash instead of solidity

Use ASSEMBLY for small keccak256 hashes, in order to save gas.If the arguments to the encode call can fit into the scratch space (two words or fewer),then it's more efficient to use assembly to generate the hash (80 gas)

keccak256(abi.encodePacked(x, y)) -> assembly {mstore(0x00, a); mstore(0x20, b); let hash := keccak256(0x00, 0x40); }

File: src/UTBFeeCollector.sol

49        bytes32 constructedHash = keccak256(
50            abi.encodePacked(BANNER, keccak256(packedInfo))

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTBFeeCollector.sol#L49

#0 - raymondfam

2024-01-26T16:51:33Z

All findings are generically known to complement the bot report.

#1 - c4-pre-sort

2024-01-26T16:51:38Z

raymondfam marked the issue as sufficient quality report

#2 - alex-ppg

2024-02-04T17:56:08Z

These findings were penalized:

  • G-04: Invalid as these are input arguments and cannot be storage
  • G-06: Invalid as these functions need to be invokable
  • G-11: Inapplicable as the optimization pertains to two fixed 32-byte arguments, will not penalize but should not have been submitted

#3 - c4-judge

2024-02-04T17:56:14Z

alex-ppg marked the issue as grade-b

Findings Information

Labels

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

Awards

23.4096 USDC - $23.41

External Links

Analysis - Decent Contest

Decent-Protocol

Description overview of The Decent Contest

Decent allows users to perform transactions across multiple blockchain networks (like Ethereum, Polygon, Optimism etc.) in a single click, even if they hold funds/tokens on a different chain.

The protocol has two components phases:

  • UTB and Decent Bridge.

    • UTB handles routing the cross-chain transaction and passes it to the appropriate bridge. It supports functions like swapAndExecute (for on-chain swaps) and bridgeAndExecute (for cross-chain transactions).

    • Decent Bridge is built on LayerZero's OFT standard and acts as the actual bridge, facilitating the transfer of tokens/funds from one chain to another.

Implementation Standards:

  • Swappers: These are modules that perform same-chain transactions. They must implement the ISwapper interface.

  • Bridge Adapters: These are modules that facilitate cross-chain transactions. They must implement the IBridgeAdapter interface. Examples include DecentBridgeAdapter and StargateBridgeAdapter.

Decent simplifies the process of making transactions across various blockchains, allowing users to use different tokens and providing flexibility in payment methods. The platform is built on specific libraries and standards to ensure compatibility and extendability.

The key contracts of Decent protocol for this Audit are:

As the audit of these contracts, I checked for potential security vulnerabilities, such as reentrancy, access control issues, and logic flaws. Additionally, thoroughly test the functions and roles defined in these contracts to make sure they behave as expected.

System Overview

Scope

  • UTB.sol: The UTB contract facilitates one-click cross-chain transactions by coordinating token swaps, fee collection, and interaction with multiple blockchain networks through swappers and bridge adapters in the Decent protocol.

  • UTBExecutor.sol: This contract extending the Owned contract, enables the execution of payment transactions involving native or ERC20 tokens, handling transfers, approvals, and refunds, with flexibility for additional gas or native fees in the Decent protocol.

  • UTBFeeCollector.sol: UTBFeeCollector contract inheriting from UTBOwned, facilitates the collection and verification of fees, supporting both native and ERC20 tokens, with a specified signer for fee structure verification in the Decent protocol.

  • BaseAdapter.sol: The BaseAdapter contract establishes a foundation for bridge adapters in the Decent protocol, providing a mechanism to set the bridge executor and a modifier to restrict certain functions to be callable only by the designated bridge executor.

  • DecentBridgeAdapter.sol: Serves as a bridge adapter in the Decent protocol, facilitating token transfers between different blockchain networks using the Decent protocol, with customizable fee estimation and bridging functionality.

  • StargateBridgeAdapter.sol:This contract is a bridge adapter for the Stargate protocol, enabling cross-chain token transfers with customizablefee estimation and integration with the Stargate network.

  • UniSwapper.sol: The UniSwapper contract, inheriting from UTBOwned and implementing the ISwapper interface, serves as a decentralized exchange adapter for UniSwap, enabling token swaps with customizable parameters, fee estimation, and seamless integration into the Universal Token Bridge (UTB) ecosystem.

  • DcntEth.sol: The contract is extending the OFTV2 token contract, represents a decentralized token on the Ethereum blockchain named "Decent Eth" (with symbol "DcntEth") that supports minting and burning functionalities, and is associated with a Decent Eth Router for controlled access to these operations.

  • DecentEthRouter.sol: The contract is an Ethereum router facilitating cross-chain transactions with DecentEth tokens, supporting the transfer and call functionalities, and managing reserves of WETH for bridging operations between Ethereum and other chains.

  • DecentBridgeExecutor.sol: This contract is serving as an executor for cross-chain transactions, handling the execution of transactions with either WETH or native Ether, based on the gas currency configuration, and facilitating calls to target contracts.

Protocol Roles

Here are the key roles

  1. Owner Role:

    • Identified by the onlyOwner modifier.
    • Typically has privileged access to certain functions that are critical for the protocol's owner.
    • Responsible for setting configurations, adding destination bridges, and managing the protocol.
  2. Router Role:

    • Identified by the onlyRouter modifier.
    • The router is responsible for specific actions and configurations within the protocol, particularly related to routing and interacting with other components of the system.
  3. LzApp Role:

    • Identified by the onlyLzApp modifier.
    • Represents the Layer Zero application, specifically the DecentEthRouter contract, allowing it to call certain functions.
  4. User Roles:

    • Various modifiers such as userDepositing and userIsWithdrawing are used to manage user-specific actions.
    • Users can deposit and withdraw assets from the protocol.

Privileged Roles

Some privileged roles exercise powers over the contracts:

  • Bridge Executor
    modifier onlyExecutor() {
        require(
            msg.sender == address(bridgeExecutor),
            "Only bridge executor can call this"
        );
        _;
    }
  • Router
    modifier onlyRouter() {
        require(msg.sender == router);
        _;
    }
  • lz App
    modifier onlyLzApp() {
        require(address(dcntEth) == msg.sender, "DecentEthRouter: only lz App can call");
        _;
    }

These roles help define the permissions and access levels within the protocol, ensuring that only authorized entities can perform certain operations. The roles contribute to the overall security and functionality of the decentralized application by controlling access to critical functions and resources.

Approach Taken-in Evaluating The Decent Protocol

Accordingly, I analyzed and audited the subject in the following steps;

  1. Core Protocol Contract Overview:

    I focused on thoroughly understanding the codebase and providing recommendations to improve its functionality. The main goal was to take a close look at the important contracts and how they work together in the Decent Protocol.

    I start with the following contracts, which play crucial roles in the Decent protocol:

    Main Contracts I Looked At

    I start with the following contracts, which play crucial roles in the Decent protocol:

    UTB.sol DecentBridgeAdapter.sol StargateBridgeAdapter.sol UniSwapper.sol DecentEthRouter.sol

    I started my analysis by examining the UTB.sol contract, as this contract contains the core logic for Decent, enabling one-click transactions across different blockchain networks through functions like performSwap, swapAndExecute, and bridgeAndExecute

    Then, I turned my attention to the DecentBridgeAdapter.sol contract a critical component of Decent, which extends the BaseAdapter and IBridgeAdapter interfaces, facilitating cross-chain transactions by interacting with the Decent Eth Router, estimating fees, and managing the bridging of funds and execution of transactions across different chains.

    The StargateBridgeAdapter contract is an integral part of Decent, extends the BaseAdapter, IBridgeAdapter, and IStargateReceiver interfaces, facilitating cross-chain transactions via the Stargate protocol, managing the bridging of funds and execution of transactions across different chains, estimating fees, and interacting with the Stargate Router through functions like bridge, getValue, and sgReceive.

    The UniSwapper contract, an essential component of the Decent protocol, extends the UTBOwned and ISwapper interfaces, providing functionality for swapping tokens using Uniswap's V3 Router, handling different swap scenarios, and interacting with the specified Uniswap router to facilitate token swaps with features like swap, swapNoPath, swapExactIn, and swapExactOut.

  2. Documentation Review:

    Then went to Review This Documentation for a more detailed and technical explanation of Decent protocol.

  3. Compiling code and running provided tests:

    • To setup the repo, first run forge install + pnpm i

    • To run the tests, simply add the relevant files to your .env, referencing .env.example, then run forge test

  4. Manuel Code Review In this phase, I initially conducted a line-by-line analysis, following that, I engaged in a comparison mode.

    • Line by Line Analysis: Pay close attention to the contract's intended functionality and compare it with its actual behavior on a line-by-line basis.

    • Comparison Mode: Compare the implementation of each function with established standards or existing implementations, focusing on the function names to identify any deviations.

Codebase Quality

Overall, I consider the quality of the Decent protocol codebase to be Good. The code appears to be mature and well-developed. We have noticed the implementation of various standards adhere to appropriately. Details are explained below:

Codebase Quality CategoriesComments
Code Maintainability and ReliabilityThe Decent Protocol contracts demonstrates good maintainability through modular structure, consistent naming. It also prioritizes reliability by handling errors, conducting security checks. However, for enhanced reliability, consider professional security audits and documentation improvements. Regular updates are crucial in the evolving space.
Code CommentsDuring the audit of the Decent contracts codebase, I found that some areas lacked sufficient internal documentation to enable independent comprehension. While comments provided high-level context for certain constructs, lower-level logic flows and variables were not fully explained within the code itself. Following best practices like NatSpec could strengthen self-documentation. As an auditor without additional context, it was challenging to analyze code sections without external reference docs. To further understand implementation intent for those complex parts, referencing supplemental documentation was necessary.
DocumentationThe documentation of the Decent project is quite comprehensive and detailed, providing a solid overview of how Bridging is structured and how its various aspects function. However, we have noticed that there is room for additional details, such as diagrams, to gain a deeper understanding of how different contracts interact and the functions they implement. With considerable enthusiasm. We are confident that these diagrams will bring significant value to the protocol as they can be seamlessly integrated into the existing documentation, enriching it and providing a more comprehensive and detailed understanding for users, developers and auditors.
TestingThe audit scope of the contracts to be reviewed is 75%, with the aim of reaching 100% to increase the safety.
Code Structure and FormattingThe codebase contracts are well-structured and formatted. but some order of functions does not follow the Solidity Style Guide According to the Solidity Style Guide, functions should be grouped according to their visibility and ordered: constructor, receive, fallback, external, public, internal, private. Within a grouping, place the view and pure functions last.
ErrorUse custom errors, custom errors are available from solidity version 0.8.4. Custom errors are more easily processed in try-catch blocks, and are easier to re-use and maintain.

Systemic & Centralization Risks

The analysis provided highlights several significant systemic and centralization risks present in the Decent protocol. These risks encompass concentration risk in UTB, UTBExecutor risk and more, third-party dependency risk, and centralization risks arising.

Here's an analysis of potential systemic and centralization risks in the contracts:

Systemic Risks:

  1. UTB.sol:

    • External Calls Without Checks: External calls, such as performSwap and callBridge, are made without checking the return values. A failed external call might leave the contract in an inconsistent state. It's important to handle potential failures properly.
  2. UTBFeeCollector.sol

    • Signature Verification Reliance: The security of fee collection relies heavily on the correctness of the signature verification process. Any vulnerability or weakness in the signature verification logic could lead to unauthorized fee collection.
  3. StargateBridgeAdapter.sol

    • Value Calculation Precision: The calculation of the value variable in the callBridge function involves multiplication and division operations with potentially large numbers. Ensure that these calculations are precise and do not result in unintended rounding errors.
  4. UniSwapper.sol

    • Single swapping mechanism relies on Uniswap router which could break or have exploits impacting all trades.

Centralization Risks:

  1. UTB.sol:

    • Single Owner Control: The contract is owned by a single address (Owned), creating centralization risks. The owner has significant control over critical functions such as setting the executor, wrapped token, fee collector, registering swappers, and bridge adapters.

    • Fee Collection Centralization: The fee collection mechanism (feeCollector.collectFees) is central to the contract, relying on a single fee collector contract. If this fee collector becomes compromised or malfunctions, it could impact the entire fee collection process.

    • Bridge Adapter Registration: The registration of bridge adapters is centralized, allowing only the owner to add or modify bridge adapters. This creates a single point of control and potential risk if the owner becomes malicious or compromised.

  2. UTBFeeCollector.sol

    • Single Owner Control: The contract is owned by a single address (UTBOwned), creating centralization risks. The owner has significant control over critical functions, such as setting the signer and redeeming fees.

    • Signer Configuration: The signer address, crucial for fee verification, is set by the owner. If the owner is compromised or acts maliciously, they can set a malicious signer, compromising the entire fee collection process.

    • RedeemFees Function: The ability to redeem fees is centralized in the owner. This function allows the owner to withdraw collected fees, and its execution is solely dependent on the owner's decision.

  3. DecentBridgeAdapter.sol

    • Centralized Destination Bridge Adapter Registration: The registration of remote bridge adapters (destinationBridgeAdapter) is centralized and controlled by the owner. If the owner is compromised or acts maliciously, they could register a malicious destination bridge adapter.

Properly managing these risks and implementing best practices in security and decentralization will contribute to the sustainability and long-term success of the Decent protocol.

Conclusion

Here are the key points I would include in a conclusion:

  • Overall the code quality and architecture of the Decent protocol contracts are good. The code is well structured, tested and follows solidity best practices.

  • Some opportunities for improvement include adding more internal documentation, error handling, formal verification, and upgrading to support future changes.

  • There are some Centralization risks arise from single owner control over critical settings in contracts like UTB, reliance on single fee collector/signer, and centralized bridge adapter registration.

  • Systemic risks like external call vulnerabilities and value calculation precision issues could impact the protocol.

  • Following best practices in security, decentralization and ongoing maintenance/audits will help ensure the long-term sustainability and success of the protocol.

  • It is also highly recommended that the team continues to invest in security measures such as mitigation reviews, audits, and bug bounty programs to maintain the security and reliability of the project.

Time spent:

15 hours

#0 - c4-pre-sort

2024-01-26T17:56:34Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2024-02-03T14:49:47Z

alex-ppg 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