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

[contractstaking]fix vote bug when change delegate #3888

Merged
merged 17 commits into from
Jun 19, 2023
Merged

[contractstaking]fix vote bug when change delegate #3888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
4529cd4
fix not remove candidate map when change delegate
envestcc Jun 14, 2023
1702378
return clone data in read-only method of cache
envestcc Jun 15, 2023
ed174f4
add tests
envestcc Jun 15, 2023
b72f7bf
add unit test
envestcc Jun 15, 2023
54e5c20
fix totalbucketcount bug when loadfromdb
envestcc Jun 15, 2023
ee61724
add unit test
envestcc Jun 15, 2023
c7500e2
refactor: move Height and TotalBucketType out of delta
envestcc Jun 16, 2023
640581c
refactor: use deltaStateUnchanged instead of deltaStateReverted
envestcc Jun 16, 2023
54f26db
add unit test for delta_state
envestcc Jun 16, 2023
b98ff2a
address comments
envestcc Jun 16, 2023
da5edb2
address comments
envestcc Jun 17, 2023
2cae048
address comments
envestcc Jun 18, 2023
64c7f8e
move height from cache to indexer
envestcc Jun 19, 2023
e131946
address comments
envestcc Jun 19, 2023
a504f59
Update cache.go
dustinxie Jun 19, 2023
a6e6d03
Update cache.go
dustinxie Jun 19, 2023
325df6c
Update indexer.go
dustinxie Jun 19, 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
9 changes: 9 additions & 0 deletions action/protocol/staking/contractstake_bucket_type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ func (bt *ContractStakingBucketType) Deserialize(b []byte) error {
return bt.loadProto(&m)
}

// Clone clones the bucket type
func (bt *ContractStakingBucketType) Clone() *ContractStakingBucketType {
dustinxie marked this conversation as resolved.
Show resolved Hide resolved
return &ContractStakingBucketType{
Amount: big.NewInt(0).Set(bt.Amount),
Duration: bt.Duration,
ActivatedAt: bt.ActivatedAt,
}
}

