-
Notifications
You must be signed in to change notification settings - Fork 492
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
Add indexerLedgerForEval
interface
#2897
Add indexerLedgerForEval
interface
#2897
Conversation
ledger/eval.go
Outdated
// in the trusting mode are excluded, and the present functions only request data | ||
// at the latest round. | ||
type indexerLedgerForEval interface { | ||
BlockHdr(basics.Round) (bookkeeping.BlockHeader, 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.
when do you need different block header rounds ?
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.
We only need to previous block header. Do you think I should simplify it?
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; Naming it LatestBlockHdr() would be a good name... and in indexerLedgerConnector. BlockHdr
you should test that round == blockHdr.Round(), or error. ( i.e. verify that the ledger wasn't really referring to a header which isn't the latest )
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.
btw - same applied for Totals()
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.
done
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.
Do you think that we can move the indexer related implementation to its own file ?
maybe indexereval.X ?
Good idea, I moved the indexer code. |
Codecov Report
@@ Coverage Diff @@
## master #2897 +/- ##
==========================================
- Coverage 47.27% 47.27% -0.01%
==========================================
Files 355 356 +1
Lines 57199 57257 +58
==========================================
+ Hits 27040 27067 +27
- Misses 27101 27125 +24
- Partials 3058 3065 +7
Continue to review full report at Codecov.
|
Looks good - could you please verify it passes all the auto tests ? |
Summary
Add a new ledger interface for Indexer that is significantly simpler and allows implementing batching in a more straight-forward way. As a result, the Indexer code doesn't need to know the specifics of go-algorand and doesn't need to implement its own accounts cache for batching.