-
Notifications
You must be signed in to change notification settings - Fork 39
Teach blockchain publisher to bootstrap messages #998
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
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
| return logs[0], nil | ||
| } | ||
|
|
||
| func (m *BlockchainPublisher) BootstrapGroupMessages( |
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.
With multiple messages it becomes easier for us to exceed the limits on a single tx. Are we planning some mechanism to break up multiple payloads into smaller chunks if we get errors there?
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.
I plan to use this function to bootstrap one message at a time. Initially I don't think we need to batch anything and risk reverting tx's.
The amount of messages to backfill can be easily migrated in a few days, and then the migrator would continue pulling these on the fly.
If down the road we're confident this can be used with batches then I'll modify it to guarantee the data published in the batch is not bigger than the max data a tx can hold.
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.
Glad I asked. That makes sense
| return nil, err | ||
| } | ||
|
|
||
| return logs[0], nil |
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.
make sure the size is at least 1. Otherwise this will segfault. if len(logs) != 1
| parseFunc func(types.Log) (*T, error), | ||
| expectedEventCount int, | ||
| ) ([]*T, error) { | ||
| events := make([]*T, 0) |
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.
pre-allocate the right size here?
| return nil, err | ||
| } | ||
|
|
||
| return logs[0], nil |
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.
same. Make sure to check the array size before accessing by index.
Add batch publishing methods
BootstrapGroupMessagesandBootstrapIdentityUpdatestoBlockchainPublisherfor publishing multiple messages in single transactionsBootstrapGroupMessagesandBootstrapIdentityUpdatesmethods toBlockchainPublisherenabling batch publishing of multiple group messages or identity updates in single blockchain transactionsfindLogfunction withfindLogsfunction that returns multiple logs and validates expected event counts with newErrNoLogsFounderror handlingPublishGroupMessageandPublishIdentityUpdatemethods to use the newfindLogsfunction and updates callback return types inwithNonceto support slicesblockchainPublisher_test.gowith error handling validation for various scenarios📍Where to Start
Start with the new
BootstrapGroupMessagesandBootstrapIdentityUpdatesmethods inblockchainPublisher.goto understand the batch publishing functionality.Changes since #998 opened
findLogsfunction to individual publisher methods [a481e8f]findLogsfunction [a481e8f]Macroscope summarized a481e8f.