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

fix!: just in time updates would break references in same batch #329

Merged
merged 20 commits into from
Aug 20, 2024

Conversation

QuantumExplorer
Copy link
Member

@QuantumExplorer QuantumExplorer commented Aug 20, 2024

Issue being fixed or feature implemented

There was a very bad issue that would lead to state corruption. To understand the issue we first need a refresher on just in time updates. Just in time updates are updates that can modify a value being updated during batch execution. This is important for our fee system in drive. For example if you have a field "cat" that was 3 bytes and you added it in epoch 0, then modified it to cats in epoch 1. you would have added 3 bytes in epoch 0 and 1 byte in epoch 1.

Just in time updates does the merge automatically so the client library only needs to push "cats", then GroveDB sees that there is a value already present "cat" and asks the client library to merge the element flags (which in drive contain information about the byte storage).

References in a batch would not take into account this merge and would instead use the unmerged inserted value. This would lead to incorrect hashes being stored for references which would break the proof system.

We also found an issue with Merks being required too often which is fixed in this PR.

What was done?

When we insert an item or a sum item, we merge the element flags before assigning to the reference. This is not cached and there are improvements to be had on this system.
Merks are now properly cached and are not requeried as often.
There was some refactoring done and some renaming as well to make this fix easier to understand.

How Has This Been Tested?

Added many tests.

Breaking Changes

This is a breaking change.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

grovedb/build.rs Outdated Show resolved Hide resolved
@QuantumExplorer QuantumExplorer merged commit 713ad96 into develop Aug 20, 2024
7 checks passed
@QuantumExplorer QuantumExplorer deleted the verify_ref_after_batch_op branch August 20, 2024 11:42
@QuantumExplorer QuantumExplorer restored the verify_ref_after_batch_op branch August 20, 2024 11:49
@QuantumExplorer QuantumExplorer deleted the verify_ref_after_batch_op branch August 24, 2024 11:27
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.

3 participants