func (bt *ContractStakingBucketType) toProto() *stakingpb.BucketType {
return &stakingpb.BucketType{
Amount: bt.Amount.String(),
Expand Down
20 changes: 20 additions & 0 deletions blockindex/contractstaking/bucket_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,26 @@ func (bi *bucketInfo) Deserialize(b []byte) error {
return bi.loadProto(&m)
}

// clone clones the bucket info
func (bi *bucketInfo) clone() *bucketInfo {
delegate := bi.Delegate
if delegate != nil {
delegate, _ = address.FromBytes(delegate.Bytes())
}
owner := bi.Owner
if owner != nil {
owner, _ = address.FromBytes(owner.Bytes())
}
return &bucketInfo{
TypeIndex: bi.TypeIndex,
CreatedAt: bi.CreatedAt,
UnlockedAt: bi.UnlockedAt,
UnstakedAt: bi.UnstakedAt,
Delegate: delegate,
Owner: owner,
}
}

func (bi *bucketInfo) toProto() *contractstakingpb.BucketInfo {
pb := &contractstakingpb.BucketInfo{
TypeIndex: bi.TypeIndex,
Expand Down
113 changes: 38 additions & 75 deletions blockindex/contractstaking/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ type (
candidateBucketMap map[string]map[uint64]bool // map[candidate]bucket
bucketTypeMap map[uint64]*BucketType // map[bucketTypeId]BucketType
propertyBucketTypeMap map[int64]map[uint64]uint64 // map[amount][duration]index
height uint64
totalBucketCount uint64 // total number of buckets including burned buckets
contractAddress string // contract address for the bucket
mutex sync.RWMutex // a RW mutex for the cache to protect concurrent access
totalBucketCount uint64 // total number of buckets including burned buckets
contractAddress string // contract address for the bucket
mutex sync.RWMutex // a RW mutex for the cache to protect concurrent access
}
)

Expand All @@ -44,12 +43,6 @@ func newContractStakingCache(contractAddr string) *contractStakingCache {
}
}

func (s *contractStakingCache) Height() uint64 {
s.mutex.RLock()
defer s.mutex.RUnlock()
return s.height
}

func (s *contractStakingCache) CandidateVotes(candidate address.Address) *big.Int {
s.mutex.RLock()
defer s.mutex.RUnlock()
Expand Down Expand Up @@ -79,9 +72,9 @@ func (s *contractStakingCache) Buckets() []*Bucket {
defer s.mutex.RUnlock()

vbs := []*Bucket{}
for id, bi := range s.getAllBucketInfo() {
for id, bi := range s.bucketInfoMap {
bt := s.mustGetBucketType(bi.TypeIndex)
vb := assembleBucket(id, bi, bt, s.contractAddress)
vb := assembleBucket(id, bi.clone(), bt, s.contractAddress)
vbs = append(vbs, vb)
}
return vbs
Expand Down Expand Up @@ -126,7 +119,7 @@ func (s *contractStakingCache) BucketsByCandidate(candidate address.Address) []*
s.mutex.RLock()
defer s.mutex.RUnlock()

bucketMap := s.getBucketInfoByCandidate(candidate)
bucketMap := s.candidateBucketMap[candidate.String()]
vbs := make([]*Bucket, 0, len(bucketMap))
for id := range bucketMap {
vb := s.mustGetBucket(id)
Expand All @@ -153,7 +146,7 @@ func (s *contractStakingCache) TotalBucketCount() uint64 {
s.mutex.RLock()
defer s.mutex.RUnlock()

return s.getTotalBucketCount()
return s.totalBucketCount
}

func (s *contractStakingCache) ActiveBucketTypes() map[uint64]*BucketType {
Expand All @@ -163,7 +156,7 @@ func (s *contractStakingCache) ActiveBucketTypes() map[uint64]*BucketType {
m := make(map[uint64]*BucketType)
for k, v := range s.bucketTypeMap {
if v.ActivatedAt != maxBlockNumber {
m[k] = v
m[k] = v.Clone()
}
}
return m
Expand All @@ -190,18 +183,15 @@ func (s *contractStakingCache) DeleteBucketInfo(id uint64) {
s.deleteBucketInfo(id)
}

func (s *contractStakingCache) PutHeight(h uint64) {
s.mutex.Lock()
defer s.mutex.Unlock()

s.putHeight(h)
}

func (s *contractStakingCache) Merge(delta *contractStakingDelta) error {
s.mutex.Lock()
defer s.mutex.Unlock()

return s.merge(delta)
if err := s.merge(delta); err != nil {
return err
}
s.putTotalBucketCount(s.totalBucketCount + delta.AddedBucketCnt())
return nil
}

func (s *contractStakingCache) PutTotalBucketCount(count uint64) {
Copy link
Member

Choose a reason for hiding this comment

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

what's the purpose of a public PutTotalBucketCount func?

Copy link
Member Author

@envestcc envestcc Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

the two principles could easily avoid dead lock:

  • Lock & Unlock is called in public method instead of private
  • public method can't call public

Copy link
Member

Choose a reason for hiding this comment

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

seems only used in test code, could be removed

Expand Down Expand Up @@ -233,21 +223,6 @@ func (s *contractStakingCache) LoadFromDB(kvstore db.KVStore) error {
s.mutex.Lock()
defer s.mutex.Unlock()

delta := newContractStakingDelta()
// load height
var height uint64
h, err := kvstore.Get(_StakingNS, _stakingHeightKey)
if err != nil {
if !errors.Is(err, db.ErrNotExist) {
return err
}
height = 0
} else {
height = byteutil.BytesToUint64BigEndian(h)

}
delta.PutHeight(height)

// load total bucket count
var totalBucketCount uint64
tbc, err := kvstore.Get(_StakingNS, _stakingTotalBucketCountKey)
Expand All @@ -258,7 +233,7 @@ func (s *contractStakingCache) LoadFromDB(kvstore db.KVStore) error {
} else {
totalBucketCount = byteutil.BytesToUint64BigEndian(tbc)
}
delta.PutTotalBucketCount(totalBucketCount)
s.putTotalBucketCount(totalBucketCount)

// load bucket info
ks, vs, err := kvstore.Filter(_StakingBucketInfoNS, func(k, v []byte) bool { return true }, nil, nil)
Expand All @@ -270,7 +245,7 @@ func (s *contractStakingCache) LoadFromDB(kvstore db.KVStore) error {
if err := b.Deserialize(vs[i]); err != nil {
return err
}
delta.AddBucketInfo(byteutil.BytesToUint64BigEndian(ks[i]), &b)
s.putBucketInfo(byteutil.BytesToUint64BigEndian(ks[i]), &b)
}

// load bucket type
Expand All @@ -283,9 +258,9 @@ func (s *contractStakingCache) LoadFromDB(kvstore db.KVStore) error {
if err := b.Deserialize(vs[i]); err != nil {
return err
}
delta.AddBucketType(byteutil.BytesToUint64BigEndian(ks[i]), &b)
s.putBucketType(byteutil.BytesToUint64BigEndian(ks[i]), &b)
}
return s.merge(delta)
return nil
}

func (s *contractStakingCache) getBucketTypeIndex(amount *big.Int, duration uint64) (uint64, bool) {
Expand All @@ -299,7 +274,10 @@ func (s *contractStakingCache) getBucketTypeIndex(amount *big.Int, duration uint

func (s *contractStakingCache) getBucketType(id uint64) (*BucketType, bool) {
bt, ok := s.bucketTypeMap[id]
return bt, ok
if !ok {
return nil, false
}
return bt.Clone(), ok
}

func (s *contractStakingCache) mustGetBucketType(id uint64) *BucketType {
Expand All @@ -312,7 +290,10 @@ func (s *contractStakingCache) mustGetBucketType(id uint64) *BucketType {

func (s *contractStakingCache) getBucketInfo(id uint64) (*bucketInfo, bool) {
bi, ok := s.bucketInfoMap[id]
return bi, ok
if !ok {
return nil, false
}
return bi.clone(), ok
}

func (s *contractStakingCache) mustGetBucketInfo(id uint64) *bucketInfo {
Expand All @@ -338,28 +319,6 @@ func (s *contractStakingCache) getBucket(id uint64) (*Bucket, bool) {
return assembleBucket(id, bi, bt, s.contractAddress), true
}

func (s *contractStakingCache) getAllBucketInfo() map[uint64]*bucketInfo {
m := make(map[uint64]*bucketInfo)
for k, v := range s.bucketInfoMap {
m[k] = v
}
return m
}

func (s *contractStakingCache) getBucketInfoByCandidate(candidate address.Address) map[uint64]*bucketInfo {
m := make(map[uint64]*bucketInfo)
for k, v := range s.candidateBucketMap[candidate.String()] {
if v {
m[k] = s.bucketInfoMap[k]
}
}
return m
}

func (s *contractStakingCache) getTotalBucketCount() uint64 {
return s.totalBucketCount
}

func (s *contractStakingCache) putBucketType(id uint64, bt *BucketType) {
amount := bt.Amount.Int64()
s.bucketTypeMap[id] = bt
Expand All @@ -372,11 +331,21 @@ func (s *contractStakingCache) putBucketType(id uint64, bt *BucketType) {
}

func (s *contractStakingCache) putBucketInfo(id uint64, bi *bucketInfo) {
oldBi := s.bucketInfoMap[id]
Copy link
Member

Choose a reason for hiding this comment

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

Tests for the issue


func TestContractStakingCache_Debug(t *testing.T) {
	cache := newContractStakingCache("")

	cache.PutBucketType(1, &BucketType{Amount: big.NewInt(100), Duration: 100, ActivatedAt: 1})
	cache.PutBucketType(1, &BucketType{Amount: big.NewInt(100), Duration: 200, ActivatedAt: 1})
	cache.PutBucketType(1, &BucketType{Amount: big.NewInt(200), Duration: 100, ActivatedAt: 1})
	log.L().Info("debug", zap.Any("propertyBucketTypeMap", cache.propertyBucketTypeMap))
	cache.PutBucketInfo(1, &bucketInfo{TypeIndex: 1, CreatedAt: 1, UnlockedAt: maxBlockNumber, UnstakedAt: maxBlockNumber, Delegate: identityset.Address(1), Owner: identityset.Address(2)})
	cache.PutBucketInfo(1, &bucketInfo{TypeIndex: 1, CreatedAt: 1, UnlockedAt: maxBlockNumber, UnstakedAt: maxBlockNumber, Delegate: identityset.Address(2), Owner: identityset.Address(3)})
	log.L().Info("debug", zap.Any("candidateBucketMap", cache.candidateBucketMap))

}

code for reference

func (s *contractStakingCache) putBucketType(id uint64, bt *BucketType) {
	if oldBt, existed := s.bucketTypeMap[id]; existed {
		amount := oldBt.Amount.Int64()
		if _, existed := s.propertyBucketTypeMap[amount]; !existed {
			panic("property bucket type map not found")
		}
		delete(s.propertyBucketTypeMap[amount], oldBt.Duration)
		if len(s.propertyBucketTypeMap[amount]) == 0 {
			delete(s.propertyBucketTypeMap, amount)
		}
	}
	s.bucketTypeMap[id] = bt
	amount := bt.Amount.Int64()
	if _, ok := s.propertyBucketTypeMap[amount]; !ok {
		s.propertyBucketTypeMap[amount] = make(map[uint64]uint64)
	}
	s.propertyBucketTypeMap[amount][bt.Duration] = id
}

func (s *contractStakingCache) putBucketInfo(id uint64, bi *bucketInfo) {
	// delete old candidate bucket map
	if oldBi, existed := s.bucketInfoMap[id]; existed {
		oldDelegate := oldBi.Delegate.String()
		if _, existed := s.candidateBucketMap[oldDelegate]; !existed {
			panic("candidate bucket map not found") // TODO: remove this panic
		}
		delete(s.candidateBucketMap[oldDelegate], id)
		if len(s.candidateBucketMap[oldDelegate]) == 0 {
			delete(s.candidateBucketMap, oldDelegate)
		}
	}
	s.bucketInfoMap[id] = bi
	// update candidate bucket map
	newDelegate := bi.Delegate.String()
	if _, ok := s.candidateBucketMap[newDelegate]; !ok {
		s.candidateBucketMap[newDelegate] = make(map[uint64]bool)
	}
	s.candidateBucketMap[newDelegate][id] = true
}

s.bucketInfoMap[id] = bi
if _, ok := s.candidateBucketMap[bi.Delegate.String()]; !ok {
s.candidateBucketMap[bi.Delegate.String()] = make(map[uint64]bool)
// update candidate bucket map
newDelegate := bi.Delegate.String()
if _, ok := s.candidateBucketMap[newDelegate]; !ok {
s.candidateBucketMap[newDelegate] = make(map[uint64]bool)
}
s.candidateBucketMap[newDelegate][id] = true
// delete old candidate bucket map
if oldBi != nil {
oldDelegate := oldBi.Delegate.String()
if oldDelegate != newDelegate {
delete(s.candidateBucketMap[oldDelegate], id)
dustinxie marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

will contractStakingCache.merge() always return nil?

}
}
dustinxie marked this conversation as resolved.
Show resolved Hide resolved
s.candidateBucketMap[bi.Delegate.String()][id] = true
}

func (s *contractStakingCache) deleteBucketInfo(id uint64) {
Expand All @@ -391,10 +360,6 @@ func (s *contractStakingCache) deleteBucketInfo(id uint64) {
delete(s.candidateBucketMap[bi.Delegate.String()], id)
}

func (s *contractStakingCache) putHeight(h uint64) {
s.height = h
}

func (s *contractStakingCache) putTotalBucketCount(count uint64) {
s.totalBucketCount = count
}
Expand All @@ -418,7 +383,5 @@ func (s *contractStakingCache) merge(delta *contractStakingDelta) error {
}
}
}
s.putHeight(delta.GetHeight())
s.putTotalBucketCount(s.getTotalBucketCount() + delta.AddedBucketCnt())
return nil
}
Loading