[IIP-13] handle contract staking events to store buckets into index (3/3 split by 3853) #3863

envestcc · 2023-05-19T17:31:59Z

Description

For #3853 is too large to review, it has been splited into three small pr. This is the last one.
It's based on #3862

Fixes #(issue)

Type of change

Please delete options that are not relevant.

[] Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
[] Code refactor or improvement
[] Breaking change (fix or feature that would cause a new or changed behavior of existing functionality)
[] This change requires a documentation update

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 · 2023-05-19T17:46:31Z

Codecov Report

Merging #3863 (6f4939a) into master (e1f0636) will decrease coverage by 0.27%.
The diff coverage is 64.17%.

❗ Current head 6f4939a differs from pull request most recent head 122b66b. Consider uploading reports for the commit 122b66b to get more accurate results

@@            Coverage Diff             @@
##           master    #3863      +/-   ##
==========================================
- Coverage   75.38%   75.12%   -0.27%     
==========================================
  Files         303      316      +13     
  Lines       25923    26926    +1003     
==========================================
+ Hits        19541    20227     +686     
- Misses       5360     5662     +302     
- Partials     1022     1037      +15

Impacted Files	Coverage Δ
action/protocol/execution/evm/evm.go	`43.52% <0.00%> (-2.95%)`	⬇️
action/protocol/execution/evm/evmstatedbadapter.go	`66.77% <ø> (ø)`
action/protocol/poll/consortium.go	`0.00% <0.00%> (ø)`
action/protocol/poll/staking_committee.go	`43.85% <0.00%> (ø)`
action/protocol/staking/staking_statereader.go	`0.00% <0.00%> (ø)`
api/web3server_marshal.go	`93.21% <ø> (ø)`
blockindex/sgd_indexer.go	`0.00% <0.00%> (ø)`
action/protocol/staking/protocol.go	`60.50% <16.66%> (+0.62%)`	⬆️
api/web3server_utils.go	`69.20% <16.66%> (-3.67%)`	⬇️
action/protocol/rewarding/fund.go	`64.70% <23.07%> (-18.91%)`	⬇️
... and 21 more

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

CoderZhi · 2023-05-22T01:02:53Z

action/protocol/staking/contractstaking/cache.go

 }

