Page MenuHomePhabricator
Differential D3679

Remove config as a class member of BlockAssembler
AbandonedPublic
Actions

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 Passed
Unit
No 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
Herald added a reviewer: deadalnix. · View Herald TranscriptJul 16 2019, 16:38
teamcity added a comment.Jul 16 2019, 16:38

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 16 2019, 16:44

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B6867: Diff 10316!Jul 16 2019, 16:45
jasonbcox added a child revision: D3680: Refactor adding package txs to use config as a parameter.Jul 16 2019, 17:34
teamcity added a comment.Jul 16 2019, 17:35

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B6867: Diff 10316.Jul 16 2019, 17:44
teamcity added a comment.Jul 16 2019, 17:44

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

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.

teamcity added a comment.Jul 17 2019, 16:27

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

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)
jasonbcox added a child revision: D3471: Merge #9868: Abstract out the command line options for block assembly.
Harbormaster completed remote builds in B6876: Diff 10333.Jul 17 2019, 16:34
teamcity added a comment.Jul 17 2019, 16:34

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

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

Revision Contents
Changeset List

PathSize
src/
2 lines
15 lines
rpc/
6 lines
test/
64 lines
2 lines
3 lines

Diff 10316

View Options

src/miner.h

Loading...
View Options

src/miner.cpp

Loading...
View Options

src/rpc/mining.cpp

Loading...
View Options

src/test/miner_tests.cpp

Loading...
View Options

src/test/test_bitcoin.cpp

Loading...
View Options

src/test/validation_block_tests.cpp

Loading...