Fix store undo corruption on DELETE and bigdecimal min/max size inflation#819
Draft
sduchesneau wants to merge 2 commits into
Draft
Fix store undo corruption on DELETE and bigdecimal min/max size inflation#819sduchesneau wants to merge 2 commits into
sduchesneau wants to merge 2 commits into
Conversation
ApplyDeltasReverse returned from the reverse loop on the first DELETE delta, leaving all earlier deltas of the block unreverted and the store silently corrupted after a chain reorg.
The bigdecimal max/min merge branches used setNewKV on keys that already exist, adding key+value size without subtracting the previous value, so totalSizeBytes grew on every squash and could spuriously trip the max-store-size deterministic failure.
Contributor
Author
🔍 Vulnerabilities of
|
| digest | sha256:c14e42d86f1b5f330d988a6ff4c76d26090b86708600bb26c9d4465802ce4017 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 108 MB |
| packages | 353 |
📦 Base Image ubuntu:24.04
| also known as |
|
| digest | sha256:52df9b1ee71626e0088f7d400d5c6b5f7bb916f8f0c82b474289a4ece6cf3faf |
| vulnerabilities |   |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent store-integrity bugs in
storage/store:Fix 1:
ApplyDeltasReverseaborted on first DELETE delta (storage/store/delta.go)The reverse loop's
StoreDelta_DELETEcase ended withreturn, so reversing a block's deltas stopped at the first DELETE encountered (iterating backwards), leaving all earlier CREATE/UPDATE/DELETE deltas of that block unreverted. Any chain reorg (StepUndo→storesHandleUndo) over a block containing a DELETE therefore silently and permanently corrupted the store. Thereturnis removed so the loop reverses every delta.Fix 2:
totalSizeBytesinflation in bigdecimal min/max merge (storage/store/merge.go:281,359)The MAX and MIN merge branches for
bigfloat/bigdecimalcalledsetNewKVfor keys that already exist.setNewKVaddslen(key)+len(value)without subtracting the previous value's size, so every squash inflatedtotalSizeBytes, eventually tripping a spurious "store above max size" deterministic failure. Switched tosetKV(which adjusts for the previous value), matching what every other value type already does.Tests
TestApplyDeltasReverse(storage/store/delta_test.go): applies[CREATE k1, UPDATE k2, DELETE k3, DELETE k4]then reverses, asserting the full original state (kv andtotalSizeBytes) is restored. Fails before the fix, passes after.TestStore_Merge_TotalSizeBytes(storage/store/merge_test.go): min and max bigdecimal stores merged twice over existing keys, assertingtotalSizeBytesalways equals the actual kv content size and does not grow on same-size updates. Fails before the fix, passes after.go test ./storage/store/... ./pipeline/...passes.🤖 Generated with Claude Code