Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(trie): Implement parity trie crate #3390

Closed
wants to merge 47 commits into from

Conversation

dimartiro
Copy link
Contributor

@dimartiro dimartiro commented Jul 17, 2023

Changes

Implements parity trie crate used in substrate

This is based on substrate and trie-db crate

Goals

  • Get trie values using new lookup methods
  • Lazy loading trie from DB (Get nodes from DB and decode it only if necessary)

New structures

Changes

  • MemoryDB
  • Partial decoding (decode only required nodes and not the full trie)

Tests

go test ./lib/trie/... ./internal/trie/...

Issues

This is part but not complete solution for issue #2418

Primary Reviewer

@timwu20

@dimartiro dimartiro changed the title refa(trie): Implement subtrate like TrieDB refactor(trie): Implement subtrate like TrieDB Jul 17, 2023
fix(trie): Add missing copyrights

fix(trie): Hash conversion

fix(trie): Simplify memorydb
@dimartiro dimartiro marked this pull request as ready for review August 1, 2023 14:59
@dimartiro dimartiro changed the title refactor(trie): Implement subtrate like TrieDB refactor(trie): trie lazy loading support Aug 2, 2023
@dimartiro dimartiro changed the title refactor(trie): trie lazy loading support feat(trie): Implement parity trie crate Aug 4, 2023
@@ -40,3 +40,7 @@ func (n *Node) HasChild() (has bool) {
}
return false
}

func (n *Node) ChildAt(i uint) *Node {
return n.Children[i]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we do a length check here?

data map[common.Hash][]byte
}

func NewMemoryDBFromProof(encodedNodes [][]byte) (*MemoryDB, error) {
func NewMemoryDBFromProof(encodedNodes [][]byte) (*InMemoryDB, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func NewMemoryDBFromProof(encodedNodes [][]byte) (*InMemoryDB, error) {
func NewInMemoryDBFromProof(encodedNodes [][]byte) (*InMemoryDB, error) {


type HashDB interface {
Get(key []byte) (value []byte, err error)
Insert(value []byte) common.Hash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try to avoid using anything from common. Have you considered translating what they do in the rust trie lib using go generics?

/// Trait describing an object that can hash a slice of bytes. Used to abstract
/// other types over the hashing algorithm. Defines a single `hash` method and an
/// `Out` associated type with the necessary bounds.
pub trait Hasher: Sync + Send {
	/// The output type of the `Hasher`
	type Out: AsRef<[u8]>
		+ AsMut<[u8]>
		+ Default
		+ MaybeDebug
		+ core::cmp::Ord
		+ PartialEq
		+ Eq
		+ hash::Hash
		+ Send
		+ Sync
		+ Clone
		+ Copy;
	/// What to use to build `HashMap`s with this `Hasher`.
	type StdHasher: Sync + Send + Default + hash::Hasher;
	/// The length in bytes of the `Hasher` output.
	const LENGTH: usize;

	/// Compute the hash of the provided slice of bytes returning the `Out` type of the `Hasher`.
	fn hash(x: &[u8]) -> Self::Out;
}

type MemoryDBItem struct {
data []byte
// Reference count
rc int32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need reference counting?

Comment on lines +74 to +76
if data.rc <= 0 {
data.data = value
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the data is never updated if there are multiple callers to MemoryDB.Insert()?

Comment on lines +16 to +20
const (
Empty NodeType = iota
Leaf
NibbledBranch
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be it better to actually create the different types for all these nodes, rather than create one Node type that will store them all?

/// Type of node in the trie and essential information thereof.
#[derive(Eq, PartialEq, Clone)]
#[cfg_attr(feature = "std", derive(Debug))]
pub enum Node<'a> {
	/// Null trie node; could be an empty root or an empty branch entry.
	Empty,
	/// Leaf node; has key slice and value. Value may not be empty.
	Leaf(NibbleSlice<'a>, Value<'a>),
	/// Extension node; has key slice and node data. Data may not be null.
	Extension(NibbleSlice<'a>, NodeHandle<'a>),
	/// Branch node; has slice of child nodes (each possibly null)
	/// and an optional immediate node data.
	Branch([Option<NodeHandle<'a>>; nibble_ops::NIBBLE_LENGTH], Option<Value<'a>>),
	/// Branch node with support for a nibble (when extension nodes are not used).
	NibbledBranch(
		NibbleSlice<'a>,
		[Option<NodeHandle<'a>>; nibble_ops::NIBBLE_LENGTH],
		Option<Value<'a>>,
	),
}

@timwu20
Copy link
Contributor

timwu20 commented Aug 14, 2023

can you mark as draft if you're not gonna work on this @dimartiro?

@dimartiro dimartiro marked this pull request as draft August 14, 2023 19:39
@dimartiro dimartiro closed this Sep 25, 2023
@dimartiro dimartiro mentioned this pull request Dec 12, 2023
4 tasks
@dimartiro dimartiro self-assigned this Dec 12, 2023
@dimartiro dimartiro reopened this Dec 12, 2023
@dimartiro dimartiro closed this Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants