Skip to content
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

Save blockchain message #120

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Save blockchain message #120

merged 1 commit into from
Aug 29, 2024

Conversation

neekolas
Copy link
Contributor

@neekolas neekolas commented Aug 29, 2024

tl;dr

  • Implements the LogStorer interface to save messages from the blockchain
  • Adds test helpers to get dynamic config without relying on environment variables

Copy link
Contributor Author

neekolas commented Aug 29, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @neekolas and the rest of your teammates on Graphite Graphite

@neekolas neekolas force-pushed the 08-28-save_blockchain_message branch 4 times, most recently from f7b21e2 to 9724bcf Compare August 29, 2024 02:42
@neekolas neekolas requested a review from richardhuaaa August 29, 2024 02:47
@neekolas neekolas marked this pull request as ready for review August 29, 2024 02:50
@neekolas neekolas force-pushed the 08-28-save_blockchain_message branch from 9724bcf to 3c2bcb8 Compare August 29, 2024 05:06
@neekolas neekolas mentioned this pull request Aug 29, 2024
pkg/indexer/storer/groupMessage.go Show resolved Hide resolved
s.logger.Debug("Inserting message from contract", zap.String("topic", topic))

if _, err = s.queries.InsertGatewayEnvelope(ctx, queries.InsertGatewayEnvelopeParams{
// We may not want to hardcode this to 0 and have an originator ID for each smart contract?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understanding this a little bit better - what other smart contracts are we envisioning here? Is this related to sharding?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe upgradeability. At some point we are going to replace the smart contract and we have some more optionality if we've designed around it having its own originator ID where we can handle payloads differently.

Smart contract cutover would be a complicated migration to synchronize and I don't have all the answers on how to do it. But more options feels better.

OriginatorID: 0,
OriginatorSequenceID: int64(msgSent.SequenceId),
Topic: []byte(topic),
OriginatorEnvelope: msgSent.Message, // TODO:nm parse originator envelope and do some validation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably share this validation code, if you don't want to deal with it


s.logger.Debug("Inserting message from contract", zap.String("topic", topic))

if _, err = s.queries.InsertGatewayEnvelope(ctx, queries.InsertGatewayEnvelopeParams{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it right that we don't ever expect to process the same log twice? If so, is it problematic if we run into a unique constraint error here (which would mean the (originator ID, sequence ID) tuple already exists)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could if we have two instances of the replication node running, and both are indexing the chain.

If a node gets shut down mid-block, it'll also restart at the top of that block.

The only hard constraint is that they are both processing messages in ascending order.

Copy link
Contributor Author

neekolas commented Aug 29, 2024

Merge activity

  • Aug 29, 4:23 PM PDT: @neekolas started a stack merge that includes this pull request via Graphite.
  • Aug 29, 4:23 PM PDT: @neekolas merged this pull request with Graphite.

@neekolas neekolas merged commit ee504bd into main Aug 29, 2024
5 checks passed
@neekolas neekolas deleted the 08-28-save_blockchain_message branch August 29, 2024 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants