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.

Sign up for GitHub

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

Jump to bottom

[blockindex] restrict height for contract_staking_indexer during read operation #3927

Merged
merged 6 commits into from
Aug 25, 2023
Merged

[blockindex] restrict height for contract_staking_indexer during read operation #3927

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6f3b8fd
add height limit for contract staking indexer
envestcc Aug 22, 2023
a5fcd3a
update tests
envestcc Aug 22, 2023
0f88d0e
address comments
envestcc Aug 23, 2023
07e537b
address comments
envestcc Aug 24, 2023
f10a948
address comments
envestcc Aug 24, 2023
4ab8f36
address comments
envestcc Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions action/protocol/staking/contractstake_indexer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ type (
ContractStakingIndexer interface {
// CandidateVotes returns the total staked votes of a candidate
// candidate identified by owner address
CandidateVotes(ownerAddr address.Address) *big.Int
CandidateVotes(ownerAddr address.Address, height uint64) (*big.Int, error)
// Buckets returns active buckets
Buckets() ([]*VoteBucket, error)
Buckets(height uint64) ([]*VoteBucket, error)
// BucketsByIndices returns active buckets by indices
BucketsByIndices([]uint64) ([]*VoteBucket, error)
BucketsByIndices([]uint64, uint64) ([]*VoteBucket, error)
// BucketsByCandidate returns active buckets by candidate
BucketsByCandidate(ownerAddr address.Address) ([]*VoteBucket, error)
BucketsByCandidate(ownerAddr address.Address, height uint64) ([]*VoteBucket, error)
// TotalBucketCount returns the total number of buckets including burned buckets
TotalBucketCount() uint64
TotalBucketCount(height uint64) (uint64, error)
// BucketTypes returns the active bucket types
BucketTypes() ([]*ContractStakingBucketType, error)
BucketTypes(height uint64) ([]*ContractStakingBucketType, error)
// Height returns the indexer height
Height() (uint64, error)
Copy link
Member

Choose a reason for hiding this comment

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

not needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, forget to revert

}
)
69 changes: 43 additions & 26 deletions action/protocol/staking/contractstake_indexer_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion action/protocol/staking/protocol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,11 @@ func (p *Protocol) ActiveCandidates(ctx context.Context, sr protocol.StateReader
featureCtx := protocol.MustGetFeatureCtx(ctx)
for i := range list {
if p.contractStakingIndexer != nil && featureCtx.AddContractStakingVotes {
list[i].Votes.Add(list[i].Votes, p.contractStakingIndexer.CandidateVotes(list[i].Owner))
contractVotes, err := p.contractStakingIndexer.CandidateVotes(list[i].Owner, height)
Copy link
Member

Choose a reason for hiding this comment

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

height here equals to the height of block to be committed, that is, current factory/indexer's height + 1
so should use height-1? pls confirm

Copy link
Collaborator

Choose a reason for hiding this comment

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

should be covered by unit test

if err != nil {
return nil, errors.Wrap(err, "failed to get CandidateVotes from contractStakingIndexer")
}
list[i].Votes.Add(list[i].Votes, contractVotes)
}
if list[i].SelfStake.Cmp(p.config.RegistrationConsts.MinSelfStake) >= 0 {
cand = append(cand, list[i])
Expand Down
37 changes: 23 additions & 14 deletions action/protocol/staking/staking_statereader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (c *compositeStakingStateReader) readStateBuckets(ctx context.Context, req
}

// read LSD buckets
lsdBuckets, err := c.contractIndexer.Buckets()
lsdBuckets, err := c.contractIndexer.Buckets(inputHeight)
if err != nil {
return nil, 0, err
}
Expand All @@ -99,7 +99,7 @@ func (c *compositeStakingStateReader) readStateBucketsByVoter(ctx context.Contex
}

// read LSD buckets
lsdBuckets, err := c.contractIndexer.Buckets()
lsdBuckets, err := c.contractIndexer.Buckets(height)
if err != nil {
return nil, 0, err
}
Expand Down Expand Up @@ -131,7 +131,7 @@ func (c *compositeStakingStateReader) readStateBucketsByCandidate(ctx context.Co
if candidate == nil {
return &iotextypes.VoteBucketList{}, height, nil
}
lsdBuckets, err := c.contractIndexer.BucketsByCandidate(candidate.Owner)
lsdBuckets, err := c.contractIndexer.BucketsByCandidate(candidate.Owner, height)
if err != nil {
return nil, 0, err
}
Expand All @@ -156,7 +156,7 @@ func (c *compositeStakingStateReader) readStateBucketByIndices(ctx context.Conte
}

// read LSD buckets
lsdBuckets, err := c.contractIndexer.BucketsByIndices(req.GetIndex())
lsdBuckets, err := c.contractIndexer.BucketsByIndices(req.GetIndex(), height)
if err != nil {
return nil, 0, err
}
Expand All @@ -177,12 +177,16 @@ func (c *compositeStakingStateReader) readStateBucketCount(ctx context.Context,
if !c.isContractStakingEnabled() {
return bucketCnt, height, nil
}
buckets, err := c.contractIndexer.Buckets()
buckets, err := c.contractIndexer.Buckets(height)
if err != nil {
return nil, 0, err
}
bucketCnt.Active += uint64(len(buckets))
bucketCnt.Total += c.contractIndexer.TotalBucketCount()
tbc, err := c.contractIndexer.TotalBucketCount(height)
if err != nil {
return nil, 0, err
}
bucketCnt.Total += tbc
return bucketCnt, height, nil
}

Expand Down Expand Up @@ -220,7 +224,7 @@ func (c *compositeStakingStateReader) readStateCandidates(ctx context.Context, r
return candidates, height, nil
}
for _, candidate := range candidates.Candidates {
if err = addContractStakingVotes(candidate, c.contractIndexer); err != nil {
if err = addContractStakingVotes(candidate, c.contractIndexer, height); err != nil {
return nil, 0, err
}
}
Expand All @@ -238,7 +242,7 @@ func (c *compositeStakingStateReader) readStateCandidateByName(ctx context.Conte
if !protocol.MustGetFeatureCtx(ctx).AddContractStakingVotes {
return candidate, height, nil
}
if err := addContractStakingVotes(candidate, c.contractIndexer); err != nil {
if err := addContractStakingVotes(candidate, c.contractIndexer, height); err != nil {
return nil, 0, err
}
return candidate, height, nil
Expand All @@ -255,7 +259,7 @@ func (c *compositeStakingStateReader) readStateCandidateByAddress(ctx context.Co
if !protocol.MustGetFeatureCtx(ctx).AddContractStakingVotes {
return candidate, height, nil
}
if err := addContractStakingVotes(candidate, c.contractIndexer); err != nil {
if err := addContractStakingVotes(candidate, c.contractIndexer, height); err != nil {
return nil, 0, err
}
return candidate, height, nil
Expand All @@ -275,7 +279,7 @@ func (c *compositeStakingStateReader) readStateTotalStakingAmount(ctx context.Co
return accountMeta, height, nil
}
// add contract staking amount
buckets, err := c.contractIndexer.Buckets()
buckets, err := c.contractIndexer.Buckets(height)
if err != nil {
return nil, 0, err
}
Expand All @@ -291,7 +295,8 @@ func (c *compositeStakingStateReader) readStateContractStakingBucketTypes(ctx co
if !c.isContractStakingEnabled() {
return &iotextypes.ContractStakingBucketTypeList{}, c.nativeSR.Height(), nil
}
bts, err := c.contractIndexer.BucketTypes()
height := c.nativeSR.Height()
bts, err := c.contractIndexer.BucketTypes(height)
if err != nil {
return nil, 0, err
}
Expand All @@ -302,14 +307,14 @@ func (c *compositeStakingStateReader) readStateContractStakingBucketTypes(ctx co
StakedDuration: uint32(bt.Duration),
})
}
return &iotextypes.ContractStakingBucketTypeList{BucketTypes: pbBts}, c.nativeSR.Height(), nil
return &iotextypes.ContractStakingBucketTypeList{BucketTypes: pbBts}, height, nil
}

func (c *compositeStakingStateReader) isContractStakingEnabled() bool {
return c.contractIndexer != nil
}

func addContractStakingVotes(candidate *iotextypes.CandidateV2, contractStakingSR ContractStakingIndexer) error {
func addContractStakingVotes(candidate *iotextypes.CandidateV2, contractStakingSR ContractStakingIndexer, height uint64) error {
votes, ok := big.NewInt(0).SetString(candidate.TotalWeightedVotes, 10)
if !ok {
return errors.Errorf("invalid total weighted votes %s", candidate.TotalWeightedVotes)
Expand All @@ -318,7 +323,11 @@ func addContractStakingVotes(candidate *iotextypes.CandidateV2, contractStakingS
if err != nil {
return err
}
votes.Add(votes, contractStakingSR.CandidateVotes(addr))
contractVotes, err := contractStakingSR.CandidateVotes(addr, height)
if err != nil {
return err
}
votes.Add(votes, contractVotes)
candidate.TotalWeightedVotes = votes.String()
return nil
}
Expand Down
26 changes: 13 additions & 13 deletions action/protocol/staking/staking_statereader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestStakingStateReader(t *testing.T) {
sf.EXPECT().ReadView(gomock.Any()).Return(testNativeData, nil).Times(1)

contractIndexer := NewMockContractStakingIndexer(ctrl)
contractIndexer.EXPECT().Buckets().Return(testContractBuckets, nil).AnyTimes()
contractIndexer.EXPECT().Buckets(gomock.Any()).Return(testContractBuckets, nil).AnyTimes()

stakeSR, err := newCompositeStakingStateReader(contractIndexer, nil, sf)
r.NoError(err)
Expand Down Expand Up @@ -241,7 +241,7 @@ func TestStakingStateReader(t *testing.T) {
*arg0R = totalBucketCount{count: 1}
return uint64(1), nil
}).Times(1)
contractIndexer.EXPECT().BucketsByCandidate(gomock.Any()).DoAndReturn(func(arg0 address.Address) ([]*VoteBucket, error) {
contractIndexer.EXPECT().BucketsByCandidate(gomock.Any(), gomock.Any()).DoAndReturn(func(arg0 address.Address, arg1 uint64) ([]*VoteBucket, error) {
buckets := []*VoteBucket{}
for i := range testContractBuckets {
if testContractBuckets[i].Candidate.String() == arg0.String() {
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestStakingStateReader(t *testing.T) {
*arg0R = totalBucketCount{count: 1}
return uint64(1), nil
}).Times(1)
contractIndexer.EXPECT().BucketsByIndices(gomock.Any()).DoAndReturn(func(arg0 []uint64) ([]*VoteBucket, error) {
contractIndexer.EXPECT().BucketsByIndices(gomock.Any(), gomock.Any()).DoAndReturn(func(arg0 []uint64, arg1 uint64) ([]*VoteBucket, error) {
buckets := []*VoteBucket{}
for i := range arg0 {
buckets = append(buckets, testContractBuckets[arg0[i]])
Expand Down Expand Up @@ -318,7 +318,7 @@ func TestStakingStateReader(t *testing.T) {
*arg0R = totalBucketCount{count: 1}
return uint64(1), nil
}).Times(1)
contractIndexer.EXPECT().TotalBucketCount().Return(uint64(len(testContractBuckets))).Times(1)
contractIndexer.EXPECT().TotalBucketCount(gomock.Any()).Return(uint64(len(testContractBuckets)), nil).Times(1)
cfg := genesis.Default
cfg.GreenlandBlockHeight = 0
ctx = genesis.WithGenesisContext(ctx, cfg)
Expand All @@ -332,13 +332,13 @@ func TestStakingStateReader(t *testing.T) {
})
t.Run("readStateCandidates", func(t *testing.T) {
_, contractIndexer, stakeSR, ctx, r := prepare(t)
contractIndexer.EXPECT().CandidateVotes(gomock.Any()).DoAndReturn(func(ownerAddr address.Address) *big.Int {
contractIndexer.EXPECT().CandidateVotes(gomock.Any(), gomock.Any()).DoAndReturn(func(ownerAddr address.Address, height uint64) (*big.Int, error) {
for _, b := range testContractBuckets {
if b.Owner.String() == ownerAddr.String() {
return b.StakedAmount
return b.StakedAmount, nil
}
}
return big.NewInt(0)
return big.NewInt(0), nil
}).MinTimes(1)
req := &iotexapi.ReadStakingDataRequest_Candidates{
Pagination: &iotexapi.PaginationParam{
Expand All @@ -362,13 +362,13 @@ func TestStakingStateReader(t *testing.T) {
})
t.Run("readStateCandidateByName", func(t *testing.T) {
_, contractIndexer, stakeSR, ctx, r := prepare(t)
contractIndexer.EXPECT().CandidateVotes(gomock.Any()).DoAndReturn(func(ownerAddr address.Address) *big.Int {
contractIndexer.EXPECT().CandidateVotes(gomock.Any(), gomock.Any()).DoAndReturn(func(ownerAddr address.Address, height uint64) (*big.Int, error) {
for _, b := range testContractBuckets {
if b.Owner.String() == ownerAddr.String() {
return b.StakedAmount
return b.StakedAmount, nil
}
}
return big.NewInt(0)
return big.NewInt(0), nil
}).MinTimes(1)
req := &iotexapi.ReadStakingDataRequest_CandidateByName{
CandName: "cand1",
Expand All @@ -385,13 +385,13 @@ func TestStakingStateReader(t *testing.T) {
})
t.Run("readStateCandidateByAddress", func(t *testing.T) {
_, contractIndexer, stakeSR, ctx, r := prepare(t)
contractIndexer.EXPECT().CandidateVotes(gomock.Any()).DoAndReturn(func(ownerAddr address.Address) *big.Int {
contractIndexer.EXPECT().CandidateVotes(gomock.Any(), gomock.Any()).DoAndReturn(func(ownerAddr address.Address, height uint64) (*big.Int, error) {
for _, b := range testContractBuckets {
if b.Owner.String() == ownerAddr.String() {
return b.StakedAmount
return b.StakedAmount, nil
}
}
return big.NewInt(0)
return big.NewInt(0), nil
}).MinTimes(1)
req := &iotexapi.ReadStakingDataRequest_CandidateByAddress{
OwnerAddr: identityset.Address(1).String(),
Expand Down
Loading