Skip to content

binarytree: fix updated value in non empty tree bug #3893

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.

Sign up for GitHub

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

Merged
acolytec3 merged 2 commits into from
Mar 3, 2025
Merged

binarytree: fix updated value in non empty tree bug #3893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54adb83
binarytree: updated value in non empty tree test case
gabrocheleau Mar 1, 2025
1c04802
binarytree: fix update logic
gabrocheleau Mar 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions packages/binarytree/src/binaryTree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ export class BinaryTree {

// Step 1) Create or update the stem node
let stemNode: StemBinaryNode
let newStem = false
// If we found a stem node with the same stem, we'll update it.
if (
foundPath.node &&
Expand All @@ -196,12 +195,11 @@ export class BinaryTree {
stemNode = foundPath.node
} else {
// Otherwise, we'll create a new stem node.
newStem = true
stemNode = StemBinaryNode.create(stem)
this.DEBUG && this.debug(`Creating new stem node for stem: ${bytesToHex(stem)}`, ['put'])
}

// Update the values in the stem node.
// Update the values in the stem node
for (let i = 0; i < suffixes.length; i++) {
const suffix = suffixes[i]
const value = values[i]
Expand Down Expand Up @@ -232,7 +230,8 @@ export class BinaryTree {
let lastUpdatedParentPath: number[] = []

// Step 2: Add any needed new internal nodes if inserting a new stem.
if (foundPath.stack.length > 1 && newStem) {
// If updating an existing stem, just update the parent internal node reference
if (foundPath.stack.length > 1) {
// Pop the nearest node on the path.
const [nearestNode, nearestNodePath] = foundPath.stack.pop()!
const parentPath = foundPath.stack[foundPath.stack.length - 1]?.[1] ?? []
Expand All @@ -253,7 +252,7 @@ export class BinaryTree {
while (foundPath.stack.length > 1) {
const [node, path] = foundPath.stack.pop()!
if (isInternalBinaryNode(node)) {
// Set child pointer to the last internal node in the putStack (last updated internal node)
// Set child pointer to the last internal node in the putStack (last updated internal node)
node.setChild(lastUpdatedParentPath[lastUpdatedParentPath.length - 1], {
hash: putStack[putStack.length - 1][0], // Reuse hash already computed above
path: lastUpdatedParentPath,
Expand Down
39 changes: 24 additions & 15 deletions packages/binarytree/test/binarytree.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,21 +432,30 @@ describe('insert', () => {
'Tree roots should match regardless of insertion order',
)

// Ensure that updates on existing stem/index pairs are reflected in the root and result in consistent trees
const preUpdateRoot = tree1.root()

// Insert a new value on an existing stem and verify the roots match
await tree1.put(
keyValuePairs[0].originalKey.slice(0, 31),
[6],
[hexToBytes(`0x${'06'.repeat(32)}`)],
)
await tree2.put(
keyValuePairs[0].originalKey.slice(0, 31),
[6],
[hexToBytes(`0x${'06'.repeat(32)}`)],
)
const stemToUpdate = keyValuePairs[0].hashedKey.slice(0, 31)
const indexToUpdate = 6
const updatedValue = hexToBytes(`0x${'06'.repeat(32)}`)
await tree1.put(stemToUpdate, [indexToUpdate], [updatedValue])
await tree2.put(stemToUpdate, [indexToUpdate], [updatedValue])
assert.deepEqual(tree1.root(), tree2.root())

const postUpdateRoot = tree1.root()
assert.isFalse(equalsBytes(preUpdateRoot, postUpdateRoot), 'The tree root should have updated')

// Ensure that we can retrieve the updated value
const [retrievedValue] = await tree1.get(stemToUpdate, [indexToUpdate])
assert.exists(retrievedValue, 'Updated value should exist')
assert.isTrue(
equalsBytes(retrievedValue!, updatedValue),
'Retrieved value should match the updated value',
)
})
it('should dump leaf values and node hashes', async () => {
const tree1 = await createBinaryTree()
const tree = await createBinaryTree()

// Create an array of 100 random key/value pairs by hashing keys.
const keyValuePairs = []
Expand All @@ -468,10 +477,10 @@ describe('insert', () => {
for (const { hashedKey, value } of keyValuePairs) {
const stem = hashedKey.slice(0, 31)
const index = hashedKey[31]
await tree1.put(stem, [index], [value])
await tree.put(stem, [index], [value])
}

const leafValues = await dumpLeafValues(tree1, tree1.root())
const leafValues = await dumpLeafValues(tree, tree.root())
assert.exists(leafValues)
assert.equal(leafValues!.length, 100)

Expand All @@ -483,10 +492,10 @@ describe('insert', () => {
const actualKeys = leafValues!.map(([key]) => key).sort()
assert.deepEqual(actualKeys, expectedKeys)

const nodeHashes = await dumpNodeHashes(tree1, tree1.root())
const nodeHashes = await dumpNodeHashes(tree, tree.root())
assert.exists(nodeHashes)
expect(nodeHashes!.length).toBeGreaterThan(100)
assert.equal(nodeHashes![0][1], bytesToHex(tree1.root()))
assert.equal(nodeHashes![0][1], bytesToHex(tree.root()))
})

it('should update value when inserting a duplicate key', async () => {
Expand Down