relab / hotstuff

MIT License
172 stars 53 forks source link

Refactor module system #73

Closed johningve closed 2 years ago

johningve commented 2 years ago

This PR changes how the module system works. Modules can no longer directly access a module through the "Core" object in this manner: mods.Eventloop(). Instead, modules are expected to declare a local variable that should hold the module, and then call one of the new Get methods to retrieve the module. For example:

var eventLoop *eventloop.Eventloop
mods.Get(&eventLoop)

Additionally, this PR adds a document that describes the ideas behind the module system and why it is necessary, and fixes a couple of mistakes.

codecov-commenter commented 2 years ago

Codecov Report

Merging #73 (042e6b4) into master (6152764) will increase coverage by 4.88%. The diff coverage is 78.81%.

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   57.81%   62.70%   +4.88%     
==========================================
  Files          70       73       +3     
  Lines        6427     7197     +770     
==========================================
+ Hits         3716     4513     +797     
+ Misses       2424     2384      -40     
- Partials      287      300      +13     
Flag Coverage Δ
unittests 62.70% <78.81%> (+4.88%) :arrow_up:

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

Impacted Files Coverage Δ
consensus/byzantine/byzantine.go 1.92% <0.00%> (-0.21%) :arrow_down:
crypto/bitfield.go 96.61% <0.00%> (-3.39%) :arrow_down:
genesis.go 100.00% <ø> (ø)
internal/cli/run.go 16.12% <0.00%> (ø)
leaderrotation/carousel.go 3.77% <0.00%> (-0.49%) :arrow_down:
leaderrotation/reputation.go 3.07% <0.00%> (-0.26%) :arrow_down:
metrics/clientlatency.go 2.63% <0.00%> (-0.94%) :arrow_down:
metrics/plotting/clientlatency.go 0.00% <0.00%> (ø)
metrics/plotting/reader.go 0.00% <ø> (ø)
metrics/plotting/starttimes.go 0.00% <0.00%> (ø)
... and 72 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

johningve commented 2 years ago

@meling I decided to drop the explicit modules.Implements[T] declaration for now. The rule now is that mods.Get will give the last compatible value that was added to the modules builder.

johningve commented 2 years ago

@meling I have myself reviewed all the changes and am happy with what is here. If you think that the changes to the module system look okay, then I think that we can merge it