Differential D13071

[consensus] Move miner fund logic into its own function
ClosedPublic
Actions

Authored by sdulfari on Jan 27 2023, 22:59.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC9596b195f801: [consensus] Move miner fund logic into its own function

Summary

This will help simplify the migration to miner fund as policy. There is no
change in behavior.

The ideal design would not pull in fees as a function parameter but instead be
able to determine the fees from the block itself. This is inefficient to do
right now so we take advantage of the fact that the caller has already done
this calculation.

Test Plan

ninja check check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

sdulfari created this revision.Jan 27 2023, 22:59

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 27 2023, 22:59

sdulfari requested review of this revision.Jan 27 2023, 22:59

Harbormaster completed remote builds in B21887: Diff 37754.Jan 27 2023, 23:00

Build Bitcoin ABC Diffs / Diff Testing (lint-circular-dependencies) failed.

Tail of the build log:

/work /work/abc-ci-builds/lint-circular-dependencies
A new circular dependency in the form of "minerfund -> validation -> minerfund" appears to have been introduced.

/work/abc-ci-builds/lint-circular-dependencies
Build lint-circular-dependencies failed with exit code 1

Fabien requested changes to this revision.Jan 28 2023, 04:14

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/minerfund.cpp
14 ↗	(On Diff #37754)	See my comment regarding GetBlockSubsidy: you don't need this, and you will unbreak the circular dependencies linter
67 ↗	(On Diff #37754)	Also you can pass pprev instead of pindex, so you don't crash the node if if pindex is null
72 ↗	(On Diff #37754)	You should pass blockReward directly, the block subsidy is unrelated to the miner fund
75 ↗	(On Diff #37754)	Same here, you can pass the coinbase CTxOut instead of the whole block. That's not a big win for this function but it makes it easier to write unit tests