Initial implementation #1
Conversation
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
6 times, most recently
from
f2db6d5
to
c19aa51
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
3 times, most recently
from
5555560
to
7f73a6c
Compare
There was a problem hiding this comment.
This review is half-baked, but got some comments already, so I'm pushing those so far.
I expect to re-review tomorrow after meeting with @fernandofcampos
| - "emissions.v4.InsertReputerPayloadRequest" | ||
| ``` | ||
|
|
||
| You must use the fully qualified name of the protobuf type. |
There was a problem hiding this comment.
As we publish new versions, would it be good to be able to specify regexes here?
Eg to use any v of a particular type.
There was a problem hiding this comment.
Regex is a great idea, but I think we should implement that in a new iteration.
| ID: "0x" + hex.EncodeToString(hash[:]), | ||
| Type: msgType, | ||
| Name: name, | ||
| Timestamp: time.Now().UTC(), |
There was a problem hiding this comment.
shouldn't we use block timestamp instead?
There was a problem hiding this comment.
Ingestion timestamp is powerful to have for other reasons, lets say you're figuring out if a message was from a bootrapping or "catch up" process vs a realtime ingestion. I think block height would be a good additional field tho, perhaps part of a "context" KV pair and not directly in the domain event wrapper though
There was a problem hiding this comment.
He actually does that L53 of this file
app/service/monitor.go
Outdated
| return fmt.Errorf("block is nil") | ||
| } | ||
|
|
||
| for i, tx := range block.Block.Data.Txs { |
There was a problem hiding this comment.
We could create wg to parallelise this.
And in case of error, send block id to an error topic.
This type of error handling, where errors are sent to error topics, can be used for later reprocessing and awareness of entities that contained failures.
There was a problem hiding this comment.
I had implemented this, but the processing time ended up being the same. I believe that because the processing is 100% local and lightweight, we won't benefit on processing txs in parallel. At least not while the number of transactions per block is in the order of hundreds. So I rolled it back for the sake of simplification.
There was a problem hiding this comment.
Makes sense.
And also if we want to keep ordering on the write side, I take my previous comment back!
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
3 times, most recently
from
3639be3
to
d177cd8
Compare
| return err | ||
| } | ||
|
|
||
| record := &kgo.Record{ |
There was a problem hiding this comment.
A note, we may need to set key and value both, if we want to use partitioning, and generate the key accordingly (TBD)
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
from
feedabd
to
d41e7e3
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
2 times, most recently
from
d85eba5
to
dcd22bc
Compare
Signed-off-by: Fernando Campos <[email protected]>
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
2 times, most recently
from
6e82b5c
to
cff5a33
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
from
7692e74
to
2314549
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
from
5acc39d
to
26e6be4
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
from
0997028
to
c19b00d
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
2 times, most recently
from
6afa704
to
175b0a6
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
5 times, most recently
from
addd781
to
7fc1e62
Compare
fernandofcampos
force-pushed
the
fernando/initial-implementation
branch
from
7fc1e62
to
7cf5d21
Compare
- Made changes to the configs for the new topics - Made the new topics on confluent cloud Pending review from @fernandofcampos that the config changes looks good.
- add missing staking topics - divide staking topic into multiple topics (1 per event)
No description provided.