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

trie: rename trie class to MerklePatriciaTrie #3717

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gabrocheleau
Copy link
Contributor

This PR renames the Trie class to MerklePatriciaTrie. This aligns with the modularization of state management (i.e. MerkleStateManager & VerkleStateManager) and data structure agnosticism.

I wasn't sure whether to name it "Tree" or "Trie" and decided to go with what was on the ethereum.org docs: https://ethereum.org/en/developers/docs/data-structures-and-encoding/patricia-merkle-trie/

@holgerd77
Copy link
Member

So, to make the picture complete: the old pre-monorepo package was called merkle-patricia-tree, so now we have everything in to confuse people to the max. 😂

Anyhow: I just read up again a bit on Trie vs Tree and came to the conclusion that trie is indeed the more correct term here. So that would be my tendency as well.

I thought about package renaming as well couple of weeks ago, and I think we should consequently do. If aligned with the class name that would be:

  • MerklePatriciaTrie
  • @ethereumjs/merklepatriciatrie

One thing where I am not totally out of sympathy is to at least give it some quick thought if MPT (and @ethereumjs/mpt might also be enough in this case and shorter and easier to write down, this is such a common naming in the Ethereum world that this might be enough as well.

That would be:

  • MPT
  • @ethereumjs/mpt

And last but not least. Maybe it's also ok to not fully align class + package name. I personally e.g. do not really like the long package name from above. So one option might also be:

  • MerklePatriciaTree
  • @ethereumjs/mpt

(I think this might be my favorite, but have not very strong preferences on all)

@gabrocheleau
Copy link
Contributor Author

So, to make the picture complete: the old pre-monorepo package was called merkle-patricia-tree, so now we have everything in to confuse people to the max. 😂

Anyhow: I just read up again a bit on Trie vs Tree and came to the conclusion that trie is indeed the more correct term here. So that would be my tendency as well.

I thought about package renaming as well couple of weeks ago, and I think we should consequently do. If aligned with the class name that would be:

  • MerklePatriciaTrie
  • @ethereumjs/merklepatriciatrie

One thing where I am not totally out of sympathy is to at least give it some quick thought if MPT (and @ethereumjs/mpt might also be enough in this case and shorter and easier to write down, this is such a common naming in the Ethereum world that this might be enough as well.

That would be:

  • MPT
  • @ethereumjs/mpt

And last but not least. Maybe it's also ok to not fully align class + package name. I personally e.g. do not really like the long package name from above. So one option might also be:

  • MerklePatriciaTree
  • @ethereumjs/mpt

(I think this might be my favorite, but have not very strong preferences on all)

I think my favorite is also the latter (explicit MerklePatriciaTree for the class, mpt for the package). Generally not a big fan of acronyms as I find they increase barrier to entry, but in the context of a package name I think it's fine & more concise.

@gabrocheleau gabrocheleau changed the title trie: rename trie to mpt trie: rename trie class to MerklePatriciaTrie Oct 2, 2024
@gabrocheleau
Copy link
Contributor Author

So, to make the picture complete: the old pre-monorepo package was called merkle-patricia-tree, so now we have everything in to confuse people to the max. 😂
Anyhow: I just read up again a bit on Trie vs Tree and came to the conclusion that trie is indeed the more correct term here. So that would be my tendency as well.
I thought about package renaming as well couple of weeks ago, and I think we should consequently do. If aligned with the class name that would be:

  • MerklePatriciaTrie
  • @ethereumjs/merklepatriciatrie

One thing where I am not totally out of sympathy is to at least give it some quick thought if MPT (and @ethereumjs/mpt might also be enough in this case and shorter and easier to write down, this is such a common naming in the Ethereum world that this might be enough as well.
That would be:

  • MPT
  • @ethereumjs/mpt

And last but not least. Maybe it's also ok to not fully align class + package name. I personally e.g. do not really like the long package name from above. So one option might also be:

  • MerklePatriciaTree
  • @ethereumjs/mpt

(I think this might be my favorite, but have not very strong preferences on all)

I think my favorite is also the latter (explicit MerklePatriciaTree for the class, mpt for the package). Generally not a big fan of acronyms as I find they increase barrier to entry, but in the context of a package name I think it's fine & more concise.

Side note. If we decide to do a package renaming, I would do it in a separate PR, as I feel such a major task deserves is own dedicated PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants