[staking] Handling CandidateSelfStake Action #4011
Conversation
envestcc
changed the title
envestcc
changed the title
envestcc
force-pushed
the
pr_handleselfstake
branch
2 times, most recently
from
0ed2fea
to
4a4c1c1
Compare
envestcc
force-pushed
the
pr_handleselfstake
branch
from
4a4c1c1
to
80a8280
Compare
envestcc
requested review from
CoderZhi,
dustinxie and
Liuhaai
as code owners
envestcc
force-pushed
the
pr_handleselfstake
branch
from
80a8280
to
52ba06d
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4011 +/- ##
==========================================
+ Coverage 75.38% 76.19% +0.81%
==========================================
Files 303 338 +35
Lines 25923 28774 +2851
==========================================
+ Hits 19541 21925 +2384
- Misses 5360 5733 +373
- Partials 1022 1116 +94 ☔ View full report in Codecov by Sentry. |
| } | ||
| if bucketCand = csm.GetByOwner(bucket.Candidate); bucketCand == nil { | ||
| return log, nil, &handleError{ | ||
| err: errors.New("bucket candidate does not exist"), |
There was a problem hiding this comment.
deleted, b/c candidate has been validated in p.validateBucketSelfStake()
| if cand.SelfStakeBucketIdx != candidateNoSelfStakeBucketIndex { | ||
| prevBucket, err = p.fetchBucket(csm, cand.SelfStakeBucketIdx) | ||
| if err != nil { | ||
| return log, nil, err | ||
| } | ||
| } |
| if err := validateBucketCandidate(bucket, cand.Owner); err != nil { | ||
| return err | ||
| } | ||
| if validateBucketOwner(bucket, cand.Owner) != nil && |
There was a problem hiding this comment.
should be &&, it means only owned or endorsed bucket can be used to activate candidate self
| if err := csm.Upsert(cand); err != nil { | ||
| return csmErrorToHandleError(cand.Owner.String(), err) | ||
| } |
There was a problem hiding this comment.
function deleted, b/c bucket must be voted to the candidate
| cand.SelfStake = big.NewInt(0).SetBytes(bucket.StakedAmount.Bytes()) | ||
| prevVotes := p.calculateVoteWeight(bucket, false) | ||
| postVotes := p.calculateVoteWeight(bucket, true) | ||
| if err := cand.AddVote(postVotes.Sub(postVotes, prevVotes)); err != nil { |
There was a problem hiding this comment.
if AddVote err, does the SubVote above need sto rollback? any changes to db here should be put into a transaction.
There was a problem hiding this comment.
ACID is guaranteed by workingSet
There was a problem hiding this comment.
just cand.AddVote(post), cand.SubVote(prev)
|
|
||
| func (p *Protocol) validateBucketSelfStake(ctx context.Context, csm CandidateStateManager, esm *EndorsementStateManager, bucket *VoteBucket, cand *Candidate) ReceiptError { | ||
| blkCtx := protocol.MustGetBlockCtx(ctx) | ||
| if err := validateBucketMinAmount(bucket, p.config.RegistrationConsts.MinSelfStake); err != nil { |
There was a problem hiding this comment.
maybe better
validateFn = []func()error
for _,fn := range validateFn {
if err := fn(); err != nil {
return err
}
}
There was a problem hiding this comment.
Their parameters are different, i think no need to enforce using same interface.
| prevVotes := p.calculateVoteWeight(prevBucket, true) | ||
| postVotes := p.calculateVoteWeight(prevBucket, false) | ||
| if err := cand.SubVote(prevVotes.Sub(prevVotes, postVotes)); err != nil { |
There was a problem hiding this comment.
+1, i think just cand.AddVote(post), cand.SubVote(prev)
| if err := cand.SubVote(prevVotes.Sub(prevVotes, postVotes)); err != nil { | ||
| return log, nil, err | ||
| } | ||
| cand.SelfStake = big.NewInt(0) |
| candidateNoSelfStakeBucketIndex = math.MaxUint64 | ||
| ) | ||
|
|
||
| func (p *Protocol) handleCandidateSelfStake(ctx context.Context, act *action.CandidateActivate, csm CandidateStateManager, |
| ) | ||
|
|
||
| const ( | ||
| handleCandidateSelfStake = "candidateSelfStake" |
| log := newReceiptLog(p.addr.String(), handleCandidateSelfStake, featureCtx.NewStakingReceiptFormat) | ||
|
|
||
| // caller must be the owner of a candidate | ||
| cand := csm.GetByOwner(actCtx.Caller) |
There was a problem hiding this comment.
move this to after get and validate bucket, to be consistent with other handlers
envestcc
force-pushed
the
pr_handleselfstake
branch
from
c26f5e9
to
5c7790b
Compare
| const ( | ||
| handleCandidateActivate = "candidateActivate" | ||
|
|
||
| candidateNoSelfStakeBucketIndex = math.MaxUint64 |
There was a problem hiding this comment.
remove since it's only used in test, and why is it needed in test?
There was a problem hiding this comment.
It will be used when register candidate without stake
| ) (*receiptLog, []*action.TransactionLog, error) { | ||
| actCtx := protocol.MustGetActionCtx(ctx) | ||
| featureCtx := protocol.MustGetFeatureCtx(ctx) | ||
| log := newReceiptLog(p.addr.String(), handleCandidateActivate, featureCtx.NewStakingReceiptFormat) |
|
|
||
| log.AddTopics(byteutil.Uint64ToBytesBigEndian(bucket.Index), bucket.Candidate.Bytes()) | ||
| // convert previous self-stake bucket to vote bucket | ||
| if cand.SelfStake.Cmp(big.NewInt(0)) > 0 { |
There was a problem hiding this comment.
if cand.SelfStake.Sign() > 0 is more efficient and clear
|
|
||
| // convert vote bucket to self-stake bucket | ||
| cand.SelfStakeBucketIdx = bucket.Index | ||
| cand.SelfStake = big.NewInt(0).SetBytes(bucket.StakedAmount.Bytes()) |
There was a problem hiding this comment.
cand.SelfStake is an existing big.Int, no need to new again, so:
cand.SelfStake.SetBytes(bucket.StakedAmount.Bytes())
| if err := csm.Upsert(cand); err != nil { | ||
| return log, nil, csmErrorToHandleError(cand.Owner.String(), err) | ||
| } | ||
|
|
|
Description
CandidateSelfStakeaction accept a bucket as the self-stake bucket of the sender's candidate. The bucket can be:Type of change
Please delete options that are not relevant.
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
Test Configuration:
Checklist: