code-423n4 / 2021-11-nested-findings

1 stars 1 forks source link

setMaxAllowance #215

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

setMaxAllowance is not very efficient. functions _transferFeeWithRoyalty and _handleUnderSpending invoke setMaxAllowance which does the job but my assumption is that it is not very efficient even when the allowance is a bit lower than a max uint value, but sufficient, it will still re-approve for max. This happens when approving a fee splitter in the functions mentioned above. Usually, it is a good practice not to approve more when the amount is known (and in this case it is), but if you want to optimize for gas, a one-time approval for max will work in your case as I assume fee splitter is a trusted contract. My recommendation is that you replace setMaxAllowance (or introduce a new function) that also skips the approval process when the current allowance is sufficient.

adrien-supizet commented 2 years ago

this code will be removed in #34