tokamak-network / tokamak-thanos

MIT License
6 stars 2 forks source link

Review the use of the ternary operator #221

Closed DevUreak closed 1 week ago

DevUreak commented 2 weeks ago

Describe the bug

How about applying the ternary operator to some parts of the code?

There are instances where we use the ternary operator in the project. Using the ternary operator can reduce gas consumption, and in some cases, it can also improve readability.

Configuration

Severity: low Impact

Recommendation ex. OptimismPortal.sol

base

address from = _sender; if (_sender != tx.origin) { from = AddressAliasHelper.applyL1ToL2Alias(_sender); }

after

address from = _sender != tx.origin ? AddressAliasHelper.applyL1ToL2Alias(_sender) : _sender

Exploit Scenario

Demo

nguyenzung commented 2 weeks ago

Thank you @DevUreak ,

This code belongs to the Optimism team. We did not change anything here. And do you know how much gas can we save if using ternary operator?

DevUreak commented 2 weeks ago

The gas cost per ternary operator conversion is quite low, around 50, so the difference isn't significant even if we don't make the change. I've thought about what we can gain with minimal changes and effort :)

nguyenzung commented 2 weeks ago

I think it is great! Lets use ternary operator. Thank you!

theo-learner commented 2 weeks ago

Thanks @DevUreak. I agree. Your suggestion is that it improves readability and conciseness also saving some gas.

nguyenzung commented 2 weeks ago

Please check: https://github.com/tokamak-network/tokamak-thanos/tree/OR-1807-fix-issues-from-the-internal-audit-s-feedback

DevUreak commented 1 week ago

Confirmed. e67168dbdbc5df0082fa908b609b46cb3dfb9fc9