feat(lib/trie): add descendants count to branches

[qdm12]

Changes

• Count nodes added/removed for branches in the trie
• Adapt unit tests
• Add test cases to have full coverage

💁 descendants are stored in every node since we have a delete operation that cuts of an entire branch, so it's more performant to have it get the count of descendants to remove from the branch being removed, rather than iterating over this branch and all its children.

Next PR will be to publish the count of descendants of the root node to the Prometheus server

Tests

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

Issues

• Instrument the trie with Prometheus metrics #2229

Primary Reviewer

• @timwu20

[codecov]

Codecov Report

Merging #2310 (436cb26) into qdm12-trie-copy-less (228a8d7) will increase coverage by 0.16%.
The diff coverage is 100.00%.

@@                   Coverage Diff                    @@
##           qdm12-trie-copy-less    #2310      +/-   ##
========================================================
+ Coverage                 57.97%   58.13%   +0.16%     
========================================================
  Files                       213      214       +1     
  Lines                     27984    28014      +30     
========================================================
+ Hits                      16223    16286      +63     
+ Misses                    10120    10088      -32     
+ Partials                   1641     1640       -1     

Flag	Coverage Δ
unit-tests	58.13% <100.00%> (+0.16%)	⬆️

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

Impacted Files	Coverage Δ
internal/trie/node/branch.go	100.00% <100.00%> (ø)
internal/trie/node/copy.go	100.00% <100.00%> (ø)
internal/trie/node/decode.go	100.00% <100.00%> (ø)
internal/trie/node/stats.go	100.00% <100.00%> (ø)
lib/trie/trie.go	99.36% <100.00%> (+0.06%)	⬆️
lib/babe/epoch.go	25.94% <0.00%> (-1.94%)	⬇️
dot/types/babe_digest.go	19.64% <0.00%> (-1.93%)	⬇️
dot/network/notifications.go	68.24% <0.00%> (-1.02%)	⬇️
lib/babe/epoch_handler.go	77.01% <0.00%> (-0.49%)	⬇️
... and 10 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 228a8d7...436cb26. Read the comment docs.

[noot]

seems ok but I think I'm missing the reason why nodesCreated/nodesDeleted needs to be returned from a lot of the funcs? seems to be 0/1 only in some cases?

[qdm12]

seems ok but I think I'm missing the reason why nodesCreated/nodesDeleted needs to be returned from a lot of the funcs? seems to be 0/1 only in some cases?

Agreed, I'm doing a refactoring to deduce those numbers from the branch statistics (especially since there might be more fields in the future).

However, although it's working it really does run some weird/sketchy code which should be removed by #2352 can you please review this 😺 Thanks!!

[qdm12]

Do not merge, this is not valid (found with a fuzz test), I'm working on fixing it.

[qdm12]

@noot I reverted back to return recursively nodesRemoved/nodesCreated.

The trie code modifies the passed branches in-place, so to deduce descendants without returning values, it is required to either have:

• extremely confusing and ugly code; or
• deep copy branch nodes (not recursively) before passing them, but that hurts my feelings performance-wise

[qdm12]

Sorry @timwu20 I merged this by accident, do you mind approving the PR clone #2378? Thanks!!