Coding style can lead to confusion
The code makes use of short if clauses without explicit scoping, which can lead to some confusion during development and auditing. One example is the following snippet from the function _swapMulti()
.
referralInfo memory thisReferralInfo;
if (referralCode > REFERRAL_WITH_FEE_THRESHOLD) thisReferralInfo = referralLookup[referralCode];
{
uint256 valueOut;
uint256 _swapMultiFee = swapMultiFee;
amountsOut = new uint256[](outputs.length);
...
}
Due to the use of scoping inside the function, the one-liner if sentence, and no white space between those two, it is easy at first glance to assume that the scope is only executed if the clause is true, when in reality the code is equivalent to this.
referralInfo memory thisReferralInfo;
if (referralCode > REFERRAL_WITH_FEE_THRESHOLD) {
thisReferralInfo = referralLookup[referralCode];
}
// New scope
{
uint256 valueOut;
uint256 _swapMultiFee = swapMultiFee;
amountsOut = new uint256[](outputs.length);
...
}
We recommend being explicit in the code where possible and especially in situations like the above where there is no gas cost or code size optimization driving the coding style.
During the remediation phase of the audit, Odos added explicit scoping in commit e7f4c7af↗.