[staking] Handling CandidateEndorsement Action #4020

envestcc · 2023-12-15T15:30:10Z

Description

Implement action handling for CandidateEndorsement

Fixes #(issue)

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

[] make test
[] fullsync
[] Other test (please specify)

Test Configuration:

Firmware version:
Hardware:
Toolchain:
SDK:

Checklist:

[] My code follows the style guidelines of this project
[] I have performed a self-review of my code
[] I have commented my code, particularly in hard-to-understand areas
[] I have made corresponding changes to the documentation
[] My changes generate no new warnings
[] I have added tests that prove my fix is effective or that my feature works
[] New and existing unit tests pass locally with my changes
[] Any dependent changes have been merged and published in downstream modules

codecov · 2024-01-05T12:02:53Z

Codecov Report

Attention: 547 lines in your changes are missing coverage. Please review.

Comparison is base (e1f0636) 75.38% compared to head (df180c4) 76.20%.
Report is 165 commits behind head on master.

Files	Patch %	Lines
action/protocol/staking/staking_statereader.go	69.76%	35 Missing and 17 partials ⚠️
action/protocol/execution/evm/evm.go	48.38%	47 Missing and 1 partial ⚠️
api/coreservice.go	63.24%	37 Missing and 6 partials ⚠️
api/web3server.go	79.24%	30 Missing and 3 partials ⚠️
action/candidate_endorsement.go	0.00%	31 Missing ⚠️
action/protocol/staking/protocol.go	45.09%	28 Missing ⚠️
action/candidate_activate.go	0.00%	25 Missing ⚠️
...tion/protocol/staking/contractstake_bucket_type.go	0.00%	24 Missing ⚠️
action/protocol/rewarding/fund.go	23.07%	19 Missing and 1 partial ⚠️
api/websocket.go	0.00%	20 Missing ⚠️
... and 43 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4020      +/-   ##
==========================================
+ Coverage   75.38%   76.20%   +0.81%     
==========================================
  Files         303      339      +36     
  Lines       25923    28862    +2939     
==========================================
+ Hits        19541    21993    +2452     
- Misses       5360     5746     +386     
- Partials     1022     1123     +101

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dustinxie · 2024-01-18T04:17:22Z

action/protocol/staking/handler_candidate_endorsement.go

+	if err != nil {
+		return log, nil, err
+	}
+	return log, nil, nil


no need if, just return log, nil, err

dustinxie · 2024-01-18T04:53:30Z

action/protocol/staking/handler_candidate_endorsement.go

+	}
+	log.AddTopics(byteutil.Uint64ToBytesBigEndian(bucket.Index), bucket.Candidate.Bytes(), []byte{byteutil.BoolToByte(act.Endorse())})
+
+	var err error


move esm := NewEndorsementStateManager(csm.SM()) to here
var (
err error
esm = ...
)

dustinxie · 2024-01-18T04:55:17Z

action/protocol/staking/handler_candidate_endorsement.go

