code-423n4 / 2022-01-yield-findings

1 stars 0 forks source link

approve Optimization #51

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0x1f8b

Vulnerability details

Impact

Gas saving.

Proof of Concept

The public method ConvexStakingWrapper.setApprovals is not required, also the first approve with 0 is not required, because is not possible to change curveToken, convexBooster, convexToken and convexPool, approving once it's enough, so there is no need to have this public method.

Tools Used

Manual review.

Recommended Mitigation Steps

Remove setApprovals and the IERC20(curveToken).approve(convexBooster, 0);

devtooligan commented 2 years ago

I'm not sure why the warden is saying setApprovals is not required.

As for the first approve with 0, I believe that is specifically required by the contract

GalloDaSballo commented 2 years ago

While the merits of a refactoring could be explored (but the warden didn't really do that), there can be gas savings by deleting the code, but that would happen at a loss of functionality.

For that reason am marking the finding invalid