Piper/remove multi trie support #617
Conversation
pipermerriam
force-pushed
the
piper/remove-multi-trie-support
branch
from
c7e8cc1 to
457ac14
Compare
pipermerriam
force-pushed
the
piper/remove-multi-trie-support
branch
from
f3183af to
d3b6cd4
Compare
There was a problem hiding this comment.
Dropped the support since this is no longer part of the immediate roadmap.
Hm, this feels a bit wrong, since it seems likely that we will want configurable trie classes again later. (So doesn't really belong in the YAGNI category)
But... I don't have a better suggestion, because the cost of keeping the configurable tries around might be higher than the cost of adding them back in later when sharding (or whatever) wants them.
| "trie_class {} is not supported.".format(trie_class) | ||
| ) | ||
| self.trie_class = trie_class | ||
| self.empty_root_hash = empty_root_hash |
There was a problem hiding this comment.
The root hash code feels like unnecessary inspection on the trie class details, so I'm happy to see that go. Maybe we can push the blank root hash responsibility into the trie class. IIRC, the class already keeps track of what a blank hash is anyway.
Anyway, I guess that's only relevant when we bring back configurable trie classes.
There was a problem hiding this comment.
I think this stuff is implemented in the wrong place so....
the cost of keeping the configurable tries around might be higher than the cost of adding them back in later
I think this is accurate.
Judging from the diff it seems easy enough to readd if it's needed at some later point. I'm definitely a fan of removing things when they aren't needed and keeping things as simple as possible. |
link: #599 and #615 and #611
What was wrong?
During the sharding work, support for different types of underlying
trieclasses was introduced as part of the work to support lighter witnesses.How was it fixed?
Dropped the support since this is no longer part of the immediate roadmap.
Cute Animal Picture