tokamak-network / crossTrade

Cross Trade is a new core service for optimistic rollups that complements standard withdrawals and fast withdrawals. It is trustless and do not require extensive backend.
4 stars 1 forks source link

[Gas Efficiencies] optimize gas costs by modifying function visibility #34

Closed usgeeus closed 4 months ago

usgeeus commented 4 months ago

Describe the bug functions are declared public unnecessarily.

Configuration

Impact wasting gas fee unnecessarily.

Recommendation L1CrossTrade.sol

L2CrossTrade.sol

*internal -> private Changing internal to private doesn't optimize gas costs, but it does make it more strict.

Exploit Scenario Demo

zzooppii commented 4 months ago

L1CrossTrade.sol i think front use fucntions

so L1CrossTrade.sol _approve() internal -> private --> _approve() private _getChainID() public -> private --> getChainID() public (not change) makeEncodeWithSignature() public -> private --> Need to consult with the front desk getHash() public -> private --> getHash() public (not change)

L2CrossTrade.sol _request() internal -> private --> _request() private _approve() internal -> private --> _approve() private _getChainID() internal -> private --> getChainID() public getEnterHash() public -> private --> getEnterHash() public (not change) getHash() public -> private --> getHash() public (not change)

I think it will change like this. I will tell you the final conclusion after discussing with the front desk.

usgeeus commented 4 months ago

I think unnecessary rpc calls can be made.

  1. getChainID() In order to call a function in any contract on the frontend, you need the chainId in addition to the RPC settings in advance, is there any reason to call the getChainID function from frontend?

  2. makeEncodeWithSignature(), gethash(), getEnterHash() abi.encodeWithSignature, abi.encode and keccak256 can be performed using ethers.js or other library in frontend. Is there any reason to call this function from frontend??

zzooppii commented 4 months ago

I plan to develop it in an easy-to-use way on the front end. If the front desk says it is not needed, I will change it entirely to private.

usgeeus commented 4 months ago

Ok thanks

zzooppii commented 4 months ago

@usgeeus I talked to the front team and adjusted the functions. Please confirm.

L1CrossTrade.sol _getChainID() public -> private makeEncodeWithSignature() public -> private getHash() public -> private

L2CrossTrade.sol _request() internal -> private _getChainID() internal -> private getHash() public