[staking] CandidateCenter Support for Missing and Changes of Self-Stake Bucket #4060
Conversation
envestcc
requested review from
CoderZhi,
Liuhaai and
dustinxie
as code owners
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4060 +/- ##
==========================================
+ Coverage 75.38% 76.20% +0.82%
==========================================
Files 303 338 +35
Lines 25923 28811 +2888
==========================================
+ Hits 19541 21956 +2415
- Misses 5360 5737 +377
- Partials 1022 1118 +96 ☔ View full report in Codecov by Sentry. |
envestcc
force-pushed
the
pr_collision
branch
2 times, most recently
from
05515a7 to
dd2b789
Compare
| checkCandidates(candcenter, cands) | ||
| // commit | ||
| r.NoError(candcenter.Commit()) | ||
| checkCandidates(candcenter, cands) |
There was a problem hiding this comment.
just use r.True(testEqual(candcenter, CandidateList(cands)))? adding this func does not re-use code or reduce code duplication
| // add candidates | ||
| r.NoError(candcenter.Upsert(candStaked)) | ||
| r.NoError(candcenter.Upsert(candNonStaked1)) | ||
| r.NoError(candcenter.Upsert(candNonStaked2)) |
There was a problem hiding this comment.
add more test cases:
- adding candidate with candNonStaked1/2's owner, name, operator will lead to collision
- move the update candidate test below here, can combine the 2 tests to reduce code duplication
There was a problem hiding this comment.
Okay, but Upsert will not collision on owner, updated instead
envestcc
force-pushed
the
pr_collision
branch
from
dd2b789 to
684e4b4
Compare
| r.False(candcenter.ContainsSelfStakingBucket(candStaked.SelfStakeBucketIdx)) | ||
| t.Run("nonstaked candidate not collision on bucket", func(t *testing.T) { | ||
| candcenter, err := NewCandidateCenter(nil) | ||
| r.NoError(err) |
There was a problem hiding this comment.
add a valid candidate candStaked to candcenter
| candUpdated := candStaked.Clone() | ||
| candUpdated.SelfStakeBucketIdx = 2 | ||
| r.NoError(candcenter.Upsert(candUpdated)) | ||
| checkCandidates(candcenter, []*Candidate{candUpdated}) |
There was a problem hiding this comment.
r.False(candcenter.ContainsSelfStakingBucket(candStaked.SelfStakeBucketIdx))
this is a good test line to keep
| }) | ||
| t.Run("nonstaked candidate collision on operator", func(t *testing.T) { | ||
| candcenter, err := NewCandidateCenter(nil) | ||
| r.NoError(err) |
There was a problem hiding this comment.
add a valid candidate candStaked to candcenter
| candcenter, err := NewCandidateCenter(nil) | ||
| r.NoError(err) | ||
| r.NoError(candcenter.Upsert(candNonStaked1)) | ||
| r.ErrorIs(candcenter.Upsert(candNonStaked1ColOpt), ErrInvalidOperator) |
There was a problem hiding this comment.
can combine the next test into here by just adding 1 line:
r.ErrorIs(candcenter.Upsert(candNonStaked1ColName), action.ErrInvalidCanName)
| r.ErrorIs(candcenter.Upsert(candNonStaked1ColOpt), ErrInvalidOperator) | ||
| checkCandidates(candcenter, []*Candidate{candNonStaked1}) | ||
| }) | ||
| t.Run("nonstaked candidate collision on name", func(t *testing.T) { |
There was a problem hiding this comment.
merge into previous test
|
|
||
| // isSelfStakeBucketSettled checks if self stake bucket is settled | ||
| func (d *Candidate) isSelfStakeBucketSettled() bool { | ||
| return d.SelfStakeBucketIdx != candidateNoSelfStakeBucketIndex |
There was a problem hiding this comment.
candidateNoSelfStakeBucketIndex is a special value?
There was a problem hiding this comment.
yes, it means candidate has not already been self-staked
There was a problem hiding this comment.
rename to `_candidateNoSelfStakeBucketIndex``
There was a problem hiding this comment.
what does it mean a self stake bucket is settled?
There was a problem hiding this comment.
This method is used to wrap the code d.SelfStakeBucketIdx != candidateNoSelfStakeBucketIndex to avoid making this judgement in multiple places.
Basically, it returns true in three cases:
- Candidate Activated: The candidate is in an active state and has a valid SelfStakeBucketIdx(the bucket can be self-owned or endorsed).
- Candidate Endorsement Expired: The endorsement of the candidate has expired (since the expiration of endorsement takes effect with delay, the SelfStakeBucketIdx will not be cleared before performing other operations on the bucket).
- Candidate Unstaked: The candidate has been unstaked (currently, the implementation of unstake does not clear the SelfStakeBucketIdx, but it might be more appropriate to clear it).
envestcc
force-pushed
the
pr_collision
branch
from
684e4b4 to
7bc3b6d
Compare
|
4 participants
Description
Due to the introduction of CandidateRegister without staking, there will occur candidate which lack staking and updates in self-staking bucket. However, the current
CandidateCenterdoes not support these operations. This PR aims to resolve these issues.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: