-
Notifications
You must be signed in to change notification settings - Fork 6
Add State snapshotting cheats support #424
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
* caching fix
| call_after_invariant: bool, | ||
| identified_contracts: Option<&ContractsByAddress>, | ||
| ) -> TestResult { | ||
| self.executor.strategy.runner.rollback_transaction(self.executor.strategy.context.as_ref()); |
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 do we need to rollback transaction before run?
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.
just in case if i missed it anywhere, ie table tests or else
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.
but given that setup happens outside a transaction it's changes are not being rolled back
| Self(self.0.clone()) | ||
| } | ||
|
|
||
| pub fn start_snapshotting(&mut self) { |
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 you have not decided to just copy backend like this:
pub fn save_revive_snapshot(snapshot_id: U256) {
TEST_EXTERNALITIES.with_borrow_mut(|test_ext| {
let backend = test_ext.as_backend();
REVIVE_SNAPSHOTS.with_borrow_mut(|snapshots| {
snapshots.insert(snapshot_id, backend);
});
});
}
And revert it like this
pub fn revert_revive_snapshot(snapshot_id: U256) -> bool {
REVIVE_SNAPSHOTS.with_borrow_mut(|snapshots| {
if let Some(backend) = snapshots.get(&snapshot_id).cloned() {
let mut test_externalities = ExtBuilder::default().build();
let mut backend_copy = backend;
std::mem::swap(&mut test_externalities.backend, &mut backend_copy);
TEST_EXTERNALITIES.set(test_externalities);
true
} else {
false
}
})
}
Of course this implementation is based on old code base, but when I have implemented it the changes were very minimal.
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.
transaction commit and rollback are much more faster
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? It is mem swap/copy, Rollbacks you are doing in loops. Do you have any numbers?
I see that as_backend could be slow, is there an option to use into_raw_snapshot
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.
yeah, i'll post the comparative changes today. for fuzz tests for example executor.clone was very much noticeable in the .trace output compared to transaction_start and transaction_rollback
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? It is mem swap/copy, Rollbacks you are doing in loops. Do you have any numbers?
I see that as_backend could be slow, is there an option to use into_raw_snapshot
it was even slower than as_backend(), in the thread local impl but i'll add benches today to an issue and tag you so that it's more visible
ecd69f2 to
70e30d9
Compare
Description
see title.