-
Notifications
You must be signed in to change notification settings - Fork 16
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
feature/delete_temp_siks_when_election_ends #1080
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review done!
3e2c992
to
d1b02d8
Compare
…rged by the IST after results calculation
…ous API, splitting census proofs generation from siks proof generations. SIK related purges on election with tempSIKs moved to the top of the functions, trying to solve IST and State issue
For the temporary SIK elections, we need Arbo to implement a Delete() mechanism. The current Delete method removes the key, leaf and also all the intermediary nodes that are not needed. Signed-off-by: p4u <[email protected]>
Fix the upAfterDelete function and unify it with up(). So now also Update and Add methods are removing empty nodes. Add a test to check the disk size after some operations. Signed-off-by: p4u <[email protected]>
Signed-off-by: p4u <[email protected]>
Having a second implementation of the db interface is useful for testing purposes. We can easily switch from pebble to leveldb in order to test something in a different storage layer. In addition add a Compact() method to the db interface.
Remove all intermediate nodes from the database once they are not necessary anymore. Break up the code into smaller Go files.
When a leaf of the tree is removed, if the neighbour child is not empty, it should be moved to the upper level where there are no other leaves. Add tests to verify the operation.
Since we do not hold old intermediate nodes anymore, SetRoot(), Snapshot(), Dump() and so on require to have an existing current intermediate node (or nil to use the current root). To this end we introduce a new method to facilitate retrieve the list of existing roots from a level.
The main issue was that on update, if the key and value are the same, the clean up routine for removing old intermediate nodes, was removing the current valid intermediate nodes up to the root. In addition adapt some tests to become compatible with the new impl. And remove some that are not longer valid.
ecf9ef8
to
960d9f3
Compare
Finally... everything works as expected :) @lucasmenendez please make a new review |
a7560bd
to
503240b
Compare
should not be squashed, the commits are correctly ordered and with correct comments |
t.emptyHash = make([]byte, t.hashFunction.Len()) // empty | ||
var err error | ||
t.emptyNode, _, err = t.newIntermediate(t.emptyHash, t.emptyHash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the empty node is required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty node is used to compare in the up() function if both childs are empty. Precomputing and comparing is faster.
Ensure we produce the same root using both methods. Also update the dev genesis file.
503240b
to
753fe1f
Compare
Is this ready @lucasmenendez? lets merge it! |
I just created an issue with a couple of optimizations that can be done in the future: #1086 |
Now the elections have a flag during creation (
tempSIKs
:boolean) which if set to true, the temporary SIKs (created usingRegisterSIKTx
) must be purged when the election ends.