Page MenuHomePhabricator

Remove config as a class member of BlockAssembler
AbandonedPublic

Authored by jasonbcox on Jul 16 2019, 16:38.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

This helps us prepare for a better backport of D3471.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
refactorcnb
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6867
Build 11781: Bitcoin ABC Buildbot (legacy)
Build 11780: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jul 16 2019, 16:38
deadalnix requested changes to this revision.Jul 16 2019, 23:46

It doesn't looks like this is creating a self consistent API. In fact, you are importing assumption about the whole program (namely, that there is one config) into BlockAssembler, which a very clear and very bad regression.

src/miner.h
164

You have a config here.

174

You are adding a new config parameter here. What if the don't match? What are the expectations here?

This revision now requires changes to proceed.Jul 16 2019, 23:46
jasonbcox updated this revision to Diff 10333.Jul 17 2019, 16:27

Pulled in D3680 - D3683 to make the API consistent. This removes the config member all in one go.

jasonbcox retitled this revision from Refactor CreateNewBlock() to use a config parameter instead of the class member to Remove config as a class member of BlockAssembler.Jul 17 2019, 16:28
jasonbcox edited the summary of this revision. (Show Details)
deadalnix requested changes to this revision.Jul 17 2019, 16:57

This has not addressed the concern one bit.

This revision now requires changes to proceed.Jul 17 2019, 16:57
jasonbcox abandoned this revision.Jul 17 2019, 18:26