R4R: Move validator cache to the keeper in stake #3075
Conversation
jackzampolin
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
|
I'm not a fan of this style of caching (not the fault of this PR, more the existing solution), I think it will get messy quickly. Moving the cache into the keeper makes it parallel-keeper-safe but otherwise doesn't help. I'd rather we find a way to do this generally with Amino (maybe through the codec object). cc @jaekwon Do you have any recommendations? This seems like a common desire for Amino clients. |
x/stake/keeper/validator.go
Outdated
| if validatorCacheList.Len() > 500 { | ||
| valToRemove := validatorCacheList.Remove(validatorCacheList.Front()).(cachedValidator) | ||
| delete(validatorCache, valToRemove.marshalled) | ||
| if k.validatorCacheList.Len() > 500 { |
There was a problem hiding this comment.
The 500 here and in the constructor should honestly be a constant imho.
|
@alexanderbez wasn't thinking this needed a pending update, also I've addressed comments here. @cwgoes do you want me to move forward with this, or should we drop it? |
Codecov Report
@@ Coverage Diff @@
## develop #3075 +/- ##
===========================================
+ Coverage 52.17% 52.18% +<.01%
===========================================
Files 136 136
Lines 9619 9621 +2
===========================================
+ Hits 5019 5021 +2
Misses 4263 4263
Partials 337 337 |
This doesn't make things worse, and it would help with parallel simulations, so for that reason I think we can merge it unless anyone else objects - approved. I've moved my concerns to a separate issue: #3088 |
Fixes: #3074
cc @cwgoes