Page MenuHomePhabricator

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

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 Passed
Unit
No Test Coverage
Build Status
Buildable 6965
Build 11977: Bitcoin ABC Buildbot (legacy)
Build 11976: arc lint + arc unit

Event Timeline

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 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
  • Rebase
  • Remove ComputeMaxGeneratedBlockSize() as it's no longer needed
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
deadalnix requested changes to this revision.Sep 27 2019, 13:10

This seems to be redundant now.

This revision now requires changes to proceed.Sep 27 2019, 13:10