Automatic removal of orphan nodes

[tzdybal]

Work in progress.

TODO:

• AutoOrphanRemoval option (false by default)
• configuration mechanism (With(opt Option)?) (variadic arguments in constructor)
• simple test
• implement the actual removing of orphans
• more extensive testing (update/delete operations, tree with multiple entries)

[musalbas]

Closing, as superseded by #36 and #37.

[liamsi]

Now that #37 was merged in here. Can we actually resolve the merge conflicts, mark this as ready for review, give it a 2nd pass and finally merge it?
@tzdybal @adlerjohn @musalbas

[adlerjohn]

This should be fully superseded by #37 unless I'm mistaken, so it can be closed @liamsi

[liamsi]

This should be fully superseded by #37 unless I'm mistaken, so it can be closed @liamsi

No, #37 builds on the changes here. Hence the changes of #37 were merged into this branch / PR in 763105b

Completes work from #24.
Revises work from #36.

ref: #24 (comment)
ref: #36 (comment)

[liamsi]

Resolved the merge conflicts. Happy to help push this over the finish line if necessary!

[adlerjohn]

The only difference between the head from #37 and this PR is in smt.go unless I'm mistaken.

diff celestiaorg/smt/smt.go roysc/smt/smt.go

The diff:

305,308d304
< 	// The offset from the bottom of the tree to the start of the side nodes.
< 	// Note: i-offsetOfSideNodes is the index into sideNodes[]
< 	offsetOfSideNodes := smt.depth() - len(sideNodes)
< 
312c308,312
< 		if i-offsetOfSideNodes < 0 || sideNodes[i-offsetOfSideNodes] == nil {
---
> 		// The offset from the bottom of the tree to the start of the side nodes
> 		// i-offsetOfSideNodes is the index into sideNodes[]
> 		offsetOfSideNodes := smt.depth() - len(sideNodes)
> 
> 		if i-offsetOfSideNodes < 0 {

Moving the offset calculation is from #35, so nothing to see there. The only thing of interest is that #37 removes the condition sideNodes[i-offsetOfSideNodes] == nil. That condition should never be hit, since none of the sideNodes will be nil, so shouldn't be a problem to remove. All tests pass in #37, so looks like it's indeed not needed.

[liamsi]

Thanks @adlerjohn 👍🏼