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

MTG-750 Calculating bubblegum and account checksums #291

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

snorochevskiy
Copy link
Contributor

@snorochevskiy snorochevskiy commented Oct 22, 2024

This is a preliminary code review that doesn't intend to be me merged into the main branch, but aims to reveal problems before the development in completed.

In scope of review, I would like to present:

  • the DB structure for bubblegum changes and epoch checksums
  • the DB structure for account NFT changes
  • epoch and grand epoch checksums calculation
  • account buckets and grand buckets calculation
  • writing of bubblegum changes from the depth of bubblegum processor
  • removing of bubblegum changes in scope of forks cleaning
  • writing of account changes in account processor
  • emitting of epoch/buckets checksums calculation on epoch change
  • emitting of epoch/bucket -re-calculation on late data arrival

See the technical design:
https://github.com/adm-metaex/doc/wiki/Data-consistency-for-peer%E2%80%90to%E2%80%90peer-indexers

Copy link
Contributor

@n00m4d n00m4d left a comment

Choose a reason for hiding this comment

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

LGTM! Only one small question at the moment

hasher = Hasher::default();
}
hasher.hash(&k);
hasher.hash(&v);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we save value as well? Why not only key or value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@n00m4d
I assume that theoretically (not sure how high the probability is) we might have a situation, when on different peers there are bubblegum changes with same pubkey+slot+seq, but different signatures.
It you believe that this is impossible, then I can remove the signature from the bubblegum change entity (and from the hashing).

Copy link
Contributor

Choose a reason for hiding this comment

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

hmmm, not sure either. Ok let's leave it for now 😅

Copy link
Contributor

@n00m4d n00m4d left a comment

Choose a reason for hiding this comment

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

Oh and also while was looking at fork cleaner got a thought that maybe we should sync with sequence consistency checker before calculating epoch hash, not to calculate epoch hash if we have gaps in tree sequences.

@snorochevskiy snorochevskiy force-pushed the MTG-688_consistency_checksum branch 2 times, most recently from f3636c6 to b504cad Compare November 1, 2024 00:50
@snorochevskiy snorochevskiy changed the title MTG-750 Adding bubblegum checksums storage MTG-750 Calculating bubblegum and account checksums Nov 1, 2024
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