Make HashDB generic #8739

dvdplm · 2018-05-30T13:51:54Z

The aim of this PR is to make patricia-trie usable from Polkadot by making it generic over both the hashing algorithm and the encoding scheme (currently Keccak-256 and RLP).

Part of #7019 and #8620

The HashDB trait – the basis of the various Trie DB types – is made generic over the hashing algorithm and output format by introducing a new Hasher trait and threading it throughout the crates using it.
With the NodeCodec trait we encapsulate the information needed to encode/decode a trie node. The NodeCodec trait is generic over the Hasher as well.

The concrete implementations we need in Parity for Keccak-256 and RLP, are provided in two new mini-crates: keccak-hasher and patricia-trie-ethereum. Here we also provide type aliases for Result and TrieError.

Benchmark compared to master:

 name                        master ns/iter  PR ns/iter        diff ns/iter  diff %  speedup
 trie_insertions_32_mir_1k   2,230,153        2,217,779            -12,374  -0.55%   x 1.01
 trie_insertions_32_ran_1k   2,216,102        2,231,613             15,511   0.70%   x 0.99
 trie_insertions_random_mid  1,556,671        1,578,555             21,884   1.41%   x 0.99
 trie_insertions_six_high    1,604,928        1,643,805             38,877   2.42%   x 0.98
 trie_insertions_six_low     3,049,674        3,090,029             40,355   1.32%   x 0.99
 trie_insertions_six_mid     2,048,429        2,059,450             11,021   0.54%   x 0.99
 trie_iter                   1,581,328        1,549,457            -31,871  -2.02%   x 1.02

Add associated const to Hasher for the fast-path Make HashDB impl for MemoryDB generic Make H256FastMap generic (eew)

Make AsHashDB generic

…urnaldb

Bump version to 0.2.0 **NOTE** Hard coded to use `KeccakHasher`

…eric * origin/master: Fix local transactions policy. (#8691) Shutdown the Snapshot Service early (#8658) network-devp2p: handle UselessPeer disconnect (#8686) Fix compilation error on nightly rust (#8707) Add a test for decoding corrupt data (#8713) Update dev chain (#8717) Remove unused imports (#8722)

dvdplm · 2018-05-30T14:02:29Z

util/hashdb/src/lib.rs


 /// `HashDB` value type.
 pub type DBValue = ElasticArray128<u8>;

 /// Trait modelling datastore keyed by a 32-byte Keccak hash.
-pub trait HashDB: AsHashDB + Send + Sync {
+pub trait HashDB: Send + Sync {


How do I best fit in AsHashDB in this? It takes a Hasher now (see below), but not sure how to make pass the type argument.

dvdplm · 2018-05-30T14:18:45Z

util/patricia_trie/src/triedb.rs

 				},
-				IterStep::Descend(Err(e)) => {
-					return Some(Err(e))
+				IterStep::Descend::<H::Out>(Err(e)) => {


Help needed here.

…he Hasher itself

eira-fransham · 2018-06-28T09:49:49Z

util/patricia_trie/src/nibbleslice.rs

@@ -154,6 +156,7 @@ impl<'a> NibbleSlice<'a> {
 	}

 	/// Encode while nibble slice in prefixed hex notation, noting whether it `is_leaf`.
+	#[inline(always)]


This function is non-trivial and so I'd probably annotate it #[inline] so LLVM can make the choice to inline or not itself. #[inline(always)] is only for really trivial functions that are really hot but you have profiled to see are not being inlined automatically (like len below) and should be used with care. Although if you've benchmarked this and found that the (always) is necessary then ignore this comment, of course!

It did show up in the traces and looked suspicious, but now that I try again it doesn't seem to make any difference at all so dialing back to just #[inline] makes sense. :)

ordian · 2018-06-29T11:45:02Z

ethcore/src/state/account.rs

-		use trie::recorder::Recorder;
-
+	// pub fn prove_storage(&self, db: &HashDB<KeccakHasher>, storage_key: H256) -> Result<(Vec<Bytes>, H256), Box<TrieError<<KeccakHasher as Hasher>::Out>>> {
+	// pub fn prove_storage(&self, db: &HashDB<KeccakHasher>, storage_key: H256) -> Result<(Vec<Bytes>, H256), Box<TrieError>> {


ordian · 2018-06-29T14:57:17Z

ethcore/src/account_db.rs

 		AccountDB {
 			db: db,
 			address_hash: address_hash,
 		}
 	}
 }

-impl<'db> HashDB for AccountDB<'db>{
+impl<'db> AsHashDB<KeccakHasher> for AccountDB<'db> {


I'm not sure whether it is possible to eliminate this boilerplate, due to super-trait upcasting still being an unresolved issue in rust (https://github.com/rust-lang/rust/issues/5665). But maybe we can ease that pain by using a macro?

We can file a refactor issue but I wouldn't call this a blocker for this PR

Added here: #9022

ordian · 2018-06-29T15:05:17Z

ethcore/src/state_db.rs

@@ -309,13 +311,15 @@ impl StateDB {
 		}
 	}

+	// TODO: needed?


Used in ethcore/src/client/client.rs:1688:52.

ordian · 2018-06-29T15:05:42Z

ethcore/src/state_db.rs

 		self.db.as_hashdb()
 	}

+	// TODO: needed?


Used in ethcore/src/client/client.rs:613:62.

ordian · 2018-06-29T15:09:44Z

util/patricia_trie/src/triedbmut.rs

-		let node = Node::from_rlp(&node_rlp, &*self.db, &mut self.storage);
+	fn cache(&mut self, hash: H::Out) -> Result<StorageHandle, H::Out, C::Error> {
+		let node_encoded = self.db.get(&hash).ok_or_else(|| TrieError::IncompleteDatabase(hash))?;
+		// let node_encoded = self.db.get(&hash).ok_or_else(|| Box::new(TrieError::IncompleteDatabase(hash)))?;


No this one's actually wrong, good catch.

* master: Minimal effective gas price in the queue (#8934) parity: fix db path when migrating to blooms db (#8975) Preserve the current abort behavior (#8995) Improve should_replace on NonceAndGasPrice (#8980) Tentative fix for missing dependency error (#8973)

rphmeier · 2018-07-02T09:09:37Z

LGTM after a merge and some extra whitespace cleanup.

ordian

LGTM modulo unresolved conflicts

5chdn · 2018-07-02T12:38:40Z

@dvdplm please resolve conflicts 💪

* master: Attempt to graceful shutdown in case of panics (#8999) simplify kvdb error types (#8924) Add option for user to set max size limit for RPC requests (#9010) bump ntp to 0.5.0 (#9009) Removed duplicate dependency (#9021)

* master: Only return error log for rustls (#9025) Update Changelogs for 1.10.8 and 1.11.5 (#9012)

…rp_sync_on_light_client * 'master' of https://github.com/paritytech/parity: Never drop local transactions from different senders. (openethereum#9002) Precise HTTP or WebSockets for JSON-RPC options (openethereum#9027) Recently rejected cache for transaction queue (openethereum#9005) Make HashDB generic (openethereum#8739) Only return error log for rustls (openethereum#9025) Update Changelogs for 1.10.8 and 1.11.5 (openethereum#9012)

dvdplm added 23 commits May 26, 2018 22:23

WIP Make HashDB generic

e0c064a

Add HeapSizeOf bound to the output type of a Hasher

1cd7ded

Add associated const to Hasher for the fast-path Make HashDB impl for MemoryDB generic Make H256FastMap generic (eew)

Add comments and todos

4bf7f6d

Cleanup and more todos

fd666d6

Move concrete hasher impl to hashdb – TODO: where do these go?

5325a70

Make AsHashDB generic

Remove unused crates/uses

041d6a2

Version bump – breaking change => 0.2.0

f716105

Version bump

8ce39eb

Derive Clone and PartialEq for KeccakHasher to satisfy the code in jo…

a7ff7c2

…urnaldb

Thread Hasher throughout the crate – WIP

ef1e48a

Use v0.2 of hashdb and memorydb

3a7c490

Make journaldb use generic hashdb/memorydb

83342c0

Bump version to 0.2.0 **NOTE** Hard coded to use `KeccakHasher`

Version bump: use newer hasdb

aa268bd

Additional bounds on Hasher output

bf01073

TrieDB, TrieDBMut, SecTrieDB, FatDB typechecks

013666f

Fix tests

1f892fe

FatDBMut and test fixes

f98e905

FatDBMut test

b42f92e

SecTrieDB tests

5e9fda0

Tests for TrieDBMut

841abc4

Recorder tests

f29132c

SecTrieDBMut + test

201cdcb

dvdplm self-assigned this May 30, 2018

dvdplm added the A0-pleasereview 🤓 Pull request needs code review. label May 30, 2018

Remove scratch pad files

6bf7e5b

dvdplm commented May 30, 2018

View reviewed changes

dvdplm added 2 commits May 30, 2018 16:02

Remove more traces of scratch pad files

fc058e6

Add a test for using MemoryDB with a different hasher (TODO)

03ff20e

dvdplm commented May 30, 2018

View reviewed changes

dvdplm added 3 commits June 27, 2018 15:52

Fix tests

1cf7c28

Rename NodeCodec::E to Error

44bb8b6

Make Recorder and Record generic over the Hasher output rather than t…

29bf049

…he Hasher itself

eira-fransham reviewed Jun 28, 2018

View reviewed changes

dvdplm added 8 commits June 28, 2018 12:34

Fix trat descr

7be2120

Remove dead code

211882e

Ditch ethcore-logger for env-logger

609ebde

Parametrize the HashMap builder from the Hasher

308f281

Less aggressive inlining

1aaaa63

Add convenience types and use them for less line noise

b76fffa

Remove unused deps

92582c7

Clean up tests

988bfaf

ordian reviewed Jun 29, 2018

View reviewed changes

dvdplm added 4 commits July 2, 2018 09:25

Address grumbles

d625d8e

Remove tempdir as test dep

67003be

Boxed errors

412ae9c

rphmeier approved these changes Jul 2, 2018

View reviewed changes

dvdplm mentioned this pull request Jul 2, 2018

Refactor: alleviate painful AsHashDB impls with macro (or magic) #9022

Closed

Remove spurious whitespace after use blocks

07dfdc7

ordian approved these changes Jul 2, 2018

View reviewed changes

5chdn added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 2, 2018

dvdplm added 2 commits July 2, 2018 14:52

Merge branch 'master' into refactor/hashdb-generic

afbf3a9

* master: Attempt to graceful shutdown in case of panics (#8999) simplify kvdb error types (#8924) Add option for user to set max size limit for RPC requests (#9010) bump ntp to 0.5.0 (#9009) Removed duplicate dependency (#9021)

Merge branch 'master' into refactor/hashdb-generic

b9e310b

* master: Only return error log for rustls (#9025) Update Changelogs for 1.10.8 and 1.11.5 (#9012)

dvdplm merged commit 9caa868 into master Jul 2, 2018

debris deleted the refactor/hashdb-generic branch July 2, 2018 17:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make HashDB generic #8739

Make HashDB generic #8739

dvdplm commented May 30, 2018 •

edited

Loading

dvdplm May 30, 2018

dvdplm May 30, 2018

eira-fransham Jun 28, 2018

dvdplm Jun 28, 2018

ordian Jun 29, 2018

ordian Jun 29, 2018

rphmeier Jul 2, 2018

dvdplm Jul 2, 2018

ordian Jun 29, 2018

ordian Jun 29, 2018

ordian Jun 29, 2018

dvdplm Jul 2, 2018

rphmeier commented Jul 2, 2018

ordian left a comment

5chdn commented Jul 2, 2018

Make HashDB generic #8739

Make HashDB generic #8739

Conversation

dvdplm commented May 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rphmeier commented Jul 2, 2018

ordian left a comment

Choose a reason for hiding this comment

5chdn commented Jul 2, 2018

dvdplm commented May 30, 2018 •

edited

Loading