suberra / funnel-contracts

Funnels are smart contracts that enforces renewable allowance as a proxy
MIT License
9 stars 0 forks source link

[Hacken 2022-11-25 Low #12] Code Consistency #47

Closed zhongfu closed 1 year ago

zhongfu commented 1 year ago

It is best practice to write code uniformly.

There is no consistency in how reverts are handled or in the messages for those reverts in the Funnel contract.

In one case, custom errors are used; in another, revert with a message; and in another, requires.

Path

./src/Funnel.sol : permit(), permitRenewable(), transferFromAndCall(), _checkOnTransferReceived(), approveRenewableAndCall(), _checkOnApprovalReceived()

Recommendation

Be consistent with the approach to reverting and the messages sent when reverting.


In the FunnelFactory contract, there is no consistency in using single-line if statements. Sometimes single-line is used, and sometimes curly braces are in use. Line 30 vs. Line 34.

Path

./src/FunnelFactory.sol : deployFunnelForToken(), getFunnelForToken(), isFunnel()

Recommendation

Be consistent with style, formatting, and patterns.


Status

New

zlace0x commented 1 year ago

Partially resolved in #58

zlace0x commented 1 year ago

Fixed in #62