[Access] Index tx results and events #4772
Conversation
339679f to
ebc6c8b
Compare
ebc6c8b to
eb2e242
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4772 +/- ##
==========================================
+ Coverage 55.79% 55.80% +0.01%
==========================================
Files 939 939
Lines 86860 86902 +42
==========================================
+ Hits 48462 48498 +36
+ Misses 34750 34749 -1
- Partials 3648 3655 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| log zerolog.Logger | ||
| db *badger.DB |
There was a problem hiding this comment.
I wish we could avoid passing an instance to the specific badger db, but I guess batch requires it? I don't like having a dependency on a specific implementation. I know this may call for a refactor, but my take on this would be to have an interface for batch (this is now even more true with implementing pebble since the same pattern will have to be established) and then have methods optionally supporting batch. Something like pseudo code:
batch := batcher.New()
s.events.Store(blockID, []flow.EventsList{result.AllEvents()}, storage.WithBatch(batch))
s.serviceEvents.Store(blockID, result.AllServiceEvents(), storage.WithBatch(batch))
batch.Flush()And then batcher instance is passed in as:
indexer.New(log, badger.NewBatcher(), registers, headers, events, results)I feel this would be a nice pattern because it will also allow the same for Pebble, but now the Batch type assumes only badger and is very implementation-specific. Just a thought.
There was a problem hiding this comment.
I think this is a good idea. There are other instances in the code where we do this so I'm reluctant to fold a larger refactor into this PR.
How about updating storage.NewBatch to take an interface instead of the whole *badger.DB instance? then all this needs is that interface?
There was a problem hiding this comment.
added here: f8c0c34
| @@ -227,6 +255,152 @@ func TestExecutionState_IndexBlockData(t *testing.T) { | |||
| assert.True(t, testRegisterFound) | |||
| }) | |||
|
|
|||
| t.Run("Index Events", func(t *testing.T) { | |||
Closes: #4739, #4674
This PR adds support for indexing
flow.LightTransactionResultsandflow.Eventsfrom ExecutionData as part of the data indexing.