Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[staking] ActiveCandidate Exclude Candidate with Expired Endorsement #4062

Merged
merged 10 commits into from
Mar 11, 2024
4 changes: 3 additions & 1 deletion action/protocol/poll/nativestakingV2.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import (

"github.com/iotexproject/go-pkgs/hash"
"github.com/iotexproject/iotex-address/address"
"github.com/iotexproject/iotex-election/util"

"github.com/iotexproject/iotex-core/action"
"github.com/iotexproject/iotex-core/action/protocol"
"github.com/iotexproject/iotex-core/action/protocol/staking"
"github.com/iotexproject/iotex-core/state"
"github.com/iotexproject/iotex-election/util"
)

type nativeStakingV2 struct {
Expand Down Expand Up @@ -83,6 +84,7 @@ func (ns *nativeStakingV2) CalculateCandidatesByHeight(ctx context.Context, sr p
return cands, err
}
bcCtx := protocol.MustGetBlockchainCtx(ctx)
// TODO: put candidates which will expired during current or next epoch behind the active candidates
return ns.filterAndSortCandidatesByVoteScore(cands, bcCtx.Tip.Timestamp), nil
}

Expand Down
11 changes: 11 additions & 0 deletions action/protocol/staking/candidate_statemanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ type (
DebitBucketPool(*big.Int, bool) error
Commit(context.Context) error
SM() protocol.StateManager
SR() protocol.StateReader
}

// CandidiateStateCommon is the common interface for candidate state manager and reader
CandidiateStateCommon interface {
ContainsSelfStakingBucket(uint64) bool
SR() protocol.StateReader
}

