tokamak-network / L2-Assets-Migration

3 stars 4 forks source link

Gas optimization in forceWithdrawClaim and claim functions #22

Closed NegruGeorge closed 1 month ago

NegruGeorge commented 1 month ago

Describe the bug In the UpgradeL1Bridge contract, the function forceWithdrawClaim and claim use string memory _hash as a parameter, which could be optimized by changing it to string calldata _hash. This change can reduce gas costs by avoiding the unnecessary copying of data into memory.

Affected Functions:

// code before
function forceWithdrawClaim(address _position, string memory _hash, address _token, uint _amount) external {
    claim(_position, _hash, _token, _amount);
}

// code before
function claim(address _position, string memory _hash, address _token, uint _amount) internal {
    // function body
}

Configuration

Impact Changing the parameter type to calldata can lead to reduced gas costs, making the function more efficient, especially for external calls.

Recommendation Change the parameter type from memory to calldata in both functions:

// code after
function forceWithdrawClaim(address _position, string calldata _hash, address _token, uint _amount) external {
    claim(_position, _hash, _token, _amount);
}

// code after
function claim(address _position, string calldata _hash, address _token, uint _amount) internal {
    // function body
}

Exploit Scenario This issue does not pose a direct security risk but leads to higher gas costs for users interacting with these functions.

Demo using the gas reporter

 const tx  =  await l1Bridge.connect(claimer2).forceWithdrawClaimAll([{
            position : deployedStorage[1],
            hashed : "0x6319b1298f3f2f709fe62fc33c4a92c766d103d33af8d3a6f3aa199657f71598",
            token : "0x2be5e8c109e2197D077D13A82dAead6a9b3433C5",
            amount : "22000000000000000000"
        }])

        const receipt = await tx.wait();
        console.log("Gas used: ", receipt.gasUsed.toString())
 const receipt = await tx.wait();
 console.log("Gas used: ", receipt.gasUsed.toString())

GAS USED with MEMORY: Gas used: 92074

GAS USED with CALLDATA: Gas used: 91437

DevUreak commented 1 month ago

Thank you for the gas usage report. I will apply it in the next update.

DevUreak commented 1 month ago

fixed ea49bc866f25816097025430e3305508eb352cba thanks !