Skip to content

Commit

Permalink
Clean up candidates (iotexproject#3696)
Browse files Browse the repository at this point in the history 
* [staking] fix name and operator map alias

* add test

* address comment

* address comment

* remove candMap

* revise candidates

* address comments

* address comment

Co-authored-by: Dustin Xie <[email protected]>
  • Loading branch information
@CoderZhi @dustinxie
CoderZhi and dustinxie authored Nov 29, 2022
1 parent 9db2242 commit c316b81
Show file tree
Hide file tree
Showing 13 changed files with 339 additions and 78 deletions.
4 changes: 4 additions & 0 deletions action/protocol/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ type (
StakingCorrectGas CheckFunc
CalculateProbationList CheckFunc
LoadCandidatesLegacy CheckFunc
CandCenterHasAlias CheckFunc
}
)

Expand Down Expand Up @@ -290,6 +291,9 @@ func WithFeatureWithHeightCtx(ctx context.Context) context.Context {
LoadCandidatesLegacy: func(height uint64) bool {
return !g.IsEaster(height)
},
CandCenterHasAlias: func(height uint64) bool {
return !g.IsOkhotsk(height)
},
},
)
}
Expand Down
5 changes: 4 additions & 1 deletion action/protocol/staking/bucket_pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package staking

import (
"bytes"
"context"
"math/big"
"testing"
"time"
Expand All @@ -16,6 +17,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/action/protocol"
"github.com/iotexproject/iotex-core/blockchain/genesis"
"github.com/iotexproject/iotex-core/state"
"github.com/iotexproject/iotex-core/test/identityset"
"github.com/iotexproject/iotex-core/testutil/testdb"
Expand Down Expand Up @@ -118,6 +120,7 @@ func TestBucketPool(t *testing.T) {
r.Equal(count, pool.Count())

var testGreenland bool
ctx := protocol.WithFeatureWithHeightCtx(genesis.WithGenesisContext(context.Background(), genesis.Default))
for _, v := range tests {
csm, err = NewCandidateStateManager(sm, v.postGreenland && testGreenland)
r.NoError(err)
Expand Down Expand Up @@ -147,7 +150,7 @@ func TestBucketPool(t *testing.T) {
}

if v.commit {
r.NoError(csm.Commit())
r.NoError(csm.Commit(ctx))
// after commit, value should reflect in base view
c, err = ConstructBaseView(sm)
r.NoError(err)
Expand Down
75 changes: 54 additions & 21 deletions action/protocol/staking/candidate_center.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import (
type (
// candChange captures the change to candidates
candChange struct {
dirty map[string]*Candidate
candidates []*Candidate
dirty map[string]*Candidate
}

// candBase is the confirmed base state
Expand Down Expand Up @@ -46,6 +47,7 @@ func listToCandChange(l CandidateList) (*candChange, error) {
if err := d.Validate(); err != nil {
return nil, err
}
cv.candidates = append(cv.candidates, d)
cv.dirty[d.Owner.String()] = d
}
return cv, nil
Expand Down Expand Up @@ -80,7 +82,7 @@ func (m *CandidateCenter) Size() int {

// All returns all candidates in candidate center
func (m *CandidateCenter) All() CandidateList {
list := m.change.all()
list := m.change.view()
if list == nil {
return m.base.all()
}
Expand All @@ -104,7 +106,7 @@ func (m CandidateCenter) Base() *CandidateCenter {

// Delta exports the pending changes
func (m *CandidateCenter) Delta() CandidateList {
return m.change.all()
return m.change.items()
}

// SetDelta sets the delta
Expand Down Expand Up @@ -138,7 +140,19 @@ func (m *CandidateCenter) SetDelta(l CandidateList) error {

// Commit writes the change into base
func (m *CandidateCenter) Commit() error {
size, err := m.base.commit(m.change)
size, err := m.base.commit(m.change, false)
if err != nil {
return err
}
m.size = size
m.change = nil
m.change = newCandChange()
return nil
}

// LegacyCommit writes the change into base with legacy logic
func (m *CandidateCenter) LegacyCommit() error {
size, err := m.base.commit(m.change, true)
if err != nil {
return err
}
Expand Down Expand Up @@ -307,7 +321,7 @@ func (cc *candChange) size() int {
return len(cc.dirty)
}

func (cc *candChange) all() CandidateList {
func (cc *candChange) view() CandidateList {
if len(cc.dirty) == 0 {
return nil
}
Expand All @@ -319,6 +333,14 @@ func (cc *candChange) all() CandidateList {
return list
}

func (cc *candChange) items() CandidateList {
var retval CandidateList
for _, c := range cc.candidates {
retval = append(retval, c.Clone())
}
return retval
}

func (cc *candChange) containsName(name string) bool {
for _, d := range cc.dirty {
if name == d.Name {
Expand Down Expand Up @@ -387,6 +409,7 @@ func (cc *candChange) upsert(d *Candidate) error {
if err := d.Validate(); err != nil {
return err
}
cc.candidates = append(cc.candidates, d)
cc.dirty[d.Owner.String()] = d
return nil
}
Expand All @@ -400,13 +423,6 @@ func (cc *candChange) collision(d *Candidate) error {
return nil
}

func (cc *candChange) delete(owner address.Address) {
if owner == nil {
return
}
delete(cc.dirty, owner.String())
}

//======================================
// candBase funcs
//======================================
Expand Down Expand Up @@ -440,18 +456,35 @@ func (cb *candBase) all() CandidateList {
return list
}

func (cb *candBase) commit(change *candChange) (int, error) {
func (cb *candBase) commit(change *candChange, keepAliasBug bool) (int, error) {
cb.lock.Lock()
defer cb.lock.Unlock()
for _, v := range change.dirty {
if err := v.Validate(); err != nil {
return 0, err
if keepAliasBug {
for _, v := range change.dirty {
if err := v.Validate(); err != nil {
return 0, err
}
d := v.Clone()
cb.ownerMap[d.Owner.String()] = d
cb.nameMap[d.Name] = d
cb.operatorMap[d.Operator.String()] = d
cb.selfStkBucketMap[d.SelfStakeBucketIdx] = d
}
} else {
for _, v := range change.candidates {
if err := v.Validate(); err != nil {
return 0, err
}
d := v.Clone()
if curr, ok := cb.ownerMap[d.Owner.String()]; ok {
delete(cb.nameMap, curr.Name)
delete(cb.operatorMap, curr.Operator.String())
}
cb.ownerMap[d.Owner.String()] = d
cb.nameMap[d.Name] = d
cb.operatorMap[d.Operator.String()] = d
cb.selfStkBucketMap[d.SelfStakeBucketIdx] = d
}
d := v.Clone()
cb.ownerMap[d.Owner.String()] = d
cb.nameMap[d.Name] = d
cb.operatorMap[d.Operator.String()] = d
cb.selfStkBucketMap[d.SelfStakeBucketIdx] = d
}
return len(cb.ownerMap), nil
}
Expand Down
127 changes: 122 additions & 5 deletions action/protocol/staking/candidate_center_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/iotexproject/iotex-core/action/protocol"
"github.com/iotexproject/iotex-core/pkg/unit"
"github.com/iotexproject/iotex-core/test/identityset"
)
Expand Down Expand Up @@ -54,8 +55,8 @@ func testEqualAllCommit(r *require.Assertions, m *CandidateCenter, old Candidate
r.NoError(err)

// number of changed cand = change
r.Equal(change, len(m.change.view()))
delta := m.Delta()
r.Equal(change, len(delta))
ser, err := delta.Serialize()
r.NoError(err)

Expand All @@ -70,8 +71,8 @@ func testEqualAllCommit(r *require.Assertions, m *CandidateCenter, old Candidate
// test commit
r.NoError(delta.Deserialize(ser))
r.NoError(m.SetDelta(delta))
r.NoError(m.Commit())
r.NoError(m.Commit()) // commit is idempotent
r.NoError(m.LegacyCommit())
r.NoError(m.LegacyCommit()) // commit is idempotent
r.Equal(size+increase, m.Size())
// m equal to current list, not equal to old
r.True(testEqual(m, list))
Expand Down Expand Up @@ -105,7 +106,7 @@ func TestCandCenter(t *testing.T) {
r.Equal(len(list), m.Size())
r.True(testEqual(m, list))
r.NoError(m.SetDelta(list))
r.NoError(m.Commit())
r.NoError(m.LegacyCommit())
r.Equal(len(testCandidates), m.Size())
r.True(testEqual(m, list))
old := m.All()
Expand Down Expand Up @@ -213,7 +214,7 @@ func TestCandCenter(t *testing.T) {
r.NoError(m.SetDelta(delta))
r.Equal(len(list), m.Size())
r.True(testEqual(m, list))
r.NoError(m.Commit())
r.NoError(m.LegacyCommit())
r.Equal(len(list), m.Size())
r.True(testEqual(m, list))

Expand Down Expand Up @@ -323,3 +324,119 @@ func TestCandCenter(t *testing.T) {
r.False(m.ContainsSelfStakingBucket(1000))
}
}

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

dk := protocol.NewDock()
view := protocol.View{}

for _, hasAlias := range []bool{false, true} {
// add 6 candidates into cand center
m, err := NewCandidateCenter(nil)
r.NoError(err)
for i, v := range testCandidates {
r.NoError(m.Upsert(testCandidates[i].d))
r.True(m.ContainsName(v.d.Name))
r.True(m.ContainsOperator(v.d.Operator))
r.Equal(v.d, m.GetByName(v.d.Name))
}
if hasAlias {
r.NoError(m.LegacyCommit())
} else {
r.NoError(m.Commit())
}
r.NoError(view.Write(_protocolID, m))

// simulate handleCandidateUpdate: update name
center := candCenterFromNewCandidateStateManager(r, view, dk)
name := testCandidates[0].d.Name
nameAlias := center.GetByName(name)
nameAlias.Equal(testCandidates[0].d)
nameAlias.Name = "break"
{
r.NoError(center.Upsert(nameAlias))
delta := center.Delta()
r.Equal(1, len(delta))
r.NoError(dk.Load(_protocolID, _stakingCandCenter, &delta))
}

center = candCenterFromNewCandidateStateManager(r, view, dk)
n := center.GetByName("break")
n.Equal(nameAlias)
r.True(center.ContainsName("break"))
// old name does not exist
r.Nil(center.GetByName(name))
r.False(center.ContainsName(name))

// simulate handleCandidateUpdate: update operator
op := testCandidates[1].d.Operator
opAlias := testCandidates[1].d.Clone()
opAlias.Operator = identityset.Address(17)
r.True(center.ContainsOperator(op))
r.False(center.ContainsOperator(opAlias.Operator))
{
r.NoError(center.Upsert(opAlias))
delta := center.Delta()
r.Equal(2, len(delta))
r.NoError(dk.Load(_protocolID, _stakingCandCenter, &delta))
}

// verify cand center with name/op alias
center = candCenterFromNewCandidateStateManager(r, view, dk)
n = center.GetByName("break")
n.Equal(nameAlias)
r.True(center.ContainsName("break"))
// old name does not exist
r.Nil(center.GetByName(name))
r.False(center.ContainsName(name))
n = center.GetByOwner(testCandidates[1].d.Owner)
n.Equal(opAlias)
r.True(center.ContainsOperator(opAlias.Operator))
// old operator does not exist
r.False(center.ContainsOperator(op))

// cand center Commit()
{
if hasAlias {
r.NoError(center.LegacyCommit())
} else {
r.NoError(center.Commit())
}
r.NoError(view.Write(_protocolID, center))
dk.Reset()
}

// verify cand center after Commit()
center = candCenterFromNewCandidateStateManager(r, view, dk)
n = center.GetByName("break")
n.Equal(nameAlias)
n = center.GetByOwner(testCandidates[1].d.Owner)
n.Equal(opAlias)
r.True(center.ContainsOperator(opAlias.Operator))
if !hasAlias {
r.Nil(center.GetByName(name))
r.False(center.ContainsName(name))
r.False(center.ContainsOperator(op))
} else {
// alias still exist in name/operator map
n = center.GetByName(name)
n.Equal(testCandidates[0].d)
r.True(center.ContainsName(name))
r.True(center.ContainsOperator(op))
}
}
}

func candCenterFromNewCandidateStateManager(r *require.Assertions, view protocol.View, dk protocol.Dock) *CandidateCenter {
// get cand center: csm.ConstructBaseView
v, err := view.Read(_protocolID)
r.NoError(err)
center := v.(*CandidateCenter).Base()
// get changes: csm.Sync()
delta := CandidateList{}
err = dk.Unload(_protocolID, _stakingCandCenter, &delta)
r.True(err == nil || err == protocol.ErrNoName)
r.NoError(center.SetDelta(delta))
return center
}
Loading

0 comments on commit c316b81

Please sign in to comment.