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.

Sign up for GitHub

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

Jump to bottom

[staking] CandidateCenter Support for Missing and Changes of Self-Stake Bucket #4060

Merged
merged 4 commits into from
Jan 30, 2024
Merged

[staking] CandidateCenter Support for Missing and Changes of Self-Stake Bucket #4060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e9061b
candidate center support selfstake bucket missing and changes
envestcc Jan 18, 2024
4969516
add test
envestcc Jan 18, 2024
350baa3
address comments
envestcc Jan 22, 2024
7bc3b6d
address comments
envestcc Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion action/protocol/staking/candidate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ func (d *Candidate) Validate() error {
return nil
}

// isSelfStakeBucketSettled checks if self stake bucket is settled
func (d *Candidate) isSelfStakeBucketSettled() bool {
return d.SelfStakeBucketIdx != candidateNoSelfStakeBucketIndex
Copy link
Contributor

Choose a reason for hiding this comment

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

candidateNoSelfStakeBucketIndex is a special value?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it means candidate has not already been self-staked

Copy link
Member

Choose a reason for hiding this comment

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

rename to `_candidateNoSelfStakeBucketIndex``

Copy link
Member

Choose a reason for hiding this comment

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

what does it mean a self stake bucket is settled?

Copy link
Member Author

Choose a reason for hiding this comment

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

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).

}

// Collision checks collsion of 2 candidates
func (d *Candidate) Collision(c *Candidate) error {
if address.Equal(d.Owner, c.Owner) {
Expand All @@ -105,7 +110,7 @@ func (d *Candidate) Collision(c *Candidate) error {
if address.Equal(c.Operator, d.Operator) {
return ErrInvalidOperator
}
if c.SelfStakeBucketIdx == d.SelfStakeBucketIdx {
if c.SelfStakeBucketIdx == d.SelfStakeBucketIdx && c.isSelfStakeBucketSettled() {
return ErrInvalidSelfStkIndex
}
return nil
Expand Down
9 changes: 6 additions & 3 deletions action/protocol/staking/candidate_center.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func (cc *candChange) containsOperator(operator address.Address) bool {

func (cc *candChange) containsSelfStakingBucket(index uint64) bool {
for _, d := range cc.dirty {
if index == d.SelfStakeBucketIdx {
if d.isSelfStakeBucketSettled() && index == d.SelfStakeBucketIdx {
return true
}
}
Expand Down Expand Up @@ -398,7 +398,7 @@ func (cc *candChange) getByOwner(owner address.Address) *Candidate {

func (cc *candChange) getBySelfStakingIndex(index uint64) *Candidate {
for _, d := range cc.dirty {
if index == d.SelfStakeBucketIdx {
if d.isSelfStakeBucketSettled() && index == d.SelfStakeBucketIdx {
return d.Clone()
}
}
Expand Down Expand Up @@ -479,11 +479,14 @@ func (cb *candBase) commit(change *candChange, keepAliasBug bool) (int, error) {
if curr, ok := cb.ownerMap[d.Owner.String()]; ok {
delete(cb.nameMap, curr.Name)
delete(cb.operatorMap, curr.Operator.String())
delete(cb.selfStkBucketMap, curr.SelfStakeBucketIdx)
}
cb.ownerMap[d.Owner.String()] = d
cb.nameMap[d.Name] = d
cb.operatorMap[d.Operator.String()] = d
cb.selfStkBucketMap[d.SelfStakeBucketIdx] = d
if d.isSelfStakeBucketSettled() {
cb.selfStkBucketMap[d.SelfStakeBucketIdx] = d
}
}
}
return len(cb.ownerMap), nil
Expand Down
139 changes: 137 additions & 2 deletions action/protocol/staking/candidate_center_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (

// testEqual verifies m contains exactly the list
func testEqual(m *CandidateCenter, l CandidateList) bool {
if m.All().Len() != len(l) {
return false
}
for _, v := range l {
d := m.GetByOwner(v.Owner)
if d == nil {
Expand All @@ -33,10 +36,10 @@ func testEqual(m *CandidateCenter, l CandidateList) bool {
}

d = m.GetBySelfStakingIndex(v.SelfStakeBucketIdx)
if d == nil {
if d == nil && v.isSelfStakeBucketSettled() {
return false
}
if !v.Equal(d) {
if d != nil && !v.Equal(d) {
return false
}
}
Expand Down Expand Up @@ -429,6 +432,138 @@ func TestFixAlias(t *testing.T) {
}
}

func TestMultipleNonStakingCandidate(t *testing.T) {
r := require.New(t)

candStaked := &Candidate{
Owner: identityset.Address(1),
Operator: identityset.Address(11),
Reward: identityset.Address(3),
Name: "self-staked",
Votes: unit.ConvertIotxToRau(1200000),
SelfStakeBucketIdx: 1,
SelfStake: unit.ConvertIotxToRau(1200000),
}
candNonStaked1 := &Candidate{
Owner: identityset.Address(2),
Operator: identityset.Address(12),
Reward: identityset.Address(3),
Name: "non-self-staked1",
Votes: big.NewInt(0),
SelfStakeBucketIdx: candidateNoSelfStakeBucketIndex,
SelfStake: big.NewInt(0),
}
candNonStaked2 := &Candidate{
Owner: identityset.Address(3),
Operator: identityset.Address(13),
Reward: identityset.Address(3),
Name: "non-self-staked2",
Votes: big.NewInt(0),
SelfStakeBucketIdx: candidateNoSelfStakeBucketIndex,
SelfStake: big.NewInt(0),
}
candNonStaked1ColOwner := &Candidate{
Owner: identityset.Address(2),
Operator: identityset.Address(22),
Reward: identityset.Address(3),
Name: "non-self-staked1-col-owner",
Votes: big.NewInt(0),
SelfStakeBucketIdx: candidateNoSelfStakeBucketIndex,
SelfStake: big.NewInt(0),
}
candNonStaked1ColOpt := &Candidate{
Owner: identityset.Address(20),
Operator: identityset.Address(12),
Reward: identityset.Address(3),
Name: "non-self-staked1-col-opt",
Votes: big.NewInt(0),
SelfStakeBucketIdx: candidateNoSelfStakeBucketIndex,
SelfStake: big.NewInt(0),
}
candNonStaked1ColName := &Candidate{
Owner: identityset.Address(21),
Operator: identityset.Address(23),
Reward: identityset.Address(3),
Name: "non-self-staked1",
Votes: big.NewInt(0),
SelfStakeBucketIdx: candidateNoSelfStakeBucketIndex,
SelfStake: big.NewInt(0),
}
candStakedColBucket := &Candidate{
Owner: identityset.Address(4),
Operator: identityset.Address(14),
Reward: identityset.Address(3),
Name: "self-staked-col-bucket",
Votes: unit.ConvertIotxToRau(1200000),
SelfStakeBucketIdx: 1,
SelfStake: unit.ConvertIotxToRau(1200000),
}

checkCandidates := func(candcenter *CandidateCenter, cands []*Candidate) {
r.True(testEqual(candcenter, CandidateList(cands)))
// commit
r.NoError(candcenter.Commit())
r.True(testEqual(candcenter, CandidateList(cands)))
// from state manager
dk := protocol.NewDock()
view := protocol.View{}
r.NoError(view.Write(_protocolID, candcenter))
dk.Reset()
candcenter = candCenterFromNewCandidateStateManager(r, view, dk)
r.True(testEqual(candcenter, CandidateList(cands)))
}
t.Run("nonstaked candidate not collision on bucket", func(t *testing.T) {
candcenter, err := NewCandidateCenter(nil)
r.NoError(err)
Copy link
Member

@dustinxie dustinxie Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

add a valid candidate candStaked to candcenter

r.NoError(candcenter.Upsert(candNonStaked1))
r.NoError(candcenter.Upsert(candNonStaked2))
checkCandidates(candcenter, []*Candidate{candNonStaked1, candNonStaked2})
})
t.Run("staked candidate collision on bucket", func(t *testing.T) {
candcenter, err := NewCandidateCenter(nil)
r.NoError(err)
r.NoError(candcenter.Upsert(candStaked))
r.ErrorIs(candcenter.Upsert(candStakedColBucket), ErrInvalidSelfStkIndex)
checkCandidates(candcenter, []*Candidate{candStaked})
})
t.Run("nonstaked candidate collision on operator", func(t *testing.T) {
candcenter, err := NewCandidateCenter(nil)
r.NoError(err)
Copy link
Member

Choose a reason for hiding this comment

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

add a valid candidate candStaked to candcenter

r.NoError(candcenter.Upsert(candNonStaked1))
r.ErrorIs(candcenter.Upsert(candNonStaked1ColOpt), ErrInvalidOperator)
Copy link
Member

Choose a reason for hiding this comment

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

can combine the next test into here by just adding 1 line:
r.ErrorIs(candcenter.Upsert(candNonStaked1ColName), action.ErrInvalidCanName)

checkCandidates(candcenter, []*Candidate{candNonStaked1})
})
t.Run("nonstaked candidate collision on name", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

merge into previous test

candcenter, err := NewCandidateCenter(nil)
r.NoError(err)
r.NoError(candcenter.Upsert(candNonStaked1))
r.ErrorIs(candcenter.Upsert(candNonStaked1ColName), action.ErrInvalidCanName)
checkCandidates(candcenter, []*Candidate{candNonStaked1})
})
t.Run("nonstaked candidate update on owner", func(t *testing.T) {
candcenter, err := NewCandidateCenter(nil)
r.NoError(err)
r.NoError(candcenter.Upsert(candNonStaked1))
r.NoError(candcenter.Upsert(candNonStaked1ColOwner))
checkCandidates(candcenter, []*Candidate{candNonStaked1ColOwner})
})
t.Run("change bucket", func(t *testing.T) {
candcenter, err := NewCandidateCenter(nil)
r.NoError(err)
r.NoError(candcenter.Upsert(candNonStaked1))
// settle self-stake bucket
candStaked := candNonStaked1.Clone()
candStaked.SelfStakeBucketIdx = 1
candStaked.SelfStake = unit.ConvertIotxToRau(1200000)
r.NoError(candcenter.Upsert(candStaked))
// change self-stake bucket
candUpdated := candStaked.Clone()
candUpdated.SelfStakeBucketIdx = 2
r.NoError(candcenter.Upsert(candUpdated))
checkCandidates(candcenter, []*Candidate{candUpdated})
Copy link
Member

Choose a reason for hiding this comment

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

r.False(candcenter.ContainsSelfStakingBucket(candStaked.SelfStakeBucketIdx))
this is a good test line to keep

})
}

func candCenterFromNewCandidateStateManager(r *require.Assertions, view protocol.View, dk protocol.Dock) *CandidateCenter {
// get cand center: csm.ConstructBaseView
v, err := view.Read(_protocolID)
Expand Down
Loading