Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Baseline bug fix (FIP-0081) #1557
Baseline bug fix (FIP-0081) #1557
Changes from 5 commits
102ee19
7b582ad
6da9f6a
e76fd96
ad45428
f4f11cd
63b7ecd
adf43f0
6825568
2d245ca
60d63d3
c19558e
1bd140e
a140b68
22cc3c8
330ef9d
06ff09c
bceeb2c
417f9e1
37c442d
252a358
8c969b4
e1836c0
7f09f47
1611afe
5c361a5
c5b2658
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
where can we write a unittest for the updated initial_pledge_for_power so that we can be more certain that the computation is what we expect?
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.
just add a new file in actors/miner/tests/, you'll see it already being tested for a particular case in actors/miner/tests/precommit_deposit_and_initial_pledge_positive_test.rs so you can copy that pattern
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.
If I recall our convo correctly @lemmih - this will be hardcoded for each network at the next upgrade?
I was probably not thinking about everything when we discussed this, but this is a value that will be updated every epoch. My thinking was that this value would be negative before the ramp was scheduled to be initiated, but update every epoch, kind of like a countdown timer. Once it is greater than 0, the code to update pledge according to the ramp will be activated. See updated line 276 of monies.rs
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.
So I think the main thing remaining is - where does the epochs_since_ramp_start parameter get updated? It seems there needs to be some way to access the current epoch of the network as well as a hardcoded value of the epoch at which the upgrade will take place.