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

add DeleteAtStem command for state rollbacks #446

Merged
merged 3 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import "errors"
var (
errInsertIntoHash = errors.New("trying to insert into hashed node")
errDeleteHash = errors.New("trying to delete from a hashed subtree")
errDeleteMissing = errors.New("trying to delete a missing group")
errDeleteUnknown = errors.New("trying to delete an out-of-view node")
errReadFromInvalid = errors.New("trying to read from an invalid child")
errSerializeHashedNode = errors.New("trying to serialize a hashed internal node")
errInsertIntoOtherStem = errors.New("insert splits a stem where it should not happen")
Expand Down
69 changes: 69 additions & 0 deletions tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,75 @@ func (n *InternalNode) Delete(key []byte, resolver NodeResolverFn) (bool, error)
}
}

func (n *InternalNode) DeleteAtStem(key []byte, resolver NodeResolverFn) (bool, error) {
nChild := offset2key(key, n.depth)
switch child := n.children[nChild].(type) {
case Empty:
return false, nil
case HashedNode:
if resolver == nil {
return false, errDeleteHash
}
payload, err := resolver(key[:n.depth+1])
if err != nil {
return false, err
}
// deserialize the payload and set it as the child
c, err := ParseNode(payload, n.depth+1)
if err != nil {
return false, err
}
n.children[nChild] = c
return n.DeleteAtStem(key, resolver)
case *LeafNode:
if !bytes.Equal(child.stem, key[:31]) {
return false, errDeleteMissing
}
gballet marked this conversation as resolved.
Show resolved Hide resolved

n.cowChild(nChild)
n.children[nChild] = Empty{}

// Check if all children are gone, if so
// signal that this node should be deleted
// as well.
for _, c := range n.children {
if _, ok := c.(Empty); !ok {
return false, nil
}
}

return true, nil
case *InternalNode:
n.cowChild(nChild)
del, err := child.DeleteAtStem(key, resolver)
if err != nil {
return false, err
}

// delete the entire child if instructed to by
// the recursive algorigthm.
if del {
n.children[nChild] = Empty{}

// Check if all children are gone, if so
// signal that this node should be deleted
// as well.
for _, c := range n.children {
if _, ok := c.(Empty); !ok {
return false, nil
}
}

return true, nil
}
Comment on lines +682 to +697
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is a copy/paste from the Delete(...) API, but I guess we could try extracting the logic to a private method?

And BTW, there's a typo (that already existed before). L680, "algorigthm"

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to do that right off the bat, but it's making the interface more complicated. I'd like to do that too, but I want to ensure correctness.

One of the differences, as you pointed out, is what happens when you delete something that doesn't exist. Since this is meant to be called from something that knows of reverse-diffs, so if the value does not exist, there is an issue somewhere.


return false, nil
default:
// only unknown nodes are left
return false, errDeleteUnknown
}
}

// Flush hashes the children of an internal node and replaces them
// with HashedNode. It also sends the current node on the flush channel.
func (n *InternalNode) Flush(flush NodeFlushFn) {
Expand Down
Loading