-func (s *contractStakingCache) getTotalBucketTypeCount() uint64 {
+func (s *contractStakingCache) GetTotalBucketTypeCount() uint64 {


rename to BucketTypeCount, and move to public function group

CoderZhi · 2023-05-22T01:04:52Z

action/protocol/staking/contractstaking/indexer.go

+	// new dirty cache for this block
+	// it's not necessary to use thread safe cache here, because only one thread will call this function
+	// and no update to cache will happen before dirty merge to clean
+	dirty := newContractStakingDirty(s.cache.Unsafe())


it doesn't hurt much to use CacheSafe, thus, Cache and CacheSafe could be merged into one.

ok, removed CacheSafe and move the mutex into Cache

Liuhaai · 2023-05-23T16:57:30Z

action/protocol/staking/contractstaking/indexer.go

@@ -82,3 +105,110 @@ func (s *Indexer) BucketTypes() ([]*BucketType, error) {
 	}
 	return bts, nil
 }
+
+// PutBlock puts a block into indexer
+func (s *Indexer) PutBlock(ctx context.Context, blk *block.Block) error {


make sure the indexer-related code/files are in /blockindex

action/protocol/staking/contractstaking/indexer.go

Liuhaai · 2023-05-23T18:43:00Z

action/protocol/staking/contractstaking/cache.go

@@ -21,6 +22,7 @@ type (
 		height                uint64
 		totalBucketCount      uint64 // total number of buckets including burned buckets
 		contractAddress       string // contract address for the bucket
+		mutex                 sync.RWMutex


On the L21, is a map necessary for amount-duration indexing? (No comment on the mtx)

yes, the params of stake are the amount and duration, this map can help get the bucket type

Liuhaai · 2023-05-23T18:43:59Z

action/protocol/staking/contractstaking/dirty_cache.go

+func (dirty *contractStakingDirty) PutBucketType(bt *BucketType) error {
+	id, _, ok := dirty.matchBucketType(bt.Amount, bt.Duration)
+	if !ok {
+		id = dirty.getBucketTypeCount()


what's the reason to use getBucketTypeCount as id?

id of the BucketType starts from 0, which is consistent with the staking contract

action/protocol/staking/contractstaking/delta_state.go

dustinxie · 2023-05-24T04:35:23Z

action/protocol/staking/contractstaking/delta_state.go

+	deltaStateAdded deltaState = iota
+	deltaStateRemoved
+	deltaStateModified
+	deltaStateReverted


what does Reverted mean?

it means a state where is added and then removed, it's reverted with the original state. If state is reverted, there is no need to be concerned about it.

dustinxie · 2023-05-24T04:37:51Z

action/protocol/staking/contractstaking/dirty_cache.go

+	// bucket related namespace in db
+	_StakingBucketInfoNS = "sbi"
+	_StakingBucketTypeNS = "sbt"
+	_StakingNS           = "s"


use a 2-letter namespace to be consistent

dustinxie · 2023-05-24T04:42:07Z

action/protocol/staking/contractstaking/indexer.go

+		return err
+	}
+	if err := s.kvstore.WriteBatch(batch); err != nil {
+		s.reloadCache()


if write fails, just discard dirty cache? can we avoid reading all items from the kvStore? that should only be needed in Start() once

to handle the retry of PutBlock

dustinxie · 2023-05-24T04:47:10Z

action/protocol/staking/contractstaking/dirty_cache.go

+	}
+}
+
+func (dirty *contractStakingDirty) PutHeight(h uint64) {


these funcs need not be public?

dustinxie · 2023-05-24T04:47:42Z

action/protocol/staking/contractstaking/delta_cache.go

+	}
+}
+
+func (s *contractStakingDelta) PutHeight(height uint64) {


these funcs need not be public?

dustinxie · 2023-05-24T05:32:40Z

action/protocol/staking/contractstaking/indexer.go

+	// dirty is the cache to update during event processing, which will be merged to clean cache after all events are processed.
+	// if errors occur during event processing, dirty cache will be discarded.
+	dirty := newContractStakingDirty(s.cache)
+	dirty.PutHeight(blk.Height())


convert these to cache's func

cache.NewWorkingset(blk.Height())

instead of indexer --> cache, dirty
indexer --> cache
let cache handle --> dirty inside

make the indexer unaware of the dirty

sonarcloud · 2023-05-24T14:54:23Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
12 Code Smells

No Coverage information
3.7% Duplication

dustinxie · 2023-05-25T00:06:32Z

action/protocol/staking/contractstaking/cache.go

@@ -247,6 +230,65 @@ func (s *contractStakingCache) BucketTypeCount() uint64 {
 	return uint64(len(s.bucketTypeMap))
 }

+func (s *contractStakingCache) LoadFromDB(kvstore db.KVStore) error {


loadFromDB(), this is internal func
also, do we have a test to test this func?

dustinxie · 2023-05-25T01:00:56Z

action/protocol/staking/contractstaking/cache.go

+		}
+	}
+	s.putHeight(delta.GetHeight())
+	s.putTotalBucketCount(s.getTotalBucketCount() + delta.AddedBucketCnt())


move these 2 lines out of merge

envestcc added 4 commits May 19, 2023 20:02

contractstaking bucket definition

955313e

indexer read

37b2ea6

handle contract events to store buckets

5aeca92

add tests

01508b5

envestcc mentioned this pull request May 19, 2023

[IIP-13] implement liquid staking indexer #3853

Closed

10 tasks

envestcc marked this pull request as ready for review May 19, 2023 17:35

envestcc requested review from CoderZhi, dustinxie, Liuhaai and a team as code owners May 19, 2023 17:35

CoderZhi reviewed May 22, 2023

View reviewed changes

envestcc added 2 commits May 22, 2023 17:02

address comments

7a745bd

Merge branch 'iip13_v2' into iip13_v2_2

979812f

envestcc mentioned this pull request May 22, 2023

[IIP-13] introduce read contract staking buckets (2/3 split by 3853) #3862

Merged

10 tasks

envestcc added 6 commits May 22, 2023 17:37

address comments

8c1f5a0

Merge branch 'iip13_v2_2' into iip13_v2_3

1613411

address comments

98490ae

address comments

a4ec390

Merge branch 'master' into iip13_v2_3

a04f3f9

address comments

60f7f9d

Liuhaai requested changes May 23, 2023

View reviewed changes

Liuhaai reviewed May 23, 2023

View reviewed changes

dustinxie reviewed May 24, 2023

View reviewed changes

wrap dirty into event handler

f1119f7

envestcc added 4 commits May 24, 2023 21:42

address comments

877b32f

move loadcache into cache

addd27a

move to blockindex

9ff634c

fix lint error

122b66b

Liuhaai approved these changes May 24, 2023

View reviewed changes

Liuhaai mentioned this pull request May 24, 2023

[IIP-13] combine native and LSD buckets #3859

Merged

10 tasks

dustinxie reviewed May 25, 2023

View reviewed changes

dustinxie approved these changes May 25, 2023

View reviewed changes

envestcc merged commit 0ba92d0 into iotexproject:master May 25, 2023

envestcc deleted the iip13_v2_3 branch May 25, 2023 01:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IIP-13] handle contract staking events to store buckets into index (3/3 split by 3853) #3863

[IIP-13] handle contract staking events to store buckets into index (3/3 split by 3853) #3863

envestcc commented May 19, 2023

codecov bot commented May 19, 2023 •

edited

Loading

CoderZhi May 22, 2023

envestcc May 22, 2023

CoderZhi May 22, 2023

envestcc May 22, 2023

Liuhaai May 23, 2023 •

edited

Loading

envestcc May 24, 2023

Liuhaai May 23, 2023

envestcc May 24, 2023

Liuhaai May 23, 2023

envestcc May 24, 2023

dustinxie May 24, 2023

envestcc May 24, 2023

dustinxie May 24, 2023

envestcc May 24, 2023

dustinxie May 24, 2023 •

edited

Loading

envestcc May 24, 2023

dustinxie May 24, 2023

envestcc May 24, 2023

dustinxie May 24, 2023

dustinxie May 24, 2023

dustinxie May 24, 2023

dustinxie May 24, 2023

envestcc May 24, 2023

sonarcloud bot commented May 24, 2023

dustinxie May 25, 2023

dustinxie May 25, 2023

[IIP-13] handle contract staking events to store buckets into index (3/3 split by 3853) #3863

[IIP-13] handle contract staking events to store buckets into index (3/3 split by 3853) #3863

Conversation

envestcc commented May 19, 2023

Description

Type of change

How Has This Been Tested?

Checklist:

codecov bot commented May 19, 2023 • 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

Liuhaai May 23, 2023 • 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

dustinxie May 24, 2023 • 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

sonarcloud bot commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 19, 2023 •

edited

Loading

Liuhaai May 23, 2023 •

edited

Loading

dustinxie May 24, 2023 •

edited

Loading