From 6a02da4824ddfb30cd27333a4f78491a381f7e7f Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Fri, 21 Oct 2022 11:49:49 -0500 Subject: [PATCH 1/8] [staking] fix name and operator map alias --- action/protocol/context.go | 4 + action/protocol/staking/bucket_pool_test.go | 2 +- action/protocol/staking/candidate_center.go | 85 +++++++++++++++++-- .../staking/candidate_center_extra.go | 2 +- .../protocol/staking/candidate_statereader.go | 8 +- .../staking/candidate_statereader_test.go | 2 +- action/protocol/staking/handlers.go | 4 +- action/protocol/staking/protocol.go | 19 +++-- 8 files changed, 103 insertions(+), 23 deletions(-) diff --git a/action/protocol/context.go b/action/protocol/context.go index 6c70c566fa..594a107f90 100644 --- a/action/protocol/context.go +++ b/action/protocol/context.go @@ -120,6 +120,7 @@ type ( StakingCorrectGas CheckFunc CalculateProbationList CheckFunc LoadCandidatesLegacy CheckFunc + FixCandCenterAlias CheckFunc } ) @@ -290,6 +291,9 @@ func WithFeatureWithHeightCtx(ctx context.Context) context.Context { LoadCandidatesLegacy: func(height uint64) bool { return !g.IsEaster(height) }, + FixCandCenterAlias: func(height uint64) bool { + return g.IsOkhotsk(height) + }, }, ) } diff --git a/action/protocol/staking/bucket_pool_test.go b/action/protocol/staking/bucket_pool_test.go index fffc7cb7a7..8557b736c5 100644 --- a/action/protocol/staking/bucket_pool_test.go +++ b/action/protocol/staking/bucket_pool_test.go @@ -77,7 +77,7 @@ func TestBucketPool(t *testing.T) { r.NoError(err) } - view, _, err := CreateBaseView(sm, false) + view, _, err := CreateBaseView(sm, false, false) r.NoError(err) sm.WriteView(_protocolID, view) pool = view.bucketPool diff --git a/action/protocol/staking/candidate_center.go b/action/protocol/staking/candidate_center.go index 6b110b4c3e..a0d83a242e 100644 --- a/action/protocol/staking/candidate_center.go +++ b/action/protocol/staking/candidate_center.go @@ -20,6 +20,25 @@ type ( dirty map[string]*Candidate } + candMap interface { + size() int + all() CandidateList + commit(*candChange) (int, error) + getByName(string) (*Candidate, bool) + getByOwner(string) (*Candidate, bool) + getByOperator(string) (*Candidate, bool) + getBySelfStakingIndex(uint64) (*Candidate, bool) + collision(*Candidate) (address.Address, address.Address, address.Address) + delete(address.Address) + // below methods for manual patching the name/operator map + clone() candMap + candsInNameMap() CandidateList + candsInOperatorMap() CandidateList + ownersList() CandidateList + recordOwner(*Candidate) + loadNameOperatorMapOwnerList(CandidateList, CandidateList, CandidateList) error + } + // candBase is the confirmed base state candBase struct { lock sync.RWMutex @@ -30,14 +49,30 @@ type ( owners CandidateList } + // candBaseFixAlias is the base state, with alias fixed + candBaseFixAlias struct { + *candBase + } + // CandidateCenter is a struct to manage the candidates CandidateCenter struct { - base *candBase - size int - change *candChange + base candMap + size int + change *candChange + fixAlias bool } + + // CandidateCenterOption is an option + CandidateCenterOption func(*CandidateCenter) ) +// FixAliasOption sets to fix alias or not +func FixAliasOption(fixAlias bool) CandidateCenterOption { + return func(c *CandidateCenter) { + c.fixAlias = fixAlias + } +} + // listToCandChange creates a candChange from list func listToCandChange(l CandidateList) (*candChange, error) { cv := newCandChange() @@ -52,16 +87,23 @@ func listToCandChange(l CandidateList) (*candChange, error) { } // NewCandidateCenter creates an instance of CandidateCenter -func NewCandidateCenter(all CandidateList) (*CandidateCenter, error) { +func NewCandidateCenter(all CandidateList, opts ...CandidateCenterOption) (*CandidateCenter, error) { delta, err := listToCandChange(all) if err != nil { return nil, err } c := CandidateCenter{ - base: newCandBase(), change: delta, } + for _, opt := range opts { + opt(&c) + } + if c.fixAlias { + c.base = newCandBaseFixAlias() + } else { + c.base = newCandBase() + } if len(all) == 0 { return &c, nil @@ -97,7 +139,7 @@ func (m *CandidateCenter) All() CandidateList { func (m CandidateCenter) Base() *CandidateCenter { return &CandidateCenter{ base: m.base, - size: len(m.base.ownerMap), + size: m.base.size(), change: newCandChange(), } } @@ -512,3 +554,34 @@ func (cb *candBase) delete(owner address.Address) { delete(cb.selfStkBucketMap, d.SelfStakeBucketIdx) } } + +//====================================== +// candBaseFixAlias funcs +//====================================== + +func newCandBaseFixAlias() *candBaseFixAlias { + return &candBaseFixAlias{ + candBase: newCandBase(), + } +} + +func (cb *candBaseFixAlias) commit(change *candChange) (int, error) { + cb.lock.Lock() + defer cb.lock.Unlock() + for _, v := range change.dirty { + if err := v.Validate(); err != nil { + return 0, err + } + d := v.Clone() + if curr, existing := cb.ownerMap[d.Owner.String()]; existing { + // this is an existing candidate, remove it from name/operator map + 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 + } + return len(cb.ownerMap), nil +} diff --git a/action/protocol/staking/candidate_center_extra.go b/action/protocol/staking/candidate_center_extra.go index 30e9554dd1..c5b3d448d7 100644 --- a/action/protocol/staking/candidate_center_extra.go +++ b/action/protocol/staking/candidate_center_extra.go @@ -6,7 +6,7 @@ package staking -func (cb *candBase) clone() *candBase { +func (cb *candBase) clone() candMap { cb.lock.RLock() defer cb.lock.RUnlock() clone := newCandBase() diff --git a/action/protocol/staking/candidate_statereader.go b/action/protocol/staking/candidate_statereader.go index 93faa954de..65ff614290 100644 --- a/action/protocol/staking/candidate_statereader.go +++ b/action/protocol/staking/candidate_statereader.go @@ -120,12 +120,12 @@ func (c *candSR) ActiveBucketsCount() uint64 { } // GetStakingStateReader returns a candidate state reader that reflects the base view -func GetStakingStateReader(sr protocol.StateReader) (CandidateStateReader, error) { +func GetStakingStateReader(sr protocol.StateReader, fixCandCenterAlias bool) (CandidateStateReader, error) { c, err := ConstructBaseView(sr) if err != nil { if errors.Cause(err) == protocol.ErrNoName { // the view does not exist yet, create it - view, height, err := CreateBaseView(sr, true) + view, height, err := CreateBaseView(sr, true, fixCandCenterAlias) if err != nil { return nil, err } @@ -172,7 +172,7 @@ func ConstructBaseView(sr protocol.StateReader) (CandidateStateReader, error) { } // CreateBaseView creates the base view from state reader -func CreateBaseView(sr protocol.StateReader, enableSMStorage bool) (*ViewData, uint64, error) { +func CreateBaseView(sr protocol.StateReader, enableSMStorage, fixCandCenterAlias bool) (*ViewData, uint64, error) { if sr == nil { return nil, 0, ErrMissingField } @@ -183,7 +183,7 @@ func CreateBaseView(sr protocol.StateReader, enableSMStorage bool) (*ViewData, u return nil, height, err } - center, err := NewCandidateCenter(all) + center, err := NewCandidateCenter(all, FixAliasOption(fixCandCenterAlias)) if err != nil { return nil, height, err } diff --git a/action/protocol/staking/candidate_statereader_test.go b/action/protocol/staking/candidate_statereader_test.go index 25d24fdda0..57f8636493 100644 --- a/action/protocol/staking/candidate_statereader_test.go +++ b/action/protocol/staking/candidate_statereader_test.go @@ -20,7 +20,7 @@ func Test_CandidateStateReader(t *testing.T) { ctrl := gomock.NewController(t) sm := testdb.NewMockStateManager(ctrl) - csr, err := GetStakingStateReader(sm) + csr, err := GetStakingStateReader(sm, false) require.NoError(err) h, err := sm.Height() diff --git a/action/protocol/staking/handlers.go b/action/protocol/staking/handlers.go index 6e662e3556..9a16245057 100644 --- a/action/protocol/staking/handlers.go +++ b/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) } diff --git a/action/protocol/staking/protocol.go b/action/protocol/staking/protocol.go index 8836066000..699346e1ca 100644 --- a/action/protocol/staking/protocol.go +++ b/action/protocol/staking/protocol.go @@ -173,12 +173,12 @@ func (p *Protocol) Start(ctx context.Context, sr protocol.StateReader) (interfac } // load view from SR - c, _, err := CreateBaseView(sr, featureCtx.ReadStateFromDB(height)) + c, _, err := CreateBaseView(sr, featureCtx.ReadStateFromDB(height), featureCtx.FixCandCenterAlias(height)) if err != nil { return nil, errors.Wrap(err, "failed to start staking protocol") } - if p.needToReadCandsMap(height) { + if p.needToReadCandsMap(ctx, height) { name, operator, owners, err := readCandCenterStateFromStateDB(sr, height) if err != nil { // stateDB does not have name/operator map yet @@ -323,7 +323,7 @@ func (p *Protocol) PreCommit(ctx context.Context, sm protocol.StateManager) erro if err != nil { return err } - if !p.needToWriteCandsMap(height) { + if !p.needToWriteCandsMap(ctx, height) { return nil } @@ -458,7 +458,8 @@ func (p *Protocol) Validate(ctx context.Context, act action.Action, sr protocol. // ActiveCandidates returns all active candidates in candidate center func (p *Protocol) ActiveCandidates(ctx context.Context, sr protocol.StateReader, height uint64) (state.CandidateList, error) { - c, err := GetStakingStateReader(sr) + featureCtx := protocol.MustGetFeatureWithHeightCtx(ctx) + c, err := GetStakingStateReader(sr, featureCtx.FixCandCenterAlias(height)) if err != nil { return nil, errors.Wrap(err, "failed to get ActiveCandidates") } @@ -601,12 +602,14 @@ func (p *Protocol) settleAction( return &r, nil } -func (p *Protocol) needToReadCandsMap(height uint64) bool { - return height > p.config.PersistStakingPatchBlock +func (p *Protocol) needToReadCandsMap(ctx context.Context, height uint64) bool { + fCtx := protocol.MustGetFeatureWithHeightCtx(ctx) + return height > p.config.PersistStakingPatchBlock && !fCtx.FixCandCenterAlias(height) } -func (p *Protocol) needToWriteCandsMap(height uint64) bool { - return height >= p.config.PersistStakingPatchBlock +func (p *Protocol) needToWriteCandsMap(ctx context.Context, height uint64) bool { + fCtx := protocol.MustGetFeatureWithHeightCtx(ctx) + return height >= p.config.PersistStakingPatchBlock && !fCtx.FixCandCenterAlias(height) } func readCandCenterStateFromStateDB(sr protocol.StateReader, height uint64) (CandidateList, CandidateList, CandidateList, error) { From d72ffecfcc3f080a02f5ea5abf79a408db5c0742 Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Wed, 26 Oct 2022 15:54:48 -0500 Subject: [PATCH 2/8] add test --- .../protocol/staking/candidate_center_test.go | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/action/protocol/staking/candidate_center_test.go b/action/protocol/staking/candidate_center_test.go index 7d877b3efe..e33da480b5 100644 --- a/action/protocol/staking/candidate_center_test.go +++ b/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" ) @@ -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 _, fixAlias := range []bool{false, true} { + // add 6 candidates into cand center + m, err := NewCandidateCenter(nil, FixAliasOption(fixAlias)) + 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()) + 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()) + 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 fixAlias { + 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 +} From ad06fe91bc1427f5b407f3364cdc04d889564539 Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Thu, 10 Nov 2022 17:54:06 -0600 Subject: [PATCH 3/8] address comment --- action/protocol/staking/protocol.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/action/protocol/staking/protocol.go b/action/protocol/staking/protocol.go index 699346e1ca..d28eeaceb9 100644 --- a/action/protocol/staking/protocol.go +++ b/action/protocol/staking/protocol.go @@ -458,8 +458,7 @@ func (p *Protocol) Validate(ctx context.Context, act action.Action, sr protocol. // ActiveCandidates returns all active candidates in candidate center func (p *Protocol) ActiveCandidates(ctx context.Context, sr protocol.StateReader, height uint64) (state.CandidateList, error) { - featureCtx := protocol.MustGetFeatureWithHeightCtx(ctx) - c, err := GetStakingStateReader(sr, featureCtx.FixCandCenterAlias(height)) + c, err := GetStakingStateReader(sr, true) if err != nil { return nil, errors.Wrap(err, "failed to get ActiveCandidates") } From 1a892d7f34fbe2e614ad6f4c4df41321fed47613 Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Fri, 21 Oct 2022 11:49:49 -0500 Subject: [PATCH 4/8] address comment --- action/protocol/context.go | 6 +++--- action/protocol/staking/candidate_center.go | 14 +++++++------- action/protocol/staking/candidate_center_test.go | 8 ++++---- action/protocol/staking/candidate_statereader.go | 8 ++++---- action/protocol/staking/protocol.go | 8 ++++---- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/action/protocol/context.go b/action/protocol/context.go index 594a107f90..7c27137b42 100644 --- a/action/protocol/context.go +++ b/action/protocol/context.go @@ -120,7 +120,7 @@ type ( StakingCorrectGas CheckFunc CalculateProbationList CheckFunc LoadCandidatesLegacy CheckFunc - FixCandCenterAlias CheckFunc + CandCenterHasAlias CheckFunc } ) @@ -291,8 +291,8 @@ func WithFeatureWithHeightCtx(ctx context.Context) context.Context { LoadCandidatesLegacy: func(height uint64) bool { return !g.IsEaster(height) }, - FixCandCenterAlias: func(height uint64) bool { - return g.IsOkhotsk(height) + CandCenterHasAlias: func(height uint64) bool { + return !g.IsOkhotsk(height) }, }, ) diff --git a/action/protocol/staking/candidate_center.go b/action/protocol/staking/candidate_center.go index a0d83a242e..e2f19b6960 100644 --- a/action/protocol/staking/candidate_center.go +++ b/action/protocol/staking/candidate_center.go @@ -59,17 +59,17 @@ type ( base candMap size int change *candChange - fixAlias bool + hasAlias bool } // CandidateCenterOption is an option CandidateCenterOption func(*CandidateCenter) ) -// FixAliasOption sets to fix alias or not -func FixAliasOption(fixAlias bool) CandidateCenterOption { +// HasAliasOption indicates the candidate center has alias +func HasAliasOption(hasAlias bool) CandidateCenterOption { return func(c *CandidateCenter) { - c.fixAlias = fixAlias + c.hasAlias = hasAlias } } @@ -99,10 +99,10 @@ func NewCandidateCenter(all CandidateList, opts ...CandidateCenterOption) (*Cand for _, opt := range opts { opt(&c) } - if c.fixAlias { - c.base = newCandBaseFixAlias() - } else { + if c.hasAlias { c.base = newCandBase() + } else { + c.base = newCandBaseFixAlias() } if len(all) == 0 { diff --git a/action/protocol/staking/candidate_center_test.go b/action/protocol/staking/candidate_center_test.go index e33da480b5..0d5f3a5bef 100644 --- a/action/protocol/staking/candidate_center_test.go +++ b/action/protocol/staking/candidate_center_test.go @@ -91,7 +91,7 @@ func testEqualAllCommit(r *require.Assertions, m *CandidateCenter, old Candidate func TestCandCenter(t *testing.T) { r := require.New(t) - m, err := NewCandidateCenter(nil) + m, err := NewCandidateCenter(nil, HasAliasOption(true)) r.NoError(err) for i, v := range testCandidates { r.NoError(m.Upsert(testCandidates[i].d)) @@ -331,9 +331,9 @@ func TestFixAlias(t *testing.T) { dk := protocol.NewDock() view := protocol.View{} - for _, fixAlias := range []bool{false, true} { + for _, hasAlias := range []bool{false, true} { // add 6 candidates into cand center - m, err := NewCandidateCenter(nil, FixAliasOption(fixAlias)) + m, err := NewCandidateCenter(nil, HasAliasOption(hasAlias)) r.NoError(err) for i, v := range testCandidates { r.NoError(m.Upsert(testCandidates[i].d)) @@ -406,7 +406,7 @@ func TestFixAlias(t *testing.T) { n = center.GetByOwner(testCandidates[1].d.Owner) n.Equal(opAlias) r.True(center.ContainsOperator(opAlias.Operator)) - if fixAlias { + if !hasAlias { r.Nil(center.GetByName(name)) r.False(center.ContainsName(name)) r.False(center.ContainsOperator(op)) diff --git a/action/protocol/staking/candidate_statereader.go b/action/protocol/staking/candidate_statereader.go index 65ff614290..61f58b77ec 100644 --- a/action/protocol/staking/candidate_statereader.go +++ b/action/protocol/staking/candidate_statereader.go @@ -120,12 +120,12 @@ func (c *candSR) ActiveBucketsCount() uint64 { } // GetStakingStateReader returns a candidate state reader that reflects the base view -func GetStakingStateReader(sr protocol.StateReader, fixCandCenterAlias bool) (CandidateStateReader, error) { +func GetStakingStateReader(sr protocol.StateReader, candCenterHasAlias bool) (CandidateStateReader, error) { c, err := ConstructBaseView(sr) if err != nil { if errors.Cause(err) == protocol.ErrNoName { // the view does not exist yet, create it - view, height, err := CreateBaseView(sr, true, fixCandCenterAlias) + view, height, err := CreateBaseView(sr, true, candCenterHasAlias) if err != nil { return nil, err } @@ -172,7 +172,7 @@ func ConstructBaseView(sr protocol.StateReader) (CandidateStateReader, error) { } // CreateBaseView creates the base view from state reader -func CreateBaseView(sr protocol.StateReader, enableSMStorage, fixCandCenterAlias bool) (*ViewData, uint64, error) { +func CreateBaseView(sr protocol.StateReader, enableSMStorage, candCenterHasAlias bool) (*ViewData, uint64, error) { if sr == nil { return nil, 0, ErrMissingField } @@ -183,7 +183,7 @@ func CreateBaseView(sr protocol.StateReader, enableSMStorage, fixCandCenterAlias return nil, height, err } - center, err := NewCandidateCenter(all, FixAliasOption(fixCandCenterAlias)) + center, err := NewCandidateCenter(all, HasAliasOption(candCenterHasAlias)) if err != nil { return nil, height, err } diff --git a/action/protocol/staking/protocol.go b/action/protocol/staking/protocol.go index d28eeaceb9..671109d7ac 100644 --- a/action/protocol/staking/protocol.go +++ b/action/protocol/staking/protocol.go @@ -173,7 +173,7 @@ func (p *Protocol) Start(ctx context.Context, sr protocol.StateReader) (interfac } // load view from SR - c, _, err := CreateBaseView(sr, featureCtx.ReadStateFromDB(height), featureCtx.FixCandCenterAlias(height)) + c, _, err := CreateBaseView(sr, featureCtx.ReadStateFromDB(height), featureCtx.CandCenterHasAlias(height)) if err != nil { return nil, errors.Wrap(err, "failed to start staking protocol") } @@ -458,7 +458,7 @@ func (p *Protocol) Validate(ctx context.Context, act action.Action, sr protocol. // ActiveCandidates returns all active candidates in candidate center func (p *Protocol) ActiveCandidates(ctx context.Context, sr protocol.StateReader, height uint64) (state.CandidateList, error) { - c, err := GetStakingStateReader(sr, true) + c, err := GetStakingStateReader(sr, false) if err != nil { return nil, errors.Wrap(err, "failed to get ActiveCandidates") } @@ -603,12 +603,12 @@ func (p *Protocol) settleAction( func (p *Protocol) needToReadCandsMap(ctx context.Context, height uint64) bool { fCtx := protocol.MustGetFeatureWithHeightCtx(ctx) - return height > p.config.PersistStakingPatchBlock && !fCtx.FixCandCenterAlias(height) + return height > p.config.PersistStakingPatchBlock && fCtx.CandCenterHasAlias(height) } func (p *Protocol) needToWriteCandsMap(ctx context.Context, height uint64) bool { fCtx := protocol.MustGetFeatureWithHeightCtx(ctx) - return height >= p.config.PersistStakingPatchBlock && !fCtx.FixCandCenterAlias(height) + return height >= p.config.PersistStakingPatchBlock && fCtx.CandCenterHasAlias(height) } func readCandCenterStateFromStateDB(sr protocol.StateReader, height uint64) (CandidateList, CandidateList, CandidateList, error) { From 4940b24105ed862a821c43f6c6a44a1735b1f986 Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Wed, 16 Nov 2022 16:44:47 -0600 Subject: [PATCH 5/8] remove candMap --- action/protocol/staking/candidate_center.go | 72 +++---------------- .../staking/candidate_center_extra.go | 4 +- 2 files changed, 12 insertions(+), 64 deletions(-) diff --git a/action/protocol/staking/candidate_center.go b/action/protocol/staking/candidate_center.go index e2f19b6960..f0b5365441 100644 --- a/action/protocol/staking/candidate_center.go +++ b/action/protocol/staking/candidate_center.go @@ -20,25 +20,6 @@ type ( dirty map[string]*Candidate } - candMap interface { - size() int - all() CandidateList - commit(*candChange) (int, error) - getByName(string) (*Candidate, bool) - getByOwner(string) (*Candidate, bool) - getByOperator(string) (*Candidate, bool) - getBySelfStakingIndex(uint64) (*Candidate, bool) - collision(*Candidate) (address.Address, address.Address, address.Address) - delete(address.Address) - // below methods for manual patching the name/operator map - clone() candMap - candsInNameMap() CandidateList - candsInOperatorMap() CandidateList - ownersList() CandidateList - recordOwner(*Candidate) - loadNameOperatorMapOwnerList(CandidateList, CandidateList, CandidateList) error - } - // candBase is the confirmed base state candBase struct { lock sync.RWMutex @@ -47,16 +28,12 @@ type ( operatorMap map[string]*Candidate selfStkBucketMap map[uint64]*Candidate owners CandidateList - } - - // candBaseFixAlias is the base state, with alias fixed - candBaseFixAlias struct { - *candBase + hasAlias bool } // CandidateCenter is a struct to manage the candidates CandidateCenter struct { - base candMap + base *candBase size int change *candChange hasAlias bool @@ -99,11 +76,7 @@ func NewCandidateCenter(all CandidateList, opts ...CandidateCenterOption) (*Cand for _, opt := range opts { opt(&c) } - if c.hasAlias { - c.base = newCandBase() - } else { - c.base = newCandBaseFixAlias() - } + c.base = newCandBase(c.hasAlias) if len(all) == 0 { return &c, nil @@ -453,12 +426,13 @@ func (cc *candChange) delete(owner address.Address) { // candBase funcs //====================================== -func newCandBase() *candBase { +func newCandBase(hasAlias bool) *candBase { return &candBase{ nameMap: make(map[string]*Candidate), ownerMap: make(map[string]*Candidate), operatorMap: make(map[string]*Candidate), selfStkBucketMap: make(map[uint64]*Candidate), + hasAlias: hasAlias, } } @@ -490,6 +464,11 @@ func (cb *candBase) commit(change *candChange) (int, error) { return 0, err } d := v.Clone() + if curr, existing := cb.ownerMap[d.Owner.String()]; !cb.hasAlias && existing { + // this is an existing candidate, remove it from name/operator map + 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 @@ -554,34 +533,3 @@ func (cb *candBase) delete(owner address.Address) { delete(cb.selfStkBucketMap, d.SelfStakeBucketIdx) } } - -//====================================== -// candBaseFixAlias funcs -//====================================== - -func newCandBaseFixAlias() *candBaseFixAlias { - return &candBaseFixAlias{ - candBase: newCandBase(), - } -} - -func (cb *candBaseFixAlias) commit(change *candChange) (int, error) { - cb.lock.Lock() - defer cb.lock.Unlock() - for _, v := range change.dirty { - if err := v.Validate(); err != nil { - return 0, err - } - d := v.Clone() - if curr, existing := cb.ownerMap[d.Owner.String()]; existing { - // this is an existing candidate, remove it from name/operator map - 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 - } - return len(cb.ownerMap), nil -} diff --git a/action/protocol/staking/candidate_center_extra.go b/action/protocol/staking/candidate_center_extra.go index c5b3d448d7..bcf4f90f56 100644 --- a/action/protocol/staking/candidate_center_extra.go +++ b/action/protocol/staking/candidate_center_extra.go @@ -6,10 +6,10 @@ package staking -func (cb *candBase) clone() candMap { +func (cb *candBase) clone() *candBase { cb.lock.RLock() defer cb.lock.RUnlock() - clone := newCandBase() + clone := newCandBase(cb.hasAlias) for name, cand := range cb.nameMap { clone.nameMap[name] = cand.Clone() } From cbc509d6cdfb5afbda141de6c404d733d9a4a963 Mon Sep 17 00:00:00 2001 From: Dustin Xie Date: Fri, 21 Oct 2022 11:49:49 -0500 Subject: [PATCH 6/8] revise candidates --- action/protocol/staking/bucket_pool_test.go | 4 +- action/protocol/staking/candidate_center.go | 75 ++++++++--------- .../staking/candidate_center_extra.go | 2 +- .../protocol/staking/candidate_center_test.go | 18 ++-- .../staking/candidate_statemanager.go | 6 +- .../protocol/staking/candidate_statereader.go | 4 +- .../staking/candidate_statereader_test.go | 2 +- action/protocol/staking/handlers_test.go | 12 ++- action/protocol/staking/protocol.go | 23 ++++-- action/protocol/staking/protocol_test.go | 6 +- action/protocol/staking/vote_reviser.go | 82 ++++++++++++++----- action/protocol/staking/vote_reviser_test.go | 60 +++++++++++--- chainservice/builder.go | 1 + 13 files changed, 192 insertions(+), 103 deletions(-) diff --git a/action/protocol/staking/bucket_pool_test.go b/action/protocol/staking/bucket_pool_test.go index 8557b736c5..2372230d1b 100644 --- a/action/protocol/staking/bucket_pool_test.go +++ b/action/protocol/staking/bucket_pool_test.go @@ -77,7 +77,7 @@ func TestBucketPool(t *testing.T) { r.NoError(err) } - view, _, err := CreateBaseView(sm, false, false) + view, _, err := CreateBaseView(sm, false) r.NoError(err) sm.WriteView(_protocolID, view) pool = view.bucketPool @@ -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) diff --git a/action/protocol/staking/candidate_center.go b/action/protocol/staking/candidate_center.go index f0b5365441..879bd4c910 100644 --- a/action/protocol/staking/candidate_center.go +++ b/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 @@ -28,28 +29,16 @@ type ( operatorMap map[string]*Candidate selfStkBucketMap map[uint64]*Candidate owners CandidateList - hasAlias bool } // CandidateCenter is a struct to manage the candidates CandidateCenter struct { - base *candBase - size int - change *candChange - hasAlias bool + base *candBase + size int + change *candChange } - - // CandidateCenterOption is an option - CandidateCenterOption func(*CandidateCenter) ) -// HasAliasOption indicates the candidate center has alias -func HasAliasOption(hasAlias bool) CandidateCenterOption { - return func(c *CandidateCenter) { - c.hasAlias = hasAlias - } -} - // listToCandChange creates a candChange from list func listToCandChange(l CandidateList) (*candChange, error) { cv := newCandChange() @@ -58,31 +47,29 @@ 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 } // NewCandidateCenter creates an instance of CandidateCenter -func NewCandidateCenter(all CandidateList, opts ...CandidateCenterOption) (*CandidateCenter, error) { +func NewCandidateCenter(all CandidateList) (*CandidateCenter, error) { delta, err := listToCandChange(all) if err != nil { return nil, err } c := CandidateCenter{ + base: newCandBase(), change: delta, } - for _, opt := range opts { - opt(&c) - } - c.base = newCandBase(c.hasAlias) if len(all) == 0 { return &c, nil } - if err := c.Commit(); err != nil { + if err := c.Commit(true); err != nil { return nil, err } return &c, nil @@ -95,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() } @@ -112,14 +99,14 @@ func (m *CandidateCenter) All() CandidateList { func (m CandidateCenter) Base() *CandidateCenter { return &CandidateCenter{ base: m.base, - size: m.base.size(), + size: len(m.base.ownerMap), change: newCandChange(), } } // Delta exports the pending changes func (m *CandidateCenter) Delta() CandidateList { - return m.change.all() + return m.change.items() } // SetDelta sets the delta @@ -152,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 } @@ -322,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 } @@ -334,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 { @@ -402,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 } @@ -415,24 +411,16 @@ 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 //====================================== -func newCandBase(hasAlias bool) *candBase { +func newCandBase() *candBase { return &candBase{ nameMap: make(map[string]*Candidate), ownerMap: make(map[string]*Candidate), operatorMap: make(map[string]*Candidate), selfStkBucketMap: make(map[uint64]*Candidate), - hasAlias: hasAlias, } } @@ -456,18 +444,19 @@ 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 curr, existing := cb.ownerMap[d.Owner.String()]; !cb.hasAlias && existing { - // this is an existing candidate, remove it from name/operator map - delete(cb.nameMap, curr.Name) - delete(cb.operatorMap, curr.Operator.String()) + 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 diff --git a/action/protocol/staking/candidate_center_extra.go b/action/protocol/staking/candidate_center_extra.go index bcf4f90f56..30e9554dd1 100644 --- a/action/protocol/staking/candidate_center_extra.go +++ b/action/protocol/staking/candidate_center_extra.go @@ -9,7 +9,7 @@ package staking func (cb *candBase) clone() *candBase { cb.lock.RLock() defer cb.lock.RUnlock() - clone := newCandBase(cb.hasAlias) + clone := newCandBase() for name, cand := range cb.nameMap { clone.nameMap[name] = cand.Clone() } diff --git a/action/protocol/staking/candidate_center_test.go b/action/protocol/staking/candidate_center_test.go index 0d5f3a5bef..7b420b8bef 100644 --- a/action/protocol/staking/candidate_center_test.go +++ b/action/protocol/staking/candidate_center_test.go @@ -55,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) @@ -71,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)) @@ -91,7 +91,7 @@ func testEqualAllCommit(r *require.Assertions, m *CandidateCenter, old Candidate func TestCandCenter(t *testing.T) { r := require.New(t) - m, err := NewCandidateCenter(nil, HasAliasOption(true)) + m, err := NewCandidateCenter(nil) r.NoError(err) for i, v := range testCandidates { r.NoError(m.Upsert(testCandidates[i].d)) @@ -106,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() @@ -214,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)) @@ -333,7 +333,7 @@ func TestFixAlias(t *testing.T) { for _, hasAlias := range []bool{false, true} { // add 6 candidates into cand center - m, err := NewCandidateCenter(nil, HasAliasOption(hasAlias)) + m, err := NewCandidateCenter(nil) r.NoError(err) for i, v := range testCandidates { r.NoError(m.Upsert(testCandidates[i].d)) @@ -341,7 +341,7 @@ func TestFixAlias(t *testing.T) { r.True(m.ContainsOperator(v.d.Operator)) r.Equal(v.d, m.GetByName(v.d.Name)) } - r.NoError(m.Commit()) + r.NoError(m.Commit(hasAlias)) r.NoError(view.Write(_protocolID, m)) // simulate handleCandidateUpdate: update name @@ -394,7 +394,7 @@ func TestFixAlias(t *testing.T) { // cand center Commit() { - r.NoError(center.Commit()) + r.NoError(center.Commit(hasAlias)) r.NoError(view.Write(_protocolID, center)) dk.Reset() } diff --git a/action/protocol/staking/candidate_statemanager.go b/action/protocol/staking/candidate_statemanager.go index 7ce38605f4..1aa090ce54 100644 --- a/action/protocol/staking/candidate_statemanager.go +++ b/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 } diff --git a/action/protocol/staking/candidate_statereader.go b/action/protocol/staking/candidate_statereader.go index 64c733b9e6..d2d7053682 100644 --- a/action/protocol/staking/candidate_statereader.go +++ b/action/protocol/staking/candidate_statereader.go @@ -151,7 +151,7 @@ func ConstructBaseView(sr protocol.StateReader) (CandidateStateReader, error) { } // CreateBaseView creates the base view from state reader -func CreateBaseView(sr protocol.StateReader, enableSMStorage, candCenterHasAlias bool) (*ViewData, uint64, error) { +func CreateBaseView(sr protocol.StateReader, enableSMStorage bool) (*ViewData, uint64, error) { if sr == nil { return nil, 0, ErrMissingField } @@ -162,7 +162,7 @@ func CreateBaseView(sr protocol.StateReader, enableSMStorage, candCenterHasAlias return nil, height, err } - center, err := NewCandidateCenter(all, HasAliasOption(candCenterHasAlias)) + center, err := NewCandidateCenter(all) if err != nil { return nil, height, err } diff --git a/action/protocol/staking/candidate_statereader_test.go b/action/protocol/staking/candidate_statereader_test.go index bbffc44995..ab5f5f7cfe 100644 --- a/action/protocol/staking/candidate_statereader_test.go +++ b/action/protocol/staking/candidate_statereader_test.go @@ -25,7 +25,7 @@ func Test_CandidateStateReader(t *testing.T) { require.NoError(err) csr, err := ConstructBaseView(sm) require.Equal(err, protocol.ErrNoName) - view, _, err := CreateBaseView(sm, false, false) + view, _, err := CreateBaseView(sm, false) require.NoError(err) csr = &candSR{ StateReader: sm, diff --git a/action/protocol/staking/handlers_test.go b/action/protocol/staking/handlers_test.go index 3c5133d105..a13a8425f8 100644 --- a/action/protocol/staking/handlers_test.go +++ b/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-- } } diff --git a/action/protocol/staking/protocol.go b/action/protocol/staking/protocol.go index 411109b52f..0f836062c5 100644 --- a/action/protocol/staking/protocol.go +++ b/action/protocol/staking/protocol.go @@ -114,7 +114,13 @@ func FindProtocol(registry *protocol.Registry) *Protocol { } // NewProtocol instantiates the protocol of staking -func NewProtocol(depositGas DepositGas, cfg *BuilderConfig, candBucketsIndexer *CandidatesBucketsIndexer, reviseHeights ...uint64) (*Protocol, error) { +func NewProtocol( + depositGas DepositGas, + cfg *BuilderConfig, + candBucketsIndexer *CandidatesBucketsIndexer, + correctCandsHeight uint64, + reviseHeights ...uint64, +) (*Protocol, error) { h := hash.Hash160b([]byte(_protocolID)) addr, err := address.FromBytes(h[:]) if err != nil { @@ -137,7 +143,7 @@ func NewProtocol(depositGas DepositGas, cfg *BuilderConfig, candBucketsIndexer * } // new vote reviser, revise ate greenland - voteReviser := NewVoteReviser(cfg.Staking.VoteWeightCalConsts, reviseHeights...) + voteReviser := NewVoteReviser(cfg.Staking.VoteWeightCalConsts, correctCandsHeight, reviseHeights...) return &Protocol{ addr: addr, @@ -173,7 +179,7 @@ func (p *Protocol) Start(ctx context.Context, sr protocol.StateReader) (interfac } // load view from SR - c, _, err := CreateBaseView(sr, featureCtx.ReadStateFromDB(height), featureCtx.CandCenterHasAlias(height)) + c, _, err := CreateBaseView(sr, featureCtx.ReadStateFromDB(height)) if err != nil { return nil, errors.Wrap(err, "failed to start staking protocol") } @@ -201,6 +207,7 @@ func (p *Protocol) CreateGenesisStates( if len(p.config.BootstrapCandidates) == 0 { return nil } + // TODO: set init values based on ctx csm, err := NewCandidateStateManager(sm, false) if err != nil { return err @@ -251,7 +258,7 @@ func (p *Protocol) CreateGenesisStates( } // commit updated view - return errors.Wrap(csm.Commit(), "failed to commit candidate change in CreateGenesisStates") + return errors.Wrap(csm.Commit(true), "failed to commit candidate change in CreateGenesisStates") } // CreatePreStates updates state manager @@ -334,7 +341,7 @@ func (p *Protocol) PreCommit(ctx context.Context, sm protocol.StateManager) erro } cc := csm.DirtyView().candCenter base := cc.base.clone() - if _, err = base.commit(cc.change); err != nil { + if _, err = base.commit(cc.change, featureWithHeightCtx.CandCenterHasAlias(height)); err != nil { return errors.Wrap(err, "failed to apply candidate change in pre-commit") } // persist nameMap/operatorMap and ownerList to stateDB @@ -344,7 +351,7 @@ func (p *Protocol) PreCommit(ctx context.Context, sm protocol.StateManager) erro if len(name) == 0 || len(op) == 0 { return ErrNilParameters } - if err := p.writeCandCenterStateToStateDB(sm, name, op, owners); err != nil { + if err := writeCandCenterStateToStateDB(sm, name, op, owners); err != nil { return errors.Wrap(err, "failed to write name/operator map to stateDB") } return nil @@ -363,7 +370,7 @@ func (p *Protocol) Commit(ctx context.Context, sm protocol.StateManager) error { } // commit updated view - return errors.Wrap(csm.Commit(), "failed to commit candidate change in Commit") + return errors.Wrap(csm.Commit(featureWithHeightCtx.CandCenterHasAlias(height)), "failed to commit candidate change in Commit") } // Handle handles a staking message @@ -627,7 +634,7 @@ func readCandCenterStateFromStateDB(sr protocol.StateReader, height uint64) (Can return name, operator, owner, nil } -func (p *Protocol) writeCandCenterStateToStateDB(sm protocol.StateManager, name, op, owners CandidateList) error { +func writeCandCenterStateToStateDB(sm protocol.StateManager, name, op, owners CandidateList) error { if _, err := sm.PutState(name, protocol.NamespaceOption(CandsMapNS), protocol.KeyOption(_nameKey)); err != nil { return err } diff --git a/action/protocol/staking/protocol_test.go b/action/protocol/staking/protocol_test.go index a83489d4e4..deb7336cca 100644 --- a/action/protocol/staking/protocol_test.go +++ b/action/protocol/staking/protocol_test.go @@ -126,7 +126,7 @@ func TestProtocol(t *testing.T) { for _, e := range testCandidates { r.NoError(csm.Upsert(e.d)) } - r.NoError(csm.Commit()) + r.NoError(csm.Commit(true)) for _, e := range testCandidates { r.True(csm.ContainsOwner(e.d.Owner)) r.True(csm.ContainsName(e.d.Name)) @@ -196,7 +196,7 @@ func TestCreatePreStates(t *testing.T) { p, err := NewProtocol(nil, &BuilderConfig{ Staking: genesis.Default.Staking, PersistStakingPatchBlock: math.MaxUint64, - }, nil, genesis.Default.GreenlandBlockHeight) + }, nil, genesis.Default.GreenlandBlockHeight, genesis.Default.GreenlandBlockHeight) require.NoError(err) ctx := protocol.WithBlockCtx( genesis.WithGenesisContext(context.Background(), genesis.Default), @@ -259,7 +259,7 @@ func Test_CreatePreStatesWithRegisterProtocol(t *testing.T) { p, err := NewProtocol(nil, &BuilderConfig{ Staking: genesis.Default.Staking, PersistStakingPatchBlock: math.MaxUint64, - }, cbi, genesis.Default.GreenlandBlockHeight) + }, cbi, genesis.Default.GreenlandBlockHeight, genesis.Default.GreenlandBlockHeight) require.NoError(err) rol := rolldpos.NewProtocol(23, 4, 3) diff --git a/action/protocol/staking/vote_reviser.go b/action/protocol/staking/vote_reviser.go index f5e3b8fbb0..aed868325e 100644 --- a/action/protocol/staking/vote_reviser.go +++ b/action/protocol/staking/vote_reviser.go @@ -14,39 +14,74 @@ import ( // VoteReviser is used to recalculate candidate votes. type VoteReviser struct { - reviseHeights []uint64 - cache map[uint64]CandidateList - c genesis.VoteWeightCalConsts + reviseHeights []uint64 + cache map[uint64][2]CandidateList + c genesis.VoteWeightCalConsts + correctCandsHeight uint64 } // NewVoteReviser creates a VoteReviser. -func NewVoteReviser(c genesis.VoteWeightCalConsts, reviseHeights ...uint64) *VoteReviser { +func NewVoteReviser(c genesis.VoteWeightCalConsts, correctCandsHeight uint64, reviseHeights ...uint64) *VoteReviser { return &VoteReviser{ - reviseHeights: reviseHeights, - cache: make(map[uint64]CandidateList), - c: c, + reviseHeights: reviseHeights, + cache: make(map[uint64][2]CandidateList), + c: c, + correctCandsHeight: correctCandsHeight, } } // Revise recalculate candidate votes on preset revising height. func (vr *VoteReviser) Revise(csm CandidateStateManager, height uint64) error { if !vr.isCacheExist(height) { + var dirty CandidateList cands, err := vr.calculateVoteWeight(csm) if err != nil { return err } - vr.storeToCache(height, cands) + if vr.needCorrectCands(height) { + for _, c := range csm.DirtyView().candCenter.base.nameMap { + dirty = append(dirty, c) + } + for _, c := range csm.DirtyView().candCenter.base.operatorMap { + dirty = append(dirty, c) + } + cands, err = vr.correctAliasCands(csm, cands) + if err != nil { + return err + } + } + vr.storeToCache(height, dirty, cands) } return vr.flush(height, csm) } +func (vr *VoteReviser) correctAliasCands(csm CandidateStateManager, cands CandidateList) (CandidateList, error) { + ownerMap := map[string]*Candidate{} + for addr, owner := range csm.DirtyView().candCenter.base.ownerMap { + ownerMap[addr] = owner + } + var retval CandidateList + for _, c := range cands { + if owner, ok := ownerMap[c.Owner.String()]; ok { + c.Operator = owner.Operator + c.Reward = owner.Reward + c.Name = owner.Name + } + retval = append(retval, c) + } + return retval, nil +} + func (vr *VoteReviser) result(height uint64) (CandidateList, bool) { cands, ok := vr.cache[height] - return cands, ok + if !ok { + return nil, false + } + return cands[1], true } -func (vr *VoteReviser) storeToCache(height uint64, cands CandidateList) { - vr.cache[height] = cands +func (vr *VoteReviser) storeToCache(height uint64, dirty, cands CandidateList) { + vr.cache[height] = [2]CandidateList{dirty, cands} } func (vr *VoteReviser) isCacheExist(height uint64) bool { @@ -61,7 +96,12 @@ func (vr *VoteReviser) NeedRevise(height uint64) bool { return true } } - return false + return vr.needCorrectCands(height) +} + +// NeedCorrectCands returns true if height needs to correct candidates +func (vr *VoteReviser) needCorrectCands(height uint64) bool { + return height == vr.correctCandsHeight } func (vr *VoteReviser) calculateVoteWeight(csm CandidateStateManager) (CandidateList, error) { @@ -117,19 +157,21 @@ func (vr *VoteReviser) calculateVoteWeight(csm CandidateStateManager) (Candidate } func (vr *VoteReviser) flush(height uint64, csm CandidateStateManager) error { - cands, ok := vr.cache[height] + cache, ok := vr.cache[height] if !ok { return nil } - sort.Sort(cands) log.L().Info("committed revise action", - zap.Uint64("height", height), zap.Int("number of cands", len(cands))) - for _, cand := range cands { - if err := csm.Upsert(cand); err != nil { - return err + zap.Uint64("height", height), zap.Int("number of cands", len(cache[1]))) + for i := 0; i < 2; i++ { + sort.Sort(cache[i]) + for _, cand := range cache[i] { + if err := csm.Upsert(cand); err != nil { + return err + } + log.L().Info("committed revise action", + zap.String("name", cand.Name), zap.String("votes", cand.Votes.String())) } - log.L().Info("committed revise action", - zap.String("name", cand.Name), zap.String("votes", cand.Votes.String())) } return nil } diff --git a/action/protocol/staking/vote_reviser_test.go b/action/protocol/staking/vote_reviser_test.go index ca3476a7d7..75e9f5eb4d 100644 --- a/action/protocol/staking/vote_reviser_test.go +++ b/action/protocol/staking/vote_reviser_test.go @@ -4,6 +4,7 @@ import ( "context" "math" "math/big" + "sort" "testing" "time" @@ -119,8 +120,8 @@ func TestVoteReviser(t *testing.T) { PersistStakingPatchBlock: math.MaxUint64, }, nil, - genesis.Default.GreenlandBlockHeight, genesis.Default.HawaiiBlockHeight, + genesis.Default.GreenlandBlockHeight, ) r.NotNil(stk) r.NoError(err) @@ -144,21 +145,44 @@ func TestVoteReviser(t *testing.T) { csm, err = NewCandidateStateManager(sm, false) r.NoError(err) + oldCand := testCandidates[3].d.Clone() + oldCand.Name = "old name" + r.NoError(csm.Upsert(oldCand)) + r.NoError(csm.Commit(true)) + r.NotNil(csm.GetByName(oldCand.Name)) // load a number of candidates - for _, e := range testCandidates { - r.NoError(csm.Upsert(e.d)) + updateCands := CandidateList{ + testCandidates[3].d, testCandidates[4].d, testCandidates[2].d, + testCandidates[0].d, testCandidates[1].d, testCandidates[5].d, } - r.NoError(csm.Commit()) + for _, e := range updateCands { + r.NoError(csm.Upsert(e)) + } + r.NoError(csm.Commit(true)) + r.NotNil(csm.GetByName(oldCand.Name)) + r.NotNil(csm.GetByName(testCandidates[3].d.Name)) // test revise - r.False(stk.voteReviser.isCacheExist(genesis.Default.GreenlandBlockHeight)) - r.False(stk.voteReviser.isCacheExist(genesis.Default.HawaiiBlockHeight)) - r.NoError(stk.voteReviser.Revise(csm, genesis.Default.HawaiiBlockHeight)) - r.NoError(csm.Commit()) - r.False(stk.voteReviser.isCacheExist(genesis.Default.GreenlandBlockHeight)) + vr := stk.voteReviser + r.False(vr.isCacheExist(genesis.Default.GreenlandBlockHeight)) + r.False(vr.isCacheExist(genesis.Default.HawaiiBlockHeight)) + r.NoError(vr.Revise(csm, genesis.Default.HawaiiBlockHeight)) + r.True(vr.isCacheExist(genesis.Default.HawaiiBlockHeight)) + verifyCands(r, sm, vr, updateCands) + // simulate first revise attempt failed -- call Revise() again + csm = verifyCands(r, sm, vr, updateCands) + r.True(vr.isCacheExist(genesis.Default.HawaiiBlockHeight)) + r.NoError(vr.Revise(csm, genesis.Default.HawaiiBlockHeight)) + // simulate protocol.Commit() + { + csm = verifyCands(r, sm, vr, updateCands) + r.NoError(csm.Commit(false)) + } + r.Nil(csm.GetByName(oldCand.Name)) + verifyCands(r, sm, vr, updateCands) // verify self-stake and total votes match - result, ok := stk.voteReviser.result(genesis.Default.HawaiiBlockHeight) + result, ok := vr.result(genesis.Default.HawaiiBlockHeight) r.True(ok) r.Equal(len(testCandidates), len(result)) cv := genesis.Default.Staking.VoteWeightCalConsts @@ -184,3 +208,19 @@ func TestVoteReviser(t *testing.T) { } } } + +func verifyCands(r *require.Assertions, sm protocol.StateManager, vr *VoteReviser, cands CandidateList) CandidateStateManager { + csm, err := NewCandidateStateManager(sm, false) + r.NoError(err) + cc := csm.DirtyView().candCenter + r.Equal(len(cands), cc.Size()) + all := cc.All() + sort.Sort(all) + r.Equal(len(cands), len(all)) + for i := range cands { + r.NotEqual(all[i].Votes, cands[i].Votes) + all[i].Votes = new(big.Int).Set(cands[i].Votes) + r.Equal(all[i], cands[i]) + } + return csm +} diff --git a/chainservice/builder.go b/chainservice/builder.go index 3b5fee2bf5..7b64cc7010 100644 --- a/chainservice/builder.go +++ b/chainservice/builder.go @@ -454,6 +454,7 @@ func (builder *Builder) registerStakingProtocol() error { StakingPatchDir: builder.cfg.Chain.StakingPatchDir, }, builder.cs.candBucketsIndexer, + builder.cfg.Genesis.OkhotskBlockHeight, builder.cfg.Genesis.GreenlandBlockHeight, builder.cfg.Genesis.HawaiiBlockHeight, ) From e14c507e76fa3de80d2ebf3d0ca287646995fc23 Mon Sep 17 00:00:00 2001 From: zhi Date: Fri, 18 Nov 2022 14:19:43 -0800 Subject: [PATCH 7/8] address comments --- action/protocol/staking/bucket_pool_test.go | 5 +- action/protocol/staking/candidate_center.go | 47 ++++++++++++----- .../protocol/staking/candidate_center_test.go | 20 +++++--- .../staking/candidate_statemanager.go | 17 +++++-- action/protocol/staking/protocol.go | 4 +- action/protocol/staking/protocol_test.go | 2 +- action/protocol/staking/vote_reviser.go | 46 ++++++++--------- action/protocol/staking/vote_reviser_test.go | 50 ++++++++----------- testutil/testdb/db.go | 17 ++++--- 9 files changed, 122 insertions(+), 86 deletions(-) diff --git a/action/protocol/staking/bucket_pool_test.go b/action/protocol/staking/bucket_pool_test.go index 2372230d1b..24aae24ab5 100644 --- a/action/protocol/staking/bucket_pool_test.go +++ b/action/protocol/staking/bucket_pool_test.go @@ -8,6 +8,7 @@ package staking import ( "bytes" + "context" "math/big" "testing" "time" @@ -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" @@ -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) @@ -147,7 +150,7 @@ func TestBucketPool(t *testing.T) { } if v.commit { - r.NoError(csm.Commit(false)) + r.NoError(csm.Commit(ctx)) // after commit, value should reflect in base view c, err = ConstructBaseView(sm) r.NoError(err) diff --git a/action/protocol/staking/candidate_center.go b/action/protocol/staking/candidate_center.go index 879bd4c910..706e6c3bba 100644 --- a/action/protocol/staking/candidate_center.go +++ b/action/protocol/staking/candidate_center.go @@ -69,7 +69,7 @@ func NewCandidateCenter(all CandidateList) (*CandidateCenter, error) { return &c, nil } - if err := c.Commit(true); err != nil { + if err := c.Commit(); err != nil { return nil, err } return &c, nil @@ -139,8 +139,20 @@ func (m *CandidateCenter) SetDelta(l CandidateList) error { } // Commit writes the change into base -func (m *CandidateCenter) Commit(keepAliasBug bool) error { - size, err := m.base.commit(m.change, keepAliasBug) +func (m *CandidateCenter) Commit() error { + 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 } @@ -447,21 +459,32 @@ func (cb *candBase) all() CandidateList { func (cb *candBase) commit(change *candChange, keepAliasBug bool) (int, error) { cb.lock.Lock() defer cb.lock.Unlock() - for _, v := range change.candidates { - 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 } - d := v.Clone() - if !keepAliasBug { + } 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 } - 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 } diff --git a/action/protocol/staking/candidate_center_test.go b/action/protocol/staking/candidate_center_test.go index 7b420b8bef..bba2da69c7 100644 --- a/action/protocol/staking/candidate_center_test.go +++ b/action/protocol/staking/candidate_center_test.go @@ -71,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(true)) - r.NoError(m.Commit(true)) // 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)) @@ -106,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(true)) + r.NoError(m.LegacyCommit()) r.Equal(len(testCandidates), m.Size()) r.True(testEqual(m, list)) old := m.All() @@ -214,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(true)) + r.NoError(m.LegacyCommit()) r.Equal(len(list), m.Size()) r.True(testEqual(m, list)) @@ -341,7 +341,11 @@ func TestFixAlias(t *testing.T) { r.True(m.ContainsOperator(v.d.Operator)) r.Equal(v.d, m.GetByName(v.d.Name)) } - r.NoError(m.Commit(hasAlias)) + if hasAlias { + r.NoError(m.LegacyCommit()) + } else { + r.NoError(m.Commit()) + } r.NoError(view.Write(_protocolID, m)) // simulate handleCandidateUpdate: update name @@ -394,7 +398,11 @@ func TestFixAlias(t *testing.T) { // cand center Commit() { - r.NoError(center.Commit(hasAlias)) + if hasAlias { + r.NoError(center.LegacyCommit()) + } else { + r.NoError(center.Commit()) + } r.NoError(view.Write(_protocolID, center)) dk.Reset() } diff --git a/action/protocol/staking/candidate_statemanager.go b/action/protocol/staking/candidate_statemanager.go index 1aa090ce54..54fc3fdf4e 100644 --- a/action/protocol/staking/candidate_statemanager.go +++ b/action/protocol/staking/candidate_statemanager.go @@ -7,6 +7,7 @@ package staking import ( + "context" "math/big" "github.com/pkg/errors" @@ -56,7 +57,7 @@ type ( Upsert(*Candidate) error CreditBucketPool(*big.Int) error DebitBucketPool(*big.Int, bool) error - Commit(bool) error + Commit(context.Context) error SM() protocol.StateManager } @@ -165,10 +166,20 @@ func (csm *candSM) DebitBucketPool(amount *big.Int, newBucket bool) error { return csm.bucketPool.DebitPool(csm, amount, newBucket) } -func (csm *candSM) Commit(keepAlias bool) error { - if err := csm.candCenter.Commit(keepAlias); err != nil { +func (csm *candSM) Commit(ctx context.Context) error { + height, err := csm.Height() + if err != nil { return err } + if featureWithHeightCtx, ok := protocol.GetFeatureWithHeightCtx(ctx); ok && featureWithHeightCtx.CandCenterHasAlias(height) { + if err := csm.candCenter.LegacyCommit(); err != nil { + return err + } + } else { + if err := csm.candCenter.Commit(); err != nil { + return err + } + } if err := csm.bucketPool.Commit(csm); err != nil { return err diff --git a/action/protocol/staking/protocol.go b/action/protocol/staking/protocol.go index 0f836062c5..b4505e3823 100644 --- a/action/protocol/staking/protocol.go +++ b/action/protocol/staking/protocol.go @@ -258,7 +258,7 @@ func (p *Protocol) CreateGenesisStates( } // commit updated view - return errors.Wrap(csm.Commit(true), "failed to commit candidate change in CreateGenesisStates") + return errors.Wrap(csm.Commit(ctx), "failed to commit candidate change in CreateGenesisStates") } // CreatePreStates updates state manager @@ -370,7 +370,7 @@ func (p *Protocol) Commit(ctx context.Context, sm protocol.StateManager) error { } // commit updated view - return errors.Wrap(csm.Commit(featureWithHeightCtx.CandCenterHasAlias(height)), "failed to commit candidate change in Commit") + return errors.Wrap(csm.Commit(ctx), "failed to commit candidate change in Commit") } // Handle handles a staking message diff --git a/action/protocol/staking/protocol_test.go b/action/protocol/staking/protocol_test.go index deb7336cca..6caff1139d 100644 --- a/action/protocol/staking/protocol_test.go +++ b/action/protocol/staking/protocol_test.go @@ -126,7 +126,7 @@ func TestProtocol(t *testing.T) { for _, e := range testCandidates { r.NoError(csm.Upsert(e.d)) } - r.NoError(csm.Commit(true)) + r.NoError(csm.Commit(ctx)) for _, e := range testCandidates { r.True(csm.ContainsOwner(e.d.Owner)) r.True(csm.ContainsName(e.d.Name)) diff --git a/action/protocol/staking/vote_reviser.go b/action/protocol/staking/vote_reviser.go index aed868325e..58a4c55352 100644 --- a/action/protocol/staking/vote_reviser.go +++ b/action/protocol/staking/vote_reviser.go @@ -15,7 +15,7 @@ import ( // VoteReviser is used to recalculate candidate votes. type VoteReviser struct { reviseHeights []uint64 - cache map[uint64][2]CandidateList + cache map[uint64]CandidateList c genesis.VoteWeightCalConsts correctCandsHeight uint64 } @@ -24,7 +24,7 @@ type VoteReviser struct { func NewVoteReviser(c genesis.VoteWeightCalConsts, correctCandsHeight uint64, reviseHeights ...uint64) *VoteReviser { return &VoteReviser{ reviseHeights: reviseHeights, - cache: make(map[uint64][2]CandidateList), + cache: make(map[uint64]CandidateList), c: c, correctCandsHeight: correctCandsHeight, } @@ -33,34 +33,35 @@ func NewVoteReviser(c genesis.VoteWeightCalConsts, correctCandsHeight uint64, re // Revise recalculate candidate votes on preset revising height. func (vr *VoteReviser) Revise(csm CandidateStateManager, height uint64) error { if !vr.isCacheExist(height) { - var dirty CandidateList cands, err := vr.calculateVoteWeight(csm) if err != nil { return err } + sort.Sort(cands) if vr.needCorrectCands(height) { - for _, c := range csm.DirtyView().candCenter.base.nameMap { - dirty = append(dirty, c) - } - for _, c := range csm.DirtyView().candCenter.base.operatorMap { - dirty = append(dirty, c) - } cands, err = vr.correctAliasCands(csm, cands) if err != nil { return err } } - vr.storeToCache(height, dirty, cands) + vr.storeToCache(height, cands) } return vr.flush(height, csm) } func (vr *VoteReviser) correctAliasCands(csm CandidateStateManager, cands CandidateList) (CandidateList, error) { + var retval CandidateList + for _, c := range csm.DirtyView().candCenter.base.nameMap { + retval = append(retval, c) + } + for _, c := range csm.DirtyView().candCenter.base.operatorMap { + retval = append(retval, c) + } + sort.Sort(retval) ownerMap := map[string]*Candidate{} for addr, owner := range csm.DirtyView().candCenter.base.ownerMap { ownerMap[addr] = owner } - var retval CandidateList for _, c := range cands { if owner, ok := ownerMap[c.Owner.String()]; ok { c.Operator = owner.Operator @@ -77,11 +78,11 @@ func (vr *VoteReviser) result(height uint64) (CandidateList, bool) { if !ok { return nil, false } - return cands[1], true + return cands, true } -func (vr *VoteReviser) storeToCache(height uint64, dirty, cands CandidateList) { - vr.cache[height] = [2]CandidateList{dirty, cands} +func (vr *VoteReviser) storeToCache(height uint64, cands CandidateList) { + vr.cache[height] = cands } func (vr *VoteReviser) isCacheExist(height uint64) bool { @@ -157,21 +158,18 @@ func (vr *VoteReviser) calculateVoteWeight(csm CandidateStateManager) (Candidate } func (vr *VoteReviser) flush(height uint64, csm CandidateStateManager) error { - cache, ok := vr.cache[height] + cands, ok := vr.cache[height] if !ok { return nil } log.L().Info("committed revise action", - zap.Uint64("height", height), zap.Int("number of cands", len(cache[1]))) - for i := 0; i < 2; i++ { - sort.Sort(cache[i]) - for _, cand := range cache[i] { - if err := csm.Upsert(cand); err != nil { - return err - } - log.L().Info("committed revise action", - zap.String("name", cand.Name), zap.String("votes", cand.Votes.String())) + zap.Uint64("height", height), zap.Int("number of cands", len(cands))) + for _, cand := range cands { + if err := csm.Upsert(cand); err != nil { + return err } + log.L().Info("committed revise action", + zap.String("name", cand.Name), zap.String("votes", cand.Votes.String())) } return nil } diff --git a/action/protocol/staking/vote_reviser_test.go b/action/protocol/staking/vote_reviser_test.go index 75e9f5eb4d..3534630f41 100644 --- a/action/protocol/staking/vote_reviser_test.go +++ b/action/protocol/staking/vote_reviser_test.go @@ -4,7 +4,6 @@ import ( "context" "math" "math/big" - "sort" "testing" "time" @@ -23,7 +22,8 @@ func TestVoteReviser(t *testing.T) { r := require.New(t) ctrl := gomock.NewController(t) - sm := testdb.NewMockStateManager(ctrl) + sm := testdb.NewMockStateManagerWithoutHeightFunc(ctrl) + sm.EXPECT().Height().Return(uint64(0), nil).Times(4) csm := newCandidateStateManager(sm) csr := newCandidateStateReader(sm) _, err := sm.PutState( @@ -120,6 +120,7 @@ func TestVoteReviser(t *testing.T) { PersistStakingPatchBlock: math.MaxUint64, }, nil, + genesis.Default.OkhotskBlockHeight, genesis.Default.HawaiiBlockHeight, genesis.Default.GreenlandBlockHeight, ) @@ -148,7 +149,7 @@ func TestVoteReviser(t *testing.T) { oldCand := testCandidates[3].d.Clone() oldCand.Name = "old name" r.NoError(csm.Upsert(oldCand)) - r.NoError(csm.Commit(true)) + r.NoError(csm.Commit(ctx)) r.NotNil(csm.GetByName(oldCand.Name)) // load a number of candidates updateCands := CandidateList{ @@ -158,7 +159,7 @@ func TestVoteReviser(t *testing.T) { for _, e := range updateCands { r.NoError(csm.Upsert(e)) } - r.NoError(csm.Commit(true)) + r.NoError(csm.Commit(ctx)) r.NotNil(csm.GetByName(oldCand.Name)) r.NotNil(csm.GetByName(testCandidates[3].d.Name)) @@ -168,19 +169,16 @@ func TestVoteReviser(t *testing.T) { r.False(vr.isCacheExist(genesis.Default.HawaiiBlockHeight)) r.NoError(vr.Revise(csm, genesis.Default.HawaiiBlockHeight)) r.True(vr.isCacheExist(genesis.Default.HawaiiBlockHeight)) - verifyCands(r, sm, vr, updateCands) // simulate first revise attempt failed -- call Revise() again - csm = verifyCands(r, sm, vr, updateCands) r.True(vr.isCacheExist(genesis.Default.HawaiiBlockHeight)) r.NoError(vr.Revise(csm, genesis.Default.HawaiiBlockHeight)) - // simulate protocol.Commit() - { - csm = verifyCands(r, sm, vr, updateCands) - r.NoError(csm.Commit(false)) - } - r.Nil(csm.GetByName(oldCand.Name)) - verifyCands(r, sm, vr, updateCands) - + sm.EXPECT().Height().DoAndReturn( + func() (uint64, error) { + return genesis.Default.HawaiiBlockHeight, nil + }, + ).Times(1) + r.NoError(csm.Commit(ctx)) + r.NotNil(csm.GetByName(oldCand.Name)) // verify self-stake and total votes match result, ok := vr.result(genesis.Default.HawaiiBlockHeight) r.True(ok) @@ -207,20 +205,12 @@ func TestVoteReviser(t *testing.T) { } } } -} - -func verifyCands(r *require.Assertions, sm protocol.StateManager, vr *VoteReviser, cands CandidateList) CandidateStateManager { - csm, err := NewCandidateStateManager(sm, false) - r.NoError(err) - cc := csm.DirtyView().candCenter - r.Equal(len(cands), cc.Size()) - all := cc.All() - sort.Sort(all) - r.Equal(len(cands), len(all)) - for i := range cands { - r.NotEqual(all[i].Votes, cands[i].Votes) - all[i].Votes = new(big.Int).Set(cands[i].Votes) - r.Equal(all[i], cands[i]) - } - return csm + r.NoError(vr.Revise(csm, genesis.Default.OkhotskBlockHeight)) + sm.EXPECT().Height().DoAndReturn( + func() (uint64, error) { + return genesis.Default.OkhotskBlockHeight, nil + }, + ).Times(1) + r.NoError(csm.Commit(ctx)) + r.Nil(csm.GetByName(oldCand.Name)) } diff --git a/testutil/testdb/db.go b/testutil/testdb/db.go index 1cc0599d1e..4eb0eff501 100644 --- a/testutil/testdb/db.go +++ b/testutil/testdb/db.go @@ -114,9 +114,16 @@ func NewMockKVStore(ctrl *gomock.Controller) db.KVStore { } // NewMockStateManager returns a in memory StateManager. -func NewMockStateManager(ctrl *gomock.Controller) protocol.StateManager { +func NewMockStateManager(ctrl *gomock.Controller) *mock_chainmanager.MockStateManager { + sm := NewMockStateManagerWithoutHeightFunc(ctrl) + sm.EXPECT().Height().Return(uint64(0), nil).AnyTimes() + + return sm +} + +// NewMockStateManager returns a in memory StateManager without default height function. +func NewMockStateManagerWithoutHeightFunc(ctrl *gomock.Controller) *mock_chainmanager.MockStateManager { sm := mock_chainmanager.NewMockStateManager(ctrl) - var h uint64 kv := NewMockKVStore(ctrl) dk := protocol.NewDock() view := protocol.View{} @@ -193,11 +200,7 @@ func NewMockStateManager(ctrl *gomock.Controller) protocol.StateManager { return 0, state.NewIterator(fv), nil }, ).AnyTimes() - sm.EXPECT().Height().DoAndReturn( - func() (uint64, error) { - return h, nil - }, - ).AnyTimes() + // sm.EXPECT().Height().Return(uint64(0), nil).AnyTimes() sm.EXPECT().Load(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(ns, key string, v interface{}) error { return dk.Load(ns, key, v) From 93d5a61217a7126a5b8c0aef4d852bdcd48b52cb Mon Sep 17 00:00:00 2001 From: zhi Date: Mon, 28 Nov 2022 14:34:31 -0800 Subject: [PATCH 8/8] address comment --- action/protocol/staking/vote_reviser.go | 4 ++-- testutil/testdb/db.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/action/protocol/staking/vote_reviser.go b/action/protocol/staking/vote_reviser.go index 58a4c55352..d61b3e11f7 100644 --- a/action/protocol/staking/vote_reviser.go +++ b/action/protocol/staking/vote_reviser.go @@ -59,8 +59,8 @@ func (vr *VoteReviser) correctAliasCands(csm CandidateStateManager, cands Candid } sort.Sort(retval) ownerMap := map[string]*Candidate{} - for addr, owner := range csm.DirtyView().candCenter.base.ownerMap { - ownerMap[addr] = owner + for _, cand := range csm.DirtyView().candCenter.base.owners { + ownerMap[cand.Owner.String()] = cand } for _, c := range cands { if owner, ok := ownerMap[c.Owner.String()]; ok { diff --git a/testutil/testdb/db.go b/testutil/testdb/db.go index 4eb0eff501..5ff528dc9b 100644 --- a/testutil/testdb/db.go +++ b/testutil/testdb/db.go @@ -121,7 +121,7 @@ func NewMockStateManager(ctrl *gomock.Controller) *mock_chainmanager.MockStateMa return sm } -// NewMockStateManager returns a in memory StateManager without default height function. +// NewMockStateManagerWithoutHeightFunc returns a in memory StateManager without default height function. func NewMockStateManagerWithoutHeightFunc(ctrl *gomock.Controller) *mock_chainmanager.MockStateManager { sm := mock_chainmanager.NewMockStateManager(ctrl) kv := NewMockKVStore(ctrl)