Page MenuHomePhabricator

Merge #9868: Abstract out the command line options for block assembly
Needs ReviewPublic

Authored by jasonbcox on Jun 29 2019, 00:05.

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Project
Summary

277b472 Run miner_tests with fixed options (Pieter Wuille)
48faf0b Abstract out BlockAssembler options (Pieter Wuille)

Tree-SHA512: 8e910904864c8653fb0358d820a4181d0496bdcbc3454c17a742af7505bfb59f7078c6d0faa9c753bdaf23b6d93d228a09913cc867fbddccd515483ebe7bfad0

Backport of Core PR9868
https://github.com/bitcoin/bitcoin/pull/9868/files

Reviewer Note: Some of the code has diverged of our own doing and even other parts have out-of-order backports applied.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
arcpatch-D3471
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6965
Build 11977: Bitcoin ABC Teamcity Staging
Build 11976: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jun 29 2019, 00:05
deadalnix requested changes to this revision.Jun 29 2019, 23:46
deadalnix added inline comments.
src/miner.h
164 ↗(On Diff #9798)

Why is that still useful?

179 ↗(On Diff #9798)

You don't need the config here.

This revision now requires changes to proceed.Jun 29 2019, 23:46
jasonbcox requested review of this revision.Jul 8 2019, 22:23
jasonbcox added inline comments.
src/miner.h
164 ↗(On Diff #9798)

It's needed because it's used a number of times in BlockAssembler::CreateNewBlock() and some other functions. It's not strictly needed, as the config could be passed into each function that needs it, but that is out of scope of this diff.

179 ↗(On Diff #9798)

It's used to call config.GetMaxBlockSize() to set a safe limit on nMaxGeneratedBlockSize, regardless of input provided via options.

deadalnix requested changes to this revision.Jul 9 2019, 01:30
deadalnix added inline comments.
src/miner.h
179 ↗(On Diff #9798)

To the contrary, it's very relevant, because the whole point of this backport is to bring configs to BloackAssembler.

This revision now requires changes to proceed.Jul 9 2019, 01:30
jasonbcox updated this revision to Diff 10335.Jul 17 2019, 18:17
  • Rebase
  • Remove ComputeMaxGeneratedBlockSize() as it's no longer needed
jasonbcox edited the summary of this revision. (Show Details)Jul 17 2019, 18:18
Fabien requested changes to this revision.Jul 25 2019, 07:02
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/miner.cpp
71 ↗(On Diff #10335)

Nit: for consistency, _mempool instead of mpool

97 ↗(On Diff #10335)

Dito

src/test/miner_tests.cpp
729 ↗(On Diff #10335)

ba2 => baWithOptions or something similar is more explicit.

This revision now requires changes to proceed.Jul 25 2019, 07:02
jasonbcox updated this revision to Diff 10474.Jul 25 2019, 16:06

Feedback + rebase

Fabien accepted this revision.Jul 26 2019, 08:27