Beanstalk: The Finale

Line of code

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/silo/EnrootFacet.sol#L75

Summary

LibTractor._user() address should be cached instead of calling the function multiple times in one call.

Vulnerability Details

the LibTractor._user() function should be cached instead of calling the function multiple times in one call is often called more than once in a function instead of being called once and then caching that value. This will not make the code more readable, but will also save on gas.

Below we can observe 2 separate functions that call .user more than once instead of caching the value after calling it one time
snippets taken from the convert function linked below
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/silo/ConvertFacet.sol#L79

additionally the function below calls user function 4 times
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/silo/EnrootFacet.sol#L75

Impact

The continuous use of a function instead of caching the value the first time will lead to unoptimized code and will cost users more gas.

Tools Used

manual review

Recommendations

cache the value after calling the user function the first time.

a correct example that cache the user address is here

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Convert/LibConvert.sol#L451