Update ref to parity-common and update seek behaviour
#9257
Conversation
ngotchac
added
A0-pleasereview 🤓
|
@dvdplm That's right! |
| @@ -1692,6 +1692,9 @@ impl BlockChainClient for Client { | |||
| if let Some(after) = after { | |||
| if let Err(e) = iter.seek(after) { | |||
| trace!(target: "fatdb", "list_accounts: Couldn't seek the DB: {:?}", e); | |||
| } else { | |||
| // Position the iterator after the `after` element | |||
| iter.next(); | |||
There was a problem hiding this comment.
Could you explain how this works? Why calling next after seek returned an Err would position the iterator after the after?
There was a problem hiding this comment.
So the else branch is actually when there has been no errors
There was a problem hiding this comment.
Ah, right, thanks for clarifying that. So we assume that after is present in the db, otherwise this is not correct?
There was a problem hiding this comment.
So in this context, after is either None or an address that is actually in DB.
There was a problem hiding this comment.
I think this could be simplified by writing:
if let Err(e) = iter.next(after) {
trace!("...");
}Cause Err is no used later on anyway
ordian
added
A8-looksgood 🦄
Cargo.toml
Outdated
| @@ -33,7 +33,7 @@ fdlimit = "0.1" | |||
| ctrlc = { git = "https://github.com/paritytech/rust-ctrlc.git" } | |||
| jsonrpc-core = { git = "https://github.com/paritytech/jsonrpc.git", branch = "parity-1.11" } | |||
| ethcore = { path = "ethcore", features = ["parity"] } | |||
| parity-bytes = { git = "https://github.com/paritytech/parity-common" } | |||
| parity-bytes = { git = "https://github.com/paritytech/parity-common", branch = "ng-fix-triedb-seek" } | |||
There was a problem hiding this comment.
Just an idea. To get "atomic" PRs, we can make parity-common repo a submodule of this repo. A submodule is pinned to a commit, so we can avoid always manually changing branches. (And it also makes browsing all ethereum-related code easier!)
There was a problem hiding this comment.
What if we just specify a commit hash for parity-common? Should be easier to implement, and should be safe.
| @@ -1692,6 +1692,9 @@ impl BlockChainClient for Client { | |||
| if let Some(after) = after { | |||
| if let Err(e) = iter.seek(after) { | |||
| trace!(target: "fatdb", "list_accounts: Couldn't seek the DB: {:?}", e); | |||
| } else { | |||
| // Position the iterator after the `after` element | |||
| iter.next(); | |||
There was a problem hiding this comment.
I think this could be simplified by writing:
if let Err(e) = iter.next(after) {
trace!("...");
}Cause Err is no used later on anyway
debris
added
A7-looksgoodcantmerge 🙄
|
Hm, the error is used in the trace log, and |
|
ah, you are totally right! I confused seek with peek 🤦♂️ |
debris
added
A8-looksgood 🦄
…rp_sync_on_light_client * 'master' of https://github.com/paritytech/parity: Propagate transactions for next 4 blocks. (openethereum#9265) decode block rlp less often (openethereum#9252) Fix eternalities tests can_create (missing parameter) (openethereum#9270) Update ref to `parity-common` and update `seek` behaviour (openethereum#9257) Comply EIP-86 with the new definition (openethereum#9140) Check if synced when using eth_getWork (openethereum#9193) (openethereum#9210)
This PR: paritytech/parity-common#13 updates the behavior of
seekin TrieDBIterators.This PR updates the reference to
parity-commonand updates the behavior ofseekwhich places the iterator on the given element (if existing) or the closest next one.