Add logger to FollowerState

[janezpodhostnik]

Extracting pieces from #3736 so its easier to digest and merge.

This one adds a currently unused log to follower state, to be used in the next PR.

[durkmurder]

The only thing I would suggest is to change logger to be the first argument, in many places we try to pass it as first argument, using automatic refactoring it could be done very easily.

[janezpodhostnik]

@durkmurder I feel like the tracer and the logger belong together contextually. I think it would make sense moving the tracer as well then:

// state. This implementation is suitable only for NON-Consensus nodes.
func NewFollowerState(
	logger zerolog.Logger,
	tracer module.Tracer,
	state *State,
	index storage.Index,
	payloads storage.Payloads,
	consumer protocol.Consumer,
	blockTimer protocol.BlockTimer,
) (*FollowerState, error) 

what do you think?

[durkmurder]

@durkmurder I feel like the tracer and the logger belong together contextually. I think it would make sense moving the tracer as well then:

// state. This implementation is suitable only for NON-Consensus nodes.

func NewFollowerState(

	logger zerolog.Logger,

	tracer module.Tracer,

	state *State,

	index storage.Index,

	payloads storage.Payloads,

	consumer protocol.Consumer,

	blockTimer protocol.BlockTimer,

) (*FollowerState, error) 

what do you think?

Yup, you could also move consumer as well, also attaching example where logger, consumers, metrics, trading are grouped together and later other dependencies

[codecov-commenter]

Codecov Report

Merging #4156 (d8ed58c) into master (52c46c8) will increase coverage by 2.38%.
The diff coverage is 35.71%.

@@            Coverage Diff             @@
##           master    #4156      +/-   ##
==========================================
+ Coverage   51.09%   53.48%   +2.38%     
==========================================
  Files         674      837     +163     
  Lines       60522    78472   +17950     
==========================================
+ Hits        30924    41967   +11043     
- Misses      27118    33152    +6034     
- Partials     2480     3353     +873     

Flag	Coverage Δ
unittests	53.48% <35.71%> (+2.38%)	⬆️

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

Impacted Files	Coverage Δ
cmd/execution_builder.go	0.00% <0.00%> (ø)
cmd/verification_builder.go	0.00% <0.00%> (ø)
state/protocol/badger/mutator.go	66.66% <100.00%> (+0.46%)	⬆️

... and 166 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[durkmurder]

Nice

[janezpodhostnik]

bors merge

[bors]

Build succeeded:

• Integration Tests (make -C integration access-tests)
• Integration Tests (make -C integration bft-tests)
• Integration Tests (make -C integration collection-tests)
• Integration Tests (make -C integration consensus-tests)
• Integration Tests (make -C integration epochs-tests)
• Integration Tests (make -C integration execution-tests)
• Integration Tests (make -C integration ghost-tests)
• Integration Tests (make -C integration mvp-tests)
• Integration Tests (make -C integration network-tests)
• Integration Tests (make -C integration verification-tests)
• Lint (./)
• Lint (./crypto/)
• Lint (./integration/)
• Unit Tests (access)
• Unit Tests (admin)
• Unit Tests (cmd)
• Unit Tests (consensus)
• Unit Tests (engine)
• Unit Tests (fvm)
• Unit Tests (ledger)
• Unit Tests (module)
• Unit Tests (network)
• Unit Tests (others)
• Unit Tests (utils)