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

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

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 ↗	(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 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.