code-423n4 / 2023-09-centrifuge-findings

16 stars 14 forks source link

Contract may consume excessive gas, potentially leading to transaction failures or expensive transactions #766

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-centrifuge/blob/512e7a71ebd9ae76384f837204216f26380c9f91/src/Root.sol#L75

Vulnerability details

Contract may consume excessive gas, potentially leading to transaction failures or expensive transactions

Some operations in the contract may consume excessive gas, potentially leading to transaction failures or expensive transactions. Gas efficiency is important to ensure that transactions are processed smoothly and do not exceed the gas limit.

Code Block Examples:

Current Implementation:

function executeScheduledRely(address target) public {
    require(schedule[target] != 0, "Root/target-not-scheduled");
    require(schedule[target] < block.timestamp, "Root/target-not-ready");

    wards[target] = 1;
    emit Rely(target);

    schedule[target] = 0;
}

Recommended Mitigation:

Optimize Gas Usage for Loops:

// Updated function to optimize gas usage for loop
function executeScheduledRely(address target) public {
    require(schedule[target] != 0, "Root/target-not-scheduled");
    require(schedule[target] < block.timestamp, "Root/target-not-ready");

    // Optimize gas usage by reducing storage writes
    if (wards[target] != 1) {
        wards[target] = 1;
        emit Rely(target);
    }

    schedule[target] = 0;
}

Explanation:

In the updated code:

  1. Optimized gas usage by avoiding unnecessary storage writes. In the original code, wards[target] was set to 1 even if it was already set to 1, leading to an unnecessary gas cost for the storage write operation.

  2. Add a condition to check if wards[target] is not already 1 before writing to storage and emitting the Rely event. This avoids excessive gas consumption when the state doesn't change.

Assessed type

Payable

c4-pre-sort commented 1 year ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #147

c4-judge commented 1 year ago

gzeon-c4 changed the severity to QA (Quality Assurance)