-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
R4R: F1 storage efficiency improvements #3333
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3333 +/- ##
==========================================
- Coverage 54.9% 54.9% -0.01%
==========================================
Files 132 132
Lines 9680 9684 +4
==========================================
+ Hits 5315 5317 +2
- Misses 4028 4030 +2
Partials 337 337 |
To-do:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👌 ++ on the simulation storage invariant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that my biggest comment is I would like to see a paragraph in the "implementation" spec (even if it doesn't exist yet, a new temporary .md
file would suffice) describing the reward referencing counting mechanism from a high level.
Otherwise, code looks clean - have some general questions, some minor suggestions within.
Co-Authored-By: cwgoes <cwgoes@pluranimity.org>
@rigelrozanski Added a Markdown doc describing the rationale behind reference counting - bd0ebc8. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐️ 🚁 💾 🦅
Closes #3303
Makes the following changes to F1 to improve storage efficiency:
DelegatorStartingInfo
is deleted when a delegation's rewards are withdrawnF1 should now have storage usage ~=
O(D + S + V)
, whereD
is the number of current delegations to active validators,S
is the number of historical slashes (since the beginning of time) of active validators, andV
is the number of active validators (also, the incentives are in favor of withdrawing rewards and decreasing storage usage). This seems reasonable as we already store delegations & active validators and slashes are costly / rare. This storage usage is checked in a simulation invariant.Because of the way this is implemented, I'm not sure we need to store the reference count at all, but I suggest we leave it in for now until we're sure about that and sure we don't need to reference historical records for any other reason. Ref #3344.
Standard checklist:
docs/
)PENDING.md
with issue #Files changed
in the github PR explorerFor Admin Use: