anza-xyz / agave

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://www.anza.xyz/
Apache License 2.0
178 stars 67 forks source link

feat: deserialize new epoch stakes bank snapshot field #1397

Closed jstarry closed 1 week ago

jstarry commented 2 weeks ago

Problem

Epoch stakes are not serialized into bank snapshots with the stake credits_observed field which is highly desired for implementing snapshot recovery of partitioned epoch rewards state.

Summary of Changes

Implements phase 1 of SIMD-0149 by deserializing the new epoch stakes map field from the bank snapshot if it exists. If the map is deserialized, it will be merged with the old epoch stakes field entries.

codecov-commenter commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 73.61111% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 82.7%. Comparing base (b711bda) to head (cf49ac2). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1397 +/- ## ========================================= - Coverage 82.7% 82.7% -0.1% ========================================= Files 871 871 Lines 370376 370438 +62 ========================================= - Hits 306561 306560 -1 - Misses 63815 63878 +63 ```
jstarry commented 1 week ago

I wasn't planning on backporting but could be nice actually. But we would definitely need the merging logic if we did backport because otherwise old versions would be deserializing but not picking up necessary epoch stakes entries from new snapshots.

jstarry commented 1 week ago

rebased to fix conflicts (just imports)

jstarry commented 1 week ago

Discussed with @brooksprumo and not planning to backport this.. phase 2 of the new field migration will be implemented for v2.1

CriesofCarrots commented 1 week ago

Discussed with @brooksprumo and not planning to backport this.. phase 2 of the new field migration will be implemented for v2.1

Sorry for not chiming in, but this is my preferred approach also 🤝

mergify[bot] commented 1 week ago

automerge label removed due to a CI failure

jstarry commented 1 week ago

Fixed a bad conflict resolution, can I get another approval?