Paths

Table of Contentst

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

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

pr9868

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6553
Build 11153: Bitcoin ABC Buildbot (legacy)
Build 11152: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jun 29 2019, 00:05

Herald added a reviewer: deadalnix. · View Herald TranscriptJun 29 2019, 00:05

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

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

Harbormaster completed remote builds in B6553: Diff 9798.Jun 29 2019, 00:12

deadalnix requested changes to this revision.Jun 29 2019, 23:46

deadalnix added inline comments.

src/miner.h
164	Why is that still useful?
179	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	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	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	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 mentioned this in D3679: Remove config as a class member of BlockAssembler.Jul 16 2019, 16:38

Rebase on D3683

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

jasonbcox edited the summary of this revision. (Show Details)Jul 16 2019, 21:12

jasonbcox added a parent revision: D3683: Remove config as a class member of BlockAssembler.

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

Harbormaster completed remote builds in B6872: Diff 10326.Jul 16 2019, 21:18

jasonbcox edited the summary of this revision. (Show Details)Jul 17 2019, 16:29

jasonbcox added a parent revision: D3679: Remove config as a class member of BlockAssembler.

Rebase
Remove ComputeMaxGeneratedBlockSize() as it's no longer needed

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

jasonbcox edited the summary of this revision. (Show Details)Jul 17 2019, 18:18

Harbormaster completed remote builds in B6878: Diff 10335.Jul 17 2019, 18:24

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

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.