Page MenuHomePhabricator

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

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


Group Reviewers
Restricted Project

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

Tree-SHA512: 8e910904864c8653fb0358d820a4181d0496bdcbc3454c17a742af7505bfb59f7078c6d0faa9c753bdaf23b6d93d228a09913cc867fbddccd515483ebe7bfad0

Backport of Core PR9868

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

rABC Bitcoin ABC
Lint OK
No Unit Test Coverage
Build Status
Buildable 6878
Build 11803: Bitcoin ABC Teamcity Staging
Build 11802: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Sat, Jun 29, 00:05
deadalnix requested changes to this revision.Sat, Jun 29, 23:46
deadalnix added inline comments.
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.Sat, Jun 29, 23:46
jasonbcox requested review of this revision.Mon, Jul 8, 22:23
jasonbcox added inline comments.
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.Tue, Jul 9, 01:30
deadalnix added inline comments.
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.Tue, Jul 9, 01:30
jasonbcox updated this revision to Diff 10335.Wed, Jul 17, 18:17
  • Rebase
  • Remove ComputeMaxGeneratedBlockSize() as it's no longer needed
jasonbcox edited the summary of this revision. (Show Details)Wed, Jul 17, 18:18