Skip to content
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

feat: SWIP 21 #275

Merged
merged 43 commits into from
Dec 3, 2024
Merged

feat: SWIP 21 #275

merged 43 commits into from
Dec 3, 2024

Conversation

0xCardinalError
Copy link
Contributor

@0xCardinalError 0xCardinalError commented Sep 24, 2024

Implementation of SWIP on how to extend node storage capacity dedicated to the reserve to be able to calibrate operators' profitability.

More detail on this in SWIP writeup
ethersphere/SWIPs#56

@0xCardinalError 0xCardinalError changed the title feat: SWIP 56 feat: SWIP 21 Oct 1, 2024
…ork properly as well as isParticipatingInUpcomingRound function
@0xCardinalError 0xCardinalError marked this pull request as ready for review October 7, 2024 11:28
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to find some other occurrence when height is handled outside of calculateEffectiveStake but I did not find.
I thought that manageStake will do something with the stake based on the height value.

src/Staking.sol Show resolved Hide resolved
src/Staking.sol Show resolved Hide resolved
test/Redistribution.test.ts Outdated Show resolved Hide resolved
@nugaon
Copy link
Member

nugaon commented Oct 10, 2024

also, please add a test when a node could not play in the round with height 0

@0xCardinalError
Copy link
Contributor Author

0xCardinalError commented Oct 10, 2024

also, please add a test when a node could not play in the round with height 0

also, please add a test when a node could not play in the round with height 0

I tried to make something like this, The problem is that when you change height you are triggering "MustStake2Rounds" as it is affecting the staking struct and we require by default that any staking changes enforces node out of play for 2 rounds. which is not a problem really as it wont be playing in those 2 rounds anyway but its problem for testing as actually after 2 rounds anchor is different so we are not testing much other then if node can play on different anchor with higher height which we already did test in other test. For node not playing with height 0 we already have test from before where we didn't even have height and we just added default one now which is zero. So I did add one new test with changing height to the same node, first it fails, then we change height and wait and then it succeeded.

src/Redistribution.sol Show resolved Hide resolved
src/Redistribution.sol Show resolved Hide resolved
@nugaon
Copy link
Member

nugaon commented Oct 14, 2024

I tried to make something like this, The problem is that when you change height you are triggering "MustStake2Rounds" as it is affecting the staking struct and we require by default that any staking changes enforces node out of play for 2 rounds

Mine blocks until you get an anchor that satisfies being out of the node's proximity with the storageDepth, but it can play with height > 0.

@0xCardinalError
Copy link
Contributor Author

I tried to make something like this, The problem is that when you change height you are triggering "MustStake2Rounds" as it is affecting the staking struct and we require by default that any staking changes enforces node out of play for 2 rounds

Mine blocks until you get an anchor that satisfies being out of the node's proximity with the storageDepth, but it can play with height > 0.

yes, that test was added.

@0xCardinalError 0xCardinalError merged commit 9173bc3 into master Dec 3, 2024
1 check passed
@0xCardinalError 0xCardinalError deleted the swip-56 branch December 3, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants