Conversation
debris
added
A0-pleasereview 🤓
| // but due to the current limitations of Rust const evaluation we can't | ||
| // do `const HASHED_NULL_NODE: H::Out = H::Out( … … )`. Perhaps one day soon? | ||
| impl NodeCodec<KeccakHasher> for RlpNodeCodec<KeccakHasher> { | ||
| impl NodeCodec<H256> for RlpNodeCodec<H256> { |
There was a problem hiding this comment.
RlpNodeCodec can is no longer tied to KeccakHasher
There was a problem hiding this comment.
this is a problem, because now the HASHED_NULL_NODE will be defined but wrong for all non-keccak hashers.
dvdplm
left a comment
dvdplm
left a comment
There was a problem hiding this comment.
Look at that, much better! :)
(Whitespace grumbles)
| where C: NodeCodec<H> | ||
| fn from_encoded<C, H>(data: &[u8], db: &HashDB<H>, storage: &mut NodeStorage<H::Out>) -> Self | ||
| where C: NodeCodec<H::Out>, | ||
| H: Hasher<Out = I>, |
| where C: NodeCodec<H> | ||
| fn inline_or_hash<C, H>(node: &[u8], db: &HashDB<H>, storage: &mut NodeStorage<H::Out>) -> NodeHandle<H::Out> | ||
| where C: NodeCodec<H::Out>, | ||
| H: Hasher<Out = I>, |
dvdplm
added
A8-looksgood 🦄
|
@rphmeier ptal, this helps with the |
| C: NodeCodec<H> + 'db | ||
| where | ||
| H: Hasher + 'db, | ||
| C: NodeCodec<H::Out> + 'db |
There was a problem hiding this comment.
in next pr, it would be good to also remove these constrains from struct declaration, cause they are redundant
rphmeier
added
A4-gotissues 💥
|
just in case it gets hidden: the |
|
I reverted |
debris
added
A0-pleasereview 🤓
dvdplm
left a comment
dvdplm
left a comment
There was a problem hiding this comment.
In the new form I guess it's not superclear to me that this is an improvement, but I'm curious to learn why we should do this! :)
|
|
||
| // post-insert action. Same as action without delete | ||
| enum InsertAction<H: Hasher> { | ||
| enum InsertAction<H> { |
There was a problem hiding this comment.
For my education, why is it better to have the type param be unbounded like this? Knowing that H is a Hasher::Out is – to my eyes at least – helpful to understand what is going on, so curious about the reasoning here.
There was a problem hiding this comment.
For my education, why is it better to have the type param be unbounded like this?
It's more simple. It's easier to write unit tests and integrate it with other parts of the code. We don't need bounded types at all. std does not use them. Only the impl are bounded
There was a problem hiding this comment.
Ok, makes sense. I think rob pointed me to https://rust-lang-nursery.github.io/api-guidelines/future-proofing.html#c-struct-bounds at one time.
| } | ||
|
|
||
| impl<H: Hasher> Node<H> { | ||
| impl<I> Node<I> where I: AsRef<[u8]> + AsMut<[u8]> + Default + HeapSizeOf + Debug + PartialEq + Eq + Hash + Send + Sync + Clone + Copy { |
There was a problem hiding this comment.
Does I stand for "item" or what was the thinking here? Maybe O for "Out" would help the reader get the right association?
There was a problem hiding this comment.
it is just an arbitrary uppercase character, I can change it to O if it makes it more readable :)
debris
added
A8-looksgood 🦄
…rp_sync_on_light_client * 'master' of https://github.com/paritytech/parity: Fixes for misbehavior reporting in AuthorityRound (openethereum#8998) A last bunch of txqueue performance optimizations (openethereum#9024) reduce number of constraints for triedb types (openethereum#9043) bump fs-swap to 0.2.3 so it is compatible with osx 10.11 again (openethereum#9050) Recursive test (openethereum#9042) Introduce more optional features in ethcore (openethereum#9020) Update ETSC bootnodes (openethereum#9038) Optimize pending transactions filter (openethereum#9026) eip160/eip161 spec: u64 -> BlockNumber (openethereum#9044)
4 participants
No description provided.