V2 update/gas optimization

[Kayanski]

This PR aims at adding a cache system for yield positions to avoid expensive queries.
We introduce 2 things :

1. Added mutable reference to positions in trait to be able to save cache when using the object for queries.
2. Added a stop to avoid storing that extra cache

This allows reducing gas price from 3.1M to 2.1M for an add-to-position operation

[Buckram123]

I feel like this type of cache gonna be very hard to manage and will spawn accidental bugs. From what I spotted with glance is autocompound doesn't reset position, meaning it won't notice new rewards until deposit/position update called, but I don't think bot should be allowed to call update (no check of sender there, but it looks like it should be admin method)

If I had to propose something:
What do you think about loading it at the start of every method that needs it and re-using for this method?
So for example you will have Strategy::load_osmosis_positions and it will return [(position_id, FullPositionBreakdown)] that you have to pass into all of the methods that can require FullPositionBreakdown. In the worst case you'll have to do some clones, but it should be cheaper than serialize/deserialize/store it completely and easier to manage

[Buckram123]

This query is expensive and serializing/deserializing/saving in state this structure is also expensive(it's pretty huge), so if we do cache it between calls I don't think we should be sending all of it on every message, but only information that we absolutely need for this execution.

[Kayanski]

Again, I'm NEVER saving the structure in state ! This field is just there to store the cache temporarily

[Buckram123]

Again, I'm NEVER saving the structure in state ! This field is just there to store the cache temporarily

Ah, I see, thanks for clearing that up. Although I would prefer to keep it separately so it doesn't get stored accidentally and no time or gas wasted for serializing/deserializing

[Kayanski]

If I make the storage structure private, it might solve the issue

[Buckram123]

If I make the storage structure private, it might solve the issue

You can make wrapper around Strategy that will have this field(or other useful fields for other protocols) like

StrategyFetched{
  strategy: Strategy,
  osmosis_positions: HashSet<position_id, FullPositionBreakDown>,
  // .. other useful info after fetching strategies
}

[Kayanski]

I feel like this type of cache gonna be very hard to manage and will spawn accidental bugs. From what I spotted with glance is autocompound doesn't reset position, meaning it won't notice new rewards until deposit/position update called, but I don't think bot should be allowed to call update (no check of sender there, but it looks like it should be admin method)

If I had to propose something: What do you think about loading it at the start of every method that needs it and re-using for this method? So for example you will have Strategy::load_osmosis_positions and it will return [(position_id, FullPositionBreakdown)] that you have to pass into all of the methods that can require FullPositionBreakdown. In the worst case you'll have to do some clones, but it should be cheaper than serialize/deserialize/store it completely and easier to manage

I'm not storing anything here ! It's just stored temporarily in the strategy object. The serialize/deserialize part is not that expensive either.