Consider erasing cache after completing deposit/withdraw/rebalance/compound operations

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXDeposit.sol#L148

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXDeposit.sol#L171-L181

Summary

I would suggest to always erase data, which was for an action already executed.

Vulnerability Details

We use a cache to store the arguments for an action, because of the two transactions pattern used by GMX and so in the second transaction we reference the cache from the first. However, best practice is to erase an object once we have finished with it.

Impact

As I could not find any path that could exploit this, I am rating it as low, but this could be a root cause with something else to abuse old data. And this could be prevented.

Tools Used

Manual Review

Recommendations

After the end of each of the actions that are using cache, delete this cache, so it is impossible to exploit old data in some creative way.