-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Reorg resistance #2317
Reorg resistance #2317
Conversation
&mut value_cache, | ||
)?; | ||
if self.index.client.get_block_count()? - self.index.block_height()?.unwrap_or(Height(0)).n() | ||
< 50 |
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.
This should probably be the maximum number of savepoints (i.e. 6). Anything higher is unnecessary unless you want to have extra reorg resistance during the initial indexing 😋
@raphjaph I implemented a very basic reorg protection with savepoints, so hope you don't mind if I keep an eye on this one and add some review comments 😋 |
|
||
let savepoints: Vec<u64> = wtx.list_persistent_savepoints()?.into_iter().collect(); | ||
dbg!(&savepoints); | ||
// self.savepoints.push_back(wtx.ephemeral_savepoint()?); |
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 think you want to create a persistent save_point here. Also, you'll need to commit wtx and reinitialise it directly after.
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.
Since you have to commit the txn and reinitialise it, it might make more sense for this logic to live directly after a self.commit(wtx)
call or maybe even inside the commit function.
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.
In fact, I think wtx has to be clean (no unflushed writes) before creating the savepoint, and you have to commit straight after it as it would make the wtx dirty.
if let Some(prev_height) = self.height.checked_sub(1) { | ||
let prev_hash = self.index.client.get_block_hash(prev_height)?; | ||
dbg!(&prev_hash); | ||
if prev_hash != block.header.prev_blockhash { | ||
log::info!("reorg detected at or before {prev_height}; rolling back index"); | ||
self.index.reorged.store(true, atomic::Ordering::Relaxed); | ||
|
||
let savepoint = wtx.get_persistent_savepoint(self.savepoints.pop_front().unwrap())?; | ||
wtx.restore_savepoint(&savepoint)?; | ||
self.savepoints.clear(); | ||
} | ||
} |
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.
This logic should probably live before the create savepoint logic, otherwise we create a savepoint and immediately rollback to an older one.
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.
your right, I put it into the Index now
Thanks for the review! This was just me playing around I'm gonna make a new branch and PR with the correct one. Have a look at #2320 |
No description provided.