+	if err := esm.Put(bucket.Index, &Endorsement{
+		ExpireHeight: endorsementNotExpireHeight,
+	}); err != nil {
+		return csmErrorToHandleError(caller.String(), err)


this is for csm to use? i. e., when the value/type of err is related to candidate error

i mean, it will return a regular err, not the handleError you may want

should return regular err here, b/c the error returned must be some unknown errors from db

dustinxie · 2024-01-18T05:02:05Z

action/protocol/staking/handler_candidate_endorsement.go

+	if err := esm.Put(bucket.Index, &Endorsement{
+		ExpireHeight: expireHeight,
+	}); err != nil {
+		return csmErrorToHandleError(caller.String(), err)


CoderZhi · 2024-01-18T04:25:13Z

action/protocol/staking/handler_candidate_endorsement.go

+
+	var err error
+	if act.Endorse() {
+		err = p.endorseCandidate(ctx, csm, esm, actCtx.Caller, bucket, cand)


expireHeight = endorsementNotExpireHeight if err := p.validateEndorsement(ctx, csm, esm, caller, bucket, cand); err != nil { return err // wrap it }

CoderZhi · 2024-01-18T04:29:54Z

action/protocol/staking/handler_candidate_endorsement.go

+	if act.Endorse() {
+		err = p.endorseCandidate(ctx, csm, esm, actCtx.Caller, bucket, cand)
+	} else {
+		err = p.unEndorseCandidate(ctx, csm, esm, actCtx.Caller, bucket, cand)


if err := validateWithdrawal(ctx, esm, caller, bucket); err != nil { return err // wrap it } expireHeight = blkCtx.BlockHeight if csm.ContainsSelfStakingBucket(bucket.Index) { expireHeight += p.config.UnEndorseWaitingBlocks }

CoderZhi · 2024-01-18T04:31:59Z

action/protocol/staking/handler_candidate_endorsement.go

+	} else {
+		err = p.unEndorseCandidate(ctx, csm, esm, actCtx.Caller, bucket, cand)
+	}
+	if err != nil {


if err := esm.Put(bucket.Index, &Endorsement{ ExpireHeight: expireHeight, }); err != nil { return log, nil, csmErrorToHandleError(caller.String(), err) } return log, nil, nil

dustinxie · 2024-01-18T05:08:55Z

action/protocol/staking/handler_candidate_endorsement.go

+
+	expireHeight := blkCtx.BlockHeight
+	if csm.ContainsSelfStakingBucket(bucket.Index) {
+		expireHeight += p.config.UnEndorseWaitingBlocks


what's the case for a regular (not self-stake) endorsed bucket?

It expire immediately if the bucket is not self-staked, otherwise, expire after unendorse waiting period. Also added code comments for this

dustinxie · 2024-01-18T05:12:13Z

action/protocol/staking/handler_candidate_endorsement.go

+
+func (p *Protocol) validateUnEndorse(ctx context.Context, esm *EndorsementStateManager, caller address.Address, bucket *VoteBucket) ReceiptError {
+	blkCtx := protocol.MustGetBlockCtx(ctx)
+	if err := validateBucketOwner(bucket, caller); err != nil {


no need to check other conditions?

The two checks should be enough:

only bucket owner is authorized

bucket should be endorsed

dustinxie · 2024-01-19T01:13:33Z

action/protocol/staking/handler_candidate_endorsement.go

+		// new endorsement with not expire height
+		if err := esm.Put(bucket.Index, &Endorsement{
+			ExpireHeight: endorsementNotExpireHeight,
+		}); err != nil {


this part is the same, can move to common end

uint64 expireHeight if act.Endorse() { xxx expireHeight = endorsementNotExpireHeight, } else { xxx expireHeight := protocol.MustGetBlockCtx(ctx).BlockHeight if csm.ContainsSelfStakingBucket(bucket.Index) { expireHeight += p.config.EndorsementWithdrawWaitingBlocks } } esm.Put(bucket.Index, &Endorsement{ ExpireHeight: expireHeight, }

dustinxie · 2024-01-19T02:07:02Z

action/protocol/staking/handler_candidate_endorsement.go

@@ -31,30 +31,26 @@ func (p *Protocol) handleCandidateEndorsement(ctx context.Context, act *action.C
 	log.AddTopics(byteutil.Uint64ToBytesBigEndian(bucket.Index), bucket.Candidate.Bytes(), []byte{byteutil.BoolToByte(act.Endorse())})

 	esm := NewEndorsementStateManager(csm.SM())
-	// handle endorsement
+	expireHeight := uint64(0)


prefer to use

var ( expireHeight uint64 esm = NewEndorsementStateManager(csm.SM()) )

it is not my preference

CoderZhi · 2024-01-25T05:50:54Z

action/protocol/staking/handler_candidate_endorsement.go

@@ -31,30 +31,26 @@ func (p *Protocol) handleCandidateEndorsement(ctx context.Context, act *action.C
 	log.AddTopics(byteutil.Uint64ToBytesBigEndian(bucket.Index), bucket.Candidate.Bytes(), []byte{byteutil.BoolToByte(act.Endorse())})

 	esm := NewEndorsementStateManager(csm.SM())
-	// handle endorsement
+	expireHeight := uint64(0)


it is not my preference

Liuhaai · 2024-01-25T06:40:35Z

blockchain/genesis/genesis.go

+			WithdrawWaitingPeriod:            3 * 24 * time.Hour,
+			MinStakeAmount:                   unit.ConvertIotxToRau(100).String(),
+			BootstrapCandidates:              []BootstrapCandidate{},
+			EndorsementWithdrawWaitingBlocks: 24 * 60 * 60 / 5,


what's the meaning of the value

After submitting the endorsement withdraw action, it takes the waiting blocks for it to take effect. During this period, the delegate can continue to participate in consensus, and the bucket is locked, meaning it cannot be unstaked or voted to others.

Liuhaai · 2024-01-25T07:03:03Z

action/protocol/staking/handler_candidate_endorsement.go

+
+	esm := NewEndorsementStateManager(csm.SM())
+	expireHeight := uint64(0)
+	if act.Endorse() {


if act.IsEndorsed() {}

Liuhaai · 2024-01-25T07:06:40Z

action/protocol/staking/handler_candidate_endorsement.go

+	if err := validateBucketSelfStake(csm, bucket, false); err != nil {
+		return err
+	}
+	return validateBucketEndorsement(esm, bucket, false, protocol.MustGetBlockCtx(ctx).BlockHeight)


could be as the method of endorsement object

sonarqubecloud · 2024-01-30T12:59:59Z

Quality Gate failed

Failed conditions

4.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

envestcc force-pushed the pr_endorsehandle branch from 490a9b4 to d226131 Compare January 5, 2024 11:47

envestcc changed the title ~~[staking] Handling CandidateEndorsement Action (based on 4017,4018,4019)~~ [staking] Handling CandidateEndorsement Action (based on 4018,4019) Jan 5, 2024

envestcc marked this pull request as ready for review January 5, 2024 12:06

envestcc requested review from CoderZhi, dustinxie and Liuhaai as code owners January 5, 2024 12:06

envestcc force-pushed the pr_endorsehandle branch from f369707 to 4285729 Compare January 10, 2024 12:16

envestcc changed the title ~~[staking] Handling CandidateEndorsement Action (based on 4018,4019)~~ [staking] Handling CandidateEndorsement Action (based on 4019) Jan 10, 2024

envestcc force-pushed the pr_endorsehandle branch 2 times, most recently from 57e706b to 0241d03 Compare January 12, 2024 15:39

envestcc changed the title ~~[staking] Handling CandidateEndorsement Action (based on 4019)~~ [staking] Handling CandidateEndorsement Action Jan 12, 2024

envestcc force-pushed the pr_endorsehandle branch 2 times, most recently from ab2e5ad to 494acb0 Compare January 18, 2024 01:57

dustinxie reviewed Jan 18, 2024

View reviewed changes

CoderZhi reviewed Jan 18, 2024

View reviewed changes

dustinxie reviewed Jan 18, 2024

View reviewed changes

dustinxie reviewed Jan 19, 2024

View reviewed changes

CoderZhi approved these changes Jan 25, 2024

View reviewed changes

Liuhaai reviewed Jan 25, 2024

View reviewed changes

Liuhaai approved these changes Jan 25, 2024

View reviewed changes

envestcc added 2 commits January 30, 2024 20:41

handle candidate endorsement

3f9756f

expire height according to self-stake

fa5fb31

envestcc added 5 commits January 30, 2024 20:41

address comments

fd62c0b

address comment

9e12edd

add test

053fd9c

remove future test

841ad95

address comment

df180c4

envestcc force-pushed the pr_endorsehandle branch from 68bfced to df180c4 Compare January 30, 2024 12:59

envestcc merged commit ed9c857 into iotexproject:master Jan 30, 2024
3 of 5 checks passed

envestcc deleted the pr_endorsehandle branch January 30, 2024 13:24

BrewTestBot mentioned this pull request Mar 25, 2024

ioctl 1.14.0 Homebrew/homebrew-core#167116

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[staking] Handling CandidateEndorsement Action #4020

[staking] Handling CandidateEndorsement Action #4020

envestcc commented Dec 15, 2023

codecov bot commented Jan 5, 2024 •

edited

Loading

dustinxie Jan 18, 2024

dustinxie Jan 18, 2024

dustinxie Jan 18, 2024

dustinxie Jan 18, 2024

envestcc Jan 18, 2024

dustinxie Jan 18, 2024

CoderZhi Jan 18, 2024

CoderZhi Jan 18, 2024

CoderZhi Jan 18, 2024

dustinxie Jan 18, 2024

envestcc Jan 18, 2024

dustinxie Jan 18, 2024 •

edited

Loading

envestcc Jan 18, 2024

dustinxie Jan 19, 2024

dustinxie Jan 19, 2024

CoderZhi Jan 25, 2024

CoderZhi Jan 25, 2024

Liuhaai Jan 25, 2024

envestcc Jan 30, 2024

Liuhaai Jan 25, 2024

Liuhaai Jan 25, 2024

sonarqubecloud bot commented Jan 30, 2024

[staking] Handling CandidateEndorsement Action #4020

[staking] Handling CandidateEndorsement Action #4020

Conversation

envestcc commented Dec 15, 2023

Description

Type of change

How Has This Been Tested?

Checklist:

codecov bot commented Jan 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustinxie Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 30, 2024

Quality Gate failed

codecov bot commented Jan 5, 2024 •

edited

Loading

dustinxie Jan 18, 2024 •

edited

Loading