Clean up candidates

action/protocol/context.go

@@ -120,6 +120,7 @@ type (
  StakingCorrectGas CheckFunc
  CalculateProbationList CheckFunc
  LoadCandidatesLegacy CheckFunc
+ CandCenterHasAlias CheckFunc
  }
 )
 
@@ -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)
+ },
  },
  )
 }

action/protocol/staking/bucket_pool_test.go

@@ -147,7 +147,7 @@ func TestBucketPool(t *testing.T) {
  }
 
  if v.commit {
- r.NoError(csm.Commit())
+ r.NoError(csm.Commit(false))
  // after commit, value should reflect in base view
  c, err = ConstructBaseView(sm)
  r.NoError(err)

action/protocol/staking/candidate_center.go

@@ -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
@@ -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
@@ -67,7 +69,7 @@ func NewCandidateCenter(all CandidateList) (*CandidateCenter, error) {
  return &c, nil
  }
 
- if err := c.Commit(); err != nil {
+ if err := c.Commit(true); err != nil {
  return nil, err
  }
  return &c, nil
@@ -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()
  }
@@ -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
@@ -137,8 +139,8 @@ 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)
+func (m *CandidateCenter) Commit(keepAliasBug bool) error {
+ size, err := m.base.commit(m.change, keepAliasBug)
  if err != nil {
  return err
  }
@@ -307,7 +309,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
  }
@@ -319,6 +321,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 {
@@ -387,6 +397,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
 }
@@ -400,13 +411,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
 //======================================
@@ -440,14 +444,20 @@ 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 {
+ for _, v := range change.candidates {
  if err := v.Validate(); err != nil {
  return 0, err
  }
  d := v.Clone()
+ if !keepAliasBug {
+ 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

action/protocol/staking/candidate_center_test.go

@@ -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"
 )
@@ -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)
 
@@ -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.Commit(true))
+ r.NoError(m.Commit(true)) // commit is idempotent
  r.Equal(size+increase, m.Size())
  // m equal to current list, not equal to old
  r.True(testEqual(m, list))
@@ -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.Commit(true))
  r.Equal(len(testCandidates), m.Size())
  r.True(testEqual(m, list))
  old := m.All()
@@ -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.Commit(true))
  r.Equal(len(list), m.Size())
  r.True(testEqual(m, list))
 
@@ -323,3 +324,111 @@ 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))
+ }
+ r.NoError(m.Commit(hasAlias))
+ 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()
+ {
+ r.NoError(center.Commit(hasAlias))
+ 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
+}

action/protocol/staking/candidate_statemanager.go

@@ -56,7 +56,7 @@ type (
  Upsert(*Candidate) error
  CreditBucketPool(*big.Int) error
  DebitBucketPool(*big.Int, bool) error
- Commit() error
+ Commit(bool) error
  SM() protocol.StateManager
  }
 
@@ -165,8 +165,8 @@ func (csm *candSM) DebitBucketPool(amount *big.Int, newBucket bool) error {
  return csm.bucketPool.DebitPool(csm, amount, newBucket)
 }
 
-func (csm *candSM) Commit() error {
- if err := csm.candCenter.Commit(); err != nil {
+func (csm *candSM) Commit(keepAlias bool) error {
+ if err := csm.candCenter.Commit(keepAlias); err != nil {
  return err
  }
 

action/protocol/staking/handlers.go

@@ -685,7 +685,7 @@ func (p *Protocol) handleCandidateRegister(ctx context.Context, act *action.Cand
  return log, nil, csmErrorToHandleError(owner.String(), err)
  }
  height, _ := csm.SM().Height()
- if p.needToWriteCandsMap(height) {
+ if p.needToWriteCandsMap(ctx, height) {
  csm.DirtyView().candCenter.base.recordOwner(c)
  }
 
@@ -768,7 +768,7 @@ func (p *Protocol) handleCandidateUpdate(ctx context.Context, act *action.Candid
  return log, csmErrorToHandleError(c.Owner.String(), err)
  }
  height, _ := csm.SM().Height()
- if p.needToWriteCandsMap(height) {
+ if p.needToWriteCandsMap(ctx, height) {
  csm.DirtyView().candCenter.base.recordOwner(c)
  }
 

action/protocol/staking/handlers_test.go

@@ -52,7 +52,17 @@ func (m *CandidateCenter) deleteForTestOnly(owner address.Address) {
  }
 
  if m.change.containsOwner(owner) {
- m.change.delete(owner)
+ if owner != nil {
+ candidates := []*Candidate{}
+ for _, c := range m.change.candidates {
+ if c.Owner.String() != owner.String() {
+ candidates = append(candidates, c)
+ }
+ }
+ m.change.candidates = candidates
+ delete(m.change.dirty, owner.String())
+ return
+ }
  m.size--
  }
 }