Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Decouple MemoryDB from RLP #8950

Closed
dvdplm opened this issue Jun 22, 2018 · 4 comments
Closed

Decouple MemoryDB from RLP #8950

dvdplm opened this issue Jun 22, 2018 · 4 comments
Labels
F6-refactor 📚 Code needs refactoring. M4-core ⛓ Core client code / Rust. P9-somedaymaybe 🌞 Issue might be worth doing eventually. Q7-involved 💪 Can be fixed by a team of developers and probably takes some time.
Milestone

Comments

@dvdplm
Copy link
Collaborator

dvdplm commented Jun 22, 2018

Currently the memorydb crate has a hard dependency on rlp.

Make it generic over the encoding scheme using the NodeCodec trait from #8739.

@dvdplm dvdplm added F6-refactor 📚 Code needs refactoring. P9-somedaymaybe 🌞 Issue might be worth doing eventually. M4-core ⛓ Core client code / Rust. labels Jun 22, 2018
@5chdn 5chdn added this to the 1.14 & ... milestone Jun 24, 2018
@debris
Copy link
Collaborator

debris commented Jul 4, 2018

this is not trivial, because of how we treat null_node and null_node_rlp. it is exactly the same issue that prevents us from decoupling NodeCodec from rlp

#9043 (comment)

@dvdplm
Copy link
Collaborator Author

dvdplm commented Jul 5, 2018

Good point.

@5chdn 5chdn modified the milestones: 2.2, 2.3 & ... Jul 17, 2018
@cheme
Copy link
Contributor

cheme commented Aug 9, 2018

Something along the lines of (using associated const)

#[derive(Clone, PartialEq)]
pub struct MemoryDB<M: MemoryDBConf> {
	data: FastMap<M::H, (DBValue, i32)>,
	hashed_null_node: <M::H as KeyHasher>::Out,
}

pub trait MemoryDBConf {
  const NULL_VALUE: &'static [u8];
  type H: KeyHasher;
}

could do it, but I feel that the added complexity (new trait with impact everywher) may not be worth it.

@dvdplm dvdplm added the Q7-involved 💪 Can be fixed by a team of developers and probably takes some time. label Aug 9, 2018
@5chdn 5chdn modified the milestones: 2.3, 2.4 & ... Sep 11, 2018
@5chdn 5chdn modified the milestones: 2.4, 2.5 & ... Oct 29, 2018
@5chdn 5chdn modified the milestones: 2.5, 2.6 & ... Jan 10, 2019
@5chdn 5chdn modified the milestones: 2.6, 2.7 & ... Feb 21, 2019
@cheme
Copy link
Contributor

cheme commented Feb 25, 2019

This issue is fixed now (memorydb contains a null node hashed instance and there is no more deps to rlp). Note that memory db is no longer in parity-ethereum but shared with substrate.

@cheme cheme closed this as completed Feb 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F6-refactor 📚 Code needs refactoring. M4-core ⛓ Core client code / Rust. P9-somedaymaybe 🌞 Issue might be worth doing eventually. Q7-involved 💪 Can be fixed by a team of developers and probably takes some time.
Projects
None yet
Development

No branches or pull requests

5 participants
@dvdplm @cheme @debris @5chdn and others