-
Notifications
You must be signed in to change notification settings - Fork 217
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 MMR rewinding #327
Add MMR rewinding #327
Conversation
created rewind and forward for MMR wip
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.
LGTM
@@ -49,6 +52,12 @@ pub(crate) struct MerkleCheckPoint { | |||
pub objects_to_del: Vec<ObjectHash>, | |||
mmr_to_add: Vec<MerkleNode>, | |||
} | |||
|
|||
pub(crate) struct CpCleanup { |
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.
pub(crate) struct CpCleanup { | |
pub(crate) struct CheckpointCleanup { |
Writing MMR out would be a screenful, Checkpoint is fine
@@ -190,16 +282,45 @@ impl MerkleChangeTracker { | |||
if !self.enabled { | |||
return Ok(()); | |||
} | |||
hashmap.drain(); |
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.
I don't think this does anything. You still need to call collect()
or take()
on this struct
You probably want 'clear' then
…On Tue, Jun 4, 2019, 10:52 Mike the Tike ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In infrastructure/merklemountainrange/src/merkle_change_tracker.rs
<#327 (comment)>:
> @@ -190,16 +282,45 @@ impl MerkleChangeTracker {
if !self.enabled {
return Ok(());
}
+ hashmap.drain();
I don't think this does anything. You still need to call collect() or
take() on this struct
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#327?email_source=notifications&email_token=ABZZALZFFOIM6QPQC6H73KTPYYUNRA5CNFSM4HQK74EKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB2PSSSQ#pullrequestreview-245311818>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABZZAL7HUFEHAJYYHC3IGBTPYYUNRANCNFSM4HQK74EA>
.
|
Nvm, see it does the same as clear, just creates a new iterator at the same time |
This PR adds the ability to rewind and forward the MMR without changing it permanently
Description
Motivation and Context
Fixes #288
How Has This Been Tested?
Added additional unit tests
Types of changes
Checklist:
development
branchcargo-fmt --all
before pushing