Introduce miner fund policy whitelist
AbandonedPublic
Actions

Authored by sdulfari on Feb 1 2023, 22:18.

Details

Reviewers

Group Reviewers

Restricted Project

Summary

We want to be able to construct a miner fund whitelist that abides by consensus
rules, policy rules, or either. This makes it easy to write code that doesn't
care about the reason for the whitelist to be used (RPC, mining, etc), only the
contents of the whitelist, without writing special logic to handle two
different lists (consensus vs policy).

This patch introduces construction of the policy whitelist without using it.

Depends on D13071 and D13094 and D13113

Test Plan

ninja check check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Branch

miner-fund-flags

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21964
Build 43563: Build Diff	build-without-wallet · build-debug · build-clang-tidy · build-diff · build-clang · lint-circular-dependencies
Build 43562: arc lint + arc unit

Event Timeline

sdulfari created this revision.Feb 1 2023, 22:18

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 1 2023, 22:18

sdulfari requested review of this revision.Feb 1 2023, 22:18

Harbormaster completed remote builds in B21916: Diff 37789.Feb 1 2023, 22:34

I don't think this design makes sense. Consensus and policy rules are exclusive, so using a bitfield for the flag just creates a bug opportunity for no benefit.

This revision now requires changes to proceed.Feb 5 2023, 10:04

Remove the bit flags in favor of a wrapper that calls the appropriate validation path depending on Wellington activation

sdulfari retitled this revision from Introduce miner fund flags to Introduce miner fund policy whitelist.Feb 8 2023, 22:30

sdulfari edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B21964: Diff 37860.Feb 8 2023, 22:48

So I think the idea of having no activation switch in the validation flow is actually the source of the problem.
Instead of simplifying it's hiding the validation flow, making it complicated and opaque: just keep it simple, and activate the feature (consensus or policy check) depending on wellington activation. That makes the switch appear twice in the code for the next 3 months, but a nice comment can be added to explain the validation flow and to remember to remove the consensus check after the activation.

src/minerfund.cpp
51–98	I'm sorry but I don't understand what you're doing here. This is 99% boilerplate and still have a tristate ? Suggestion is how the code should look like imo.

This revision now requires changes to proceed.Feb 9 2023, 09:40

sdulfari abandoned this revision.Mar 19 2023, 21:28