-
Notifications
You must be signed in to change notification settings - Fork 330
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.
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
clean up blockdao #4164
clean up blockdao #4164
Conversation
97d3daa
to
a31b03e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4164 +/- ##
==========================================
+ Coverage 75.38% 76.03% +0.64%
==========================================
Files 303 340 +37
Lines 25923 28979 +3056
==========================================
+ Hits 19541 22033 +2492
- Misses 5360 5851 +491
- Partials 1022 1095 +73 ☔ View full report in Codecov by Sentry. |
@@ -436,21 +446,17 @@ func TestBlockDAO(t *testing.T) { | |||
} | |||
} | |||
|
|||
func createTestBlockDAO(inMemory, legacy bool, compressBlock string, cfg db.Config) (BlockDAO, error) { | |||
func createFileDAO(inMemory, legacy bool, compressBlock string, cfg db.Config) (filedao.FileDAO, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createTestFileDAO
? this is code for testing use
log.L().Fatal(err.Error(), zap.Any("cfg", cfg)) | ||
// NewBlockDAOWithIndexersAndCache returns a BlockDAO with indexers which will consume blocks appended, and | ||
// caches which will speed up reading | ||
func NewBlockDAOWithIndexersAndCache(blkStore BlockDAO, indexers []BlockIndexer, cacheSize int) BlockDAO { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may just change func name to NewBlockDAOWithIndexersAndCache
, keep input param list the same
so we don't need to call explicitly fileDAO, err := filedao.NewFileDAO(cfg, deser)
upfront, then pass fileDAO
and call this.
return dao.blockStore.DeleteTipBlock() | ||
} | ||
|
||
func (dao *blockDAO) DeleteBlockToTarget(targetHeight uint64) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, these 2 need to be removed. there's an earlier PR but did not get merged
@@ -2395,4 +2431,17 @@ func classifyActions(actions []*action.SealedEnvelope) ([]*action.Transfer, []*a | |||
return tsfs, exes | |||
} | |||
|
|||
func ReceiptByActionHash(dao blockdao.BlockDAO, height uint64, h hash.Hash256) (*action.Receipt, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
receiptByActionHash
can work?
blockchain/blockdao/blockdao.go
Outdated
if err != nil { | ||
return nil, err | ||
} | ||
func receiptByActionHash(receipts []*action.Receipt, h hash.Hash256) (*action.Receipt, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can move this to _test.go
? seems only used in test
In the future, we might create blockreader and blockwriter separately from |
8be9da9
to
083250c
Compare
a2db8dd
to
f142eb6
Compare
Quality Gate failedFailed conditions |
struct blockDAO
is a wrapper of a BlockDAO with attached indexers and cache. This PR tries to clean up the blockdao related code and clarify the concept:BlockDAO
, including:GetActionByActionHash(hash.Hash256, uint64)
andGetReceiptByActionHash(hash.Hash256, uint64)
, which are not core functions of the interfaceDeleteBlockToTarget(uint64)
is not usable, because not all indexers could be revertedblockDAO
constructor functionFixes #(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: