[blockindex] restrict height for sgdindexer during write and read operation #3926
Conversation
envestcc
requested review from
CoderZhi,
dustinxie,
Liuhaai and
a team
as code owners
Codecov Report
@@ Coverage Diff @@
## master #3926 +/- ##
==========================================
+ Coverage 75.38% 76.05% +0.67%
==========================================
Files 303 328 +25
Lines 25923 27924 +2001
==========================================
+ Hits 19541 21238 +1697
- Misses 5360 5593 +233
- Partials 1022 1093 +71
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
action/protocol/execution/evm/evm.go
Outdated
| if err != nil { | ||
| return nil, 0, err | ||
| } | ||
| receiver, percentage, ok, err := sgd.CheckContract(ctx, execution.Contract(), height) |
There was a problem hiding this comment.
should be height-1? pls confirm
blockindex/sgd_indexer.go
Outdated
| if blk.Height() < sgd.startHeight { | ||
| if ignore, err := sgd.validateBlock(blk); err != nil { | ||
| return err | ||
| } else if ignore { |
There was a problem hiding this comment.
this is not very clean, i think just make it same as contractstaking.Indexer.PutBlock() which you fixed in #3924
blockindex/sgd_indexer.go
Outdated
| @@ -285,9 +285,21 @@ func (sgd *sgdRegistry) StartHeight() uint64 { | |||
|
|
|||
| // PutBlock puts a block into SGDIndexer | |||
| func (sgd *sgdRegistry) PutBlock(ctx context.Context, blk *block.Block) error { | |||
| if blk.Height() < sgd.startHeight { | |||
| tipHeight, err := sgd.Height() | |||
There was a problem hiding this comment.
wrap a function to calculate expectHeight
blockindex/sgd_indexer.go
Outdated
| if height == 0 { | ||
| return nil | ||
| } | ||
| tipHeight, err := sgd.Height() |
| @@ -481,6 +499,21 @@ func (sgd *sgdRegistry) FetchContracts(ctx context.Context) ([]*SGDIndex, error) | |||
| return sgdIndexes, nil | |||
| } | |||
|
|
|||
| func (sgd *sgdRegistry) validateQueryHeight(height uint64) error { | |||
| // 0 means latest height | |||
| if height == 0 { | |||
There was a problem hiding this comment.
generally unlikely to occur, this is merely implemented as a feature to support querying the latest height data.
| if err != nil { | ||
| return nil, 0, err | ||
| } | ||
| receiver, percentage, ok, err := sgd.CheckContract(ctx, execution.Contract(), height-1) |
There was a problem hiding this comment.
if height == 0, height - 1 will overflow
There was a problem hiding this comment.
a good reminder, but it is a hard-fork after Quebec at least, so height=0 will not occur.
There was a problem hiding this comment.
if height == 0,
height - 1will overflow
validate the height >= 1 before calling CheckContract
| @@ -285,14 +281,10 @@ func (sgd *sgdRegistry) StartHeight() uint64 { | |||
|
|
|||
| // PutBlock puts a block into SGDIndexer | |||
| func (sgd *sgdRegistry) PutBlock(ctx context.Context, blk *block.Block) error { | |||
| tipHeight, err := sgd.Height() | |||
| expectHeight, err := sgd.expectHeight() | |||
There was a problem hiding this comment.
nit: also do this for contract staking indexer? to keep it consistent
|
SonarCloud Quality Gate failed.
|
Description
Motivation:
Fixes #(issue)
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: