chore(trie): merge leaf and branch in single node struct

[qdm12]

Changes

ℹ️ the aim of this PR was to simplify and streamline encoding and decoding of nodes before moving to implement additional encoding/decoding for the v1 trie.

🆘 this is a substantial amount of changes, but most of it is just regex replacements in unit tests. All tests pass with the existing values, so it is quite safe 😉

• Removed type assertions and Node interface
• Leaf and branch structs merged in one common Node struct
• Removed trivial methods that were used to avoid type switches (i.e. GetHash())
• Update all tests
• Change Children field from an array to a slice so it can be nil for leaves
• Deduct node type from children slice (not nil == leaf)

Tests

go test -race ./internal/trie/... ./lib/trie/...

Issues

#2187

Primary Reviewer

• @kishansagathiya

[codecov]

Codecov Report

Merging #2504 (a509c65) into development (f2cdfea) will decrease coverage by 0.29%.
The diff coverage is 92.00%.

@@               Coverage Diff               @@
##           development    #2504      +/-   ##
===============================================
- Coverage        57.60%   57.30%   -0.30%     
===============================================
  Files              218      215       -3     
  Lines            28662    28417     -245     
===============================================
- Hits             16511    16285     -226     
+ Misses           10464    10455       -9     
+ Partials          1687     1677      -10     

Flag	Coverage Δ
unit-tests	57.30% <92.00%> (-0.30%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
internal/trie/node/key.go	100.00% <ø> (ø)
lib/trie/database.go	50.30% <73.91%> (-0.58%)	⬇️
internal/trie/node/types.go	75.00% <75.00%> (ø)
internal/trie/node/branch_encode.go	85.27% <76.00%> (-2.02%)	⬇️
lib/trie/lookup.go	73.91% <85.71%> (-2.09%)	⬇️
internal/trie/node/encode.go	93.33% <93.33%> (ø)
internal/trie/node/children.go	100.00% <100.00%> (ø)
internal/trie/node/copy.go	100.00% <100.00%> (ø)
internal/trie/node/decode.go	95.91% <100.00%> (-0.24%)	⬇️
internal/trie/node/dirty.go	100.00% <100.00%> (ø)
... and 18 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2cdfea...a509c65. Read the comment docs.

[kishansagathiya]

I wonder if this was possible by keeping node interface, moving common function into one struct and build leaf and branch using that common struct with the help of composition.

But, now that you have made this change, as long as all previous tests cases are covered it should be fine. Will revisit this in one more review!

[qdm12]

I wonder if this was possible by keeping node interface, moving common function into one struct and build leaf and branch using that common struct with the help of composition.

I thought about this, but I really wanted to get rid of all the type assertions and have a 'dumb' single node struct.
Maybe we can revisit later and try with composition though.

[qdm12]

Thanks for the review @kishansagathiya !
All your comments should be addressed now 😉