Include suicided accounts in state diff #8297
Conversation
ethcore/src/state/mod.rs
Outdated
| // to work. | ||
| for key in pod_account.storage.keys() { | ||
| self.storage_at(address, key)?; | ||
| match pod.get(address) { |
There was a problem hiding this comment.
if let Some(pod_account) = pod.get(address) ?
| fn query_pod(&mut self, query: &PodState, touched_addresses: &[Address]) -> trie::Result<()> { | ||
| let pod = query.get(); | ||
|
|
||
| for address in touched_addresses { |
There was a problem hiding this comment.
maybe use filter_map instead?
There was a problem hiding this comment.
Can you explain how filter_map would work in this case? Here we need to call ensure_cached for all items in touched_addresses first, so I think it might not save us LOC or code clarity. Do you mean something like this?
let cached_pod_accounts = touched_addresses.filter_map(|address| {
if !self.ensure_cached(address, RequireCache::Code, true, |a| a.is_some())? {
None
} else {
pod.get(address)
}
});
for pod_account in cached_pod_accounts {
for key in pod_account.storage.keys() {
self.storage_at(address, key)?;
}
}
There was a problem hiding this comment.
Oh, I think I get it now. Sorry for the confusion, had to go through the code to understand what's going on.
So if I get that correctly:
- For every
touched_addresswe need to callensure_cachedto populate the pre-state cache. - Additionally for every
touched_addressthat has an account in pre-state we need to query all storage keys (from the post-state).
The current loop is the most clear way to do it then.
That said, can we get a test for it?
There was a problem hiding this comment.
Okay I'll try to add a test for this.
Just a minor clarification, for (2), we query all storage keys that has been cached due to state change, but not all of the available keys. Otherwise it can be huge. :p
There was a problem hiding this comment.
I added should_trace_diff_suicided_accounts that will test touched_addresses and State::diff_from.
debris
added
A8-looksgood 🦄
* Include suicided accounts in state diff * Shorten form match -> if let * Test suicide trace diff in State
* Include suicided accounts in state diff * Shorten form match -> if let * Test suicide trace diff in State
* Warp-only sync with warp-barrier [blocknumber] flag. (#8228) * Warp-only sync with warp-after [blocknumber] flag. * Fix tests. * Fix configuration tests. * Rename to warp barrier. * Allow unsafe js eval on Parity Wallet. (#8204) * Update musicoin spec in line with gmc v2.6.2 (#8242) * Supress TemporaryInvalid verification failures. (#8256) * Include suicided accounts in state diff (#8297) * Include suicided accounts in state diff * Shorten form match -> if let * Test suicide trace diff in State * replace_home for password_files, reserved_peers and log_file (#8324) * replace_home for password_files, reserved_peers and log_file * typo: arg_log_file is Option * Enable UI by default, but only display info page. * Fix test. * Fix naming and remove old todo. * Change "wallet" with "browser UI"
* Update musicoin spec in line with gmc v2.6.2 (#8242) * Supress TemporaryInvalid verification failures. (#8256) * Include suicided accounts in state diff (#8297) * Include suicided accounts in state diff * Shorten form match -> if let * Test suicide trace diff in State * replace_home for password_files, reserved_peers and log_file (#8324) * replace_home for password_files, reserved_peers and log_file * typo: arg_log_file is Option * Bump version in util/version
Fixes #8158
The bug happens because PodState only records addresses that exist. In state diff, it uses the post PodState to populate the cache of pre-state, but missed killed addresses. So we simply collect all touched address directly from cache, and then use that to call
query_pod.