candSM struct {
Expand Down Expand Up @@ -106,6 +113,10 @@ func (csm *candSM) SM() protocol.StateManager {
return csm.StateManager
}

func (csm *candSM) SR() protocol.StateReader {
return csm.StateManager
}

// DirtyView is csm's current state, which reflects base view + applying delta saved in csm's dock
func (csm *candSM) DirtyView() *ViewData {
return &ViewData{
Expand Down
5 changes: 5 additions & 0 deletions action/protocol/staking/candidate_statereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
AllCandidates() CandidateList
TotalStakedAmount() *big.Int
ActiveBucketsCount() uint64
ContainsSelfStakingBucket(index uint64) bool
}

candSR struct {
Expand Down Expand Up @@ -110,6 +111,10 @@
return c.view.candCenter.All()
}

func (c *candSR) ContainsSelfStakingBucket(index uint64) bool {
return c.view.candCenter.ContainsSelfStakingBucket(index)

Check warning on line 115 in action/protocol/staking/candidate_statereader.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/staking/candidate_statereader.go#L114-L115

Added lines #L114 - L115 were not covered by tests
}

func (c *candSR) TotalStakedAmount() *big.Int {
return c.view.bucketPool.Total()
}
Expand Down
4 changes: 2 additions & 2 deletions action/protocol/staking/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2810,7 +2810,7 @@ func TestProtocol_FetchBucketAndValidate(t *testing.T) {
})
isSelfStake := true
isSelfStakeErr := error(nil)
patches.ApplyFunc(isSelfStakeBucket, func(featureCtx protocol.FeatureCtx, csm CandidateStateManager, bucket *VoteBucket) (bool, error) {
patches.ApplyFunc(isSelfStakeBucket, func(featureCtx protocol.FeatureCtx, csm CandidiateStateCommon, bucket *VoteBucket) (bool, error) {
return isSelfStake, isSelfStakeErr
})
_, err = p.fetchBucketAndValidate(protocol.FeatureCtx{}, csm, identityset.Address(1), 1, false, false)
Expand All @@ -2831,7 +2831,7 @@ func TestProtocol_FetchBucketAndValidate(t *testing.T) {
Owner: identityset.Address(1),
}, nil
})
patches.ApplyFunc(isSelfStakeBucket, func(featureCtx protocol.FeatureCtx, csm CandidateStateManager, bucket *VoteBucket) (bool, error) {
patches.ApplyFunc(isSelfStakeBucket, func(featureCtx protocol.FeatureCtx, csm CandidiateStateCommon, bucket *VoteBucket) (bool, error) {
return false, nil
})
defer patches.Reset()
Expand Down
42 changes: 36 additions & 6 deletions action/protocol/staking/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@

// Errors
var (
ErrWithdrawnBucket = errors.New("the bucket is already withdrawn")
TotalBucketKey = append([]byte{_const}, []byte("totalBucket")...)
ErrWithdrawnBucket = errors.New("the bucket is already withdrawn")
ErrEndorsementNotExist = errors.New("the endorsement does not exist")
TotalBucketKey = append([]byte{_const}, []byte("totalBucket")...)
)

var (
Expand Down Expand Up @@ -472,6 +473,31 @@
return nil
}

func (p *Protocol) isActiveCandidate(ctx context.Context, csr CandidateStateReader, cand *Candidate) (bool, error) {
if cand.SelfStake.Cmp(p.config.RegistrationConsts.MinSelfStake) < 0 {
return false, nil
}
featureCtx := protocol.MustGetFeatureCtx(ctx)
if featureCtx.DisableDelegateEndorsement {
// before endorsement feature, candidates with enough amount must be active
return true, nil
Copy link
Member

Choose a reason for hiding this comment

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

just return cand.SelfStake.Cmp(p.config.RegistrationConsts.MinSelfStake) >= 0, nil

}
bucket, err := csr.getBucket(cand.SelfStakeBucketIdx)
switch {
case errors.Cause(err) == state.ErrStateNotExist:

Check warning on line 487 in action/protocol/staking/protocol.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/staking/protocol.go#L485-L487

Added lines #L485 - L487 were not covered by tests
// endorse bucket has been withdrawn
return false, nil

Check warning on line 489 in action/protocol/staking/protocol.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/staking/protocol.go#L489

Added line #L489 was not covered by tests
case err != nil:
return false, errors.Wrapf(err, "failed to get bucket %d", cand.SelfStakeBucketIdx)
default:

Check warning on line 492 in action/protocol/staking/protocol.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/staking/protocol.go#L492

Added line #L492 was not covered by tests
}
selfStake, err := isSelfStakeBucket(featureCtx, csr, bucket)
if err != nil {

Check warning on line 495 in action/protocol/staking/protocol.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/staking/protocol.go#L494-L495

Added lines #L494 - L495 were not covered by tests
return false, errors.Wrapf(err, "failed to check self-stake bucket %d", cand.SelfStakeBucketIdx)
}
return selfStake, nil

Check warning on line 498 in action/protocol/staking/protocol.go

View check run for this annotation

Codecov / codecov/patch

action/protocol/staking/protocol.go#L498

Added line #L498 was not covered by tests
}

// ActiveCandidates returns all active candidates in candidate center
func (p *Protocol) ActiveCandidates(ctx context.Context, sr protocol.StateReader, height uint64) (state.CandidateList, error) {
srHeight, err := sr.Height()
Expand All @@ -496,7 +522,11 @@
}
list[i].Votes.Add(list[i].Votes, contractVotes)
}
if list[i].SelfStake.Cmp(p.config.RegistrationConsts.MinSelfStake) >= 0 {
active, err := p.isActiveCandidate(ctx, c, list[i])
Copy link
Member

Choose a reason for hiding this comment

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

why are unactive cands included in L527?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are two scenarios:

  • The self-stake bucket has been unstaked.
  • The endorsement has expired.

Both situations belong to a registered delegate, but they cannot participate in consensus due to a lack of staking.

Copy link
Member

Choose a reason for hiding this comment

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

ok. It seems the state of candidates aren't tracked in the db(active/unactive). Pls add ur comments to explain why the list is filtered above L541

if err != nil {
return nil, err
}
if active {
Copy link
Member

@dustinxie dustinxie Feb 7, 2024

Choose a reason for hiding this comment

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

from the usage here, it is checking current height, not currentEpochNum+1 height

cand = append(cand, list[i])
}
}
Expand Down Expand Up @@ -686,9 +716,9 @@
}

// isSelfStakeBucket returns true if the bucket is self-stake bucket and not expired
func isSelfStakeBucket(featureCtx protocol.FeatureCtx, csm CandidateStateManager, bucket *VoteBucket) (bool, error) {
func isSelfStakeBucket(featureCtx protocol.FeatureCtx, csc CandidiateStateCommon, bucket *VoteBucket) (bool, error) {
// bucket index should be settled in one of candidates
selfStake := csm.ContainsSelfStakingBucket(bucket.Index)
selfStake := csc.ContainsSelfStakingBucket(bucket.Index)
if featureCtx.DisableDelegateEndorsement || !selfStake {
return selfStake, nil
}
Expand All @@ -698,7 +728,7 @@
return !bucket.isUnstaked(), nil
}
// otherwise bucket should be an endorse bucket which is not expired
esm := NewEndorsementStateManager(csm.SM())
esm := NewEndorsementStateReader(csc.SR())
height, err := esm.Height()
if err != nil {
return false, err
Expand Down
Loading