MOVEONLY: Move common init code to init/common by ryanofsky · Pull Request #21732 · bitcoin/bitcoin

ryanofsky · 2021-04-19T19:47:59Z

This PR is part of the process separation project.

This change is move-only and can be easily reviewed with --color-moved=dimmed_zebra. The moves are needed to avoid duplicating common init code between different binaries (bitcoin-node, bitcoin-wallet, etc) in #10102. In #10102, each binary has it's own init file (src/init/bitcoin-node.cpp, src/init/bitcoin-wallet.cpp) so this PR moves the common code to src/init/common.cpp.

ryanofsky · 2021-04-19T21:22:42Z

Updated cade20c -> 66bbbc0 (pr/initc.1 -> pr/initc.2, compare) to fix hidden_args CI errors https://cirrus-ci.com/task/4601873798791168?logs=ci#L3751 https://cirrus-ci.com/task/5446298728923136?logs=ci#L2663 https://cirrus-ci.com/task/5516667473100800?logs=ci#L4299 https://cirrus-ci.com/task/6079617426522112?logs=ci#L2817 https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38776308

promag

Code review ACK 66bbbc0.

Verified commit by commit, with --color-moved=dimmed-zebra. Moved code is tidy up in nice functions. I don't observe changes in behavior.

DrahtBot · 2021-04-20T01:52:08Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

log: Mitigate disk filling attacks by globally rate limiting LogPrintf(…) #21706 (log: Mitigate disk filling attacks by globally rate limiting LogPrintf(…) by dergoegge)
log: Mitigate disk filling attacks by rate limiting LogPrintf #21603 (log: Mitigate disk filling attacks by rate limiting LogPrintf by dergoegge)
util: Make default arg values more specific #19471 (util: Make default arg values more specific by hebasto)
Relog configuration args on debug.log rotation #16673 (Relog configuration args on debug.log rotation by LarryRuane)
Add fee_est tool for debugging fee estimation code #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

practicalswift · 2021-04-20T10:14:28Z

Concept ACK

Nice with the init namespace too: it makes it easier to reason about encapsulation and more specifically it allows for finding all non-init call sites just by doing git grep "init::".

More generally I think it would make sense to gradually introduce more of this type of namespace use in our project.

src/init/common.cpp

maflcko

Left some unrelated doc feedback to remove nonsense comments like

// Sanity checks
SanityChecks();

--color-moved=dimmed-zebra ACK 66bbbc0 😹

Show signature and timestamp

Signature:

Timestamp of file with hash 27c6f524949cc5fa807c0a855609234ba5afe961ff88e1b28074374eea19342f -

src/init/common.cpp

src/init.cpp

src/init/common.h

src/init.cpp

theStack · 2021-04-21T17:41:36Z

Concept ACK

ryanofsky

Updated 66bbbc0 -> 615965c (pr/initc.2 -> pr/initc.3, compare) with suggested changes

src/init.cpp

src/init/common.cpp

src/init/common.h

maflcko · 2021-04-22T06:09:32Z

review ACK 615965c 🖱

Show signature and timestamp

Signature:

Timestamp of file with hash 460d9d89b06872cded3ec1cd1b35c0c9337114cc3b7071cebe084cad1768397b -

practicalswift · 2021-04-22T18:55:10Z

cr ACK 615965c: dimmed zebra looks correct

ryanofsky added 6 commits April 19, 2021 06:11

Move common global init code to init/common

a67b548

Move common sanity check code to init/common

387c4cf

Move common logging AddArg code to init/common

90469c1

Move common logging GetArgs code to init/common

1fb7fcf

Move common logging start code to init/common

5bed2ab

Move common package version code to init/common

615965c

DrahtBot added Build system Refactoring labels Apr 19, 2021

ryanofsky force-pushed the pr/initc branch from cade20c to 66bbbc0 Compare April 19, 2021 21:20

promag reviewed Apr 19, 2021

View reviewed changes

This was referenced Apr 20, 2021

log: Mitigate disk filling attacks by globally rate limiting LogPrintf(…) #21706

Closed

log: Mitigate disk filling attacks by rate limiting LogPrintf #21603

Closed

util: Make default arg values more specific #19471

Closed

practicalswift mentioned this pull request Apr 20, 2021

refactor: Move more stuff to blockstorage #21727

Merged

This was referenced Apr 20, 2021

Relog configuration args on debug.log rotation #16673

Closed

Add fee_est tool for debugging fee estimation code #10443

Closed

kiminuo reviewed Apr 21, 2021

View reviewed changes

src/init/common.cpp Show resolved Hide resolved

maflcko approved these changes Apr 21, 2021

View reviewed changes

src/init/common.cpp Show resolved Hide resolved

src/init.cpp Outdated Show resolved Hide resolved

src/init.cpp Outdated Show resolved Hide resolved

src/init/common.h Outdated Show resolved Hide resolved

src/init.cpp Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/initc branch from 66bbbc0 to 615965c Compare April 21, 2021 20:22

ryanofsky commented Apr 21, 2021

View reviewed changes

maflcko merged commit 66fd3b2 into bitcoin:master Apr 23, 2021

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 23, 2021

Merge bitcoin#21732: MOVEONLY: Move common init code to init/common

bdb105b

ryanofsky mentioned this pull request Jun 16, 2021

common: Disallow calling IsArgSet() on ALLOW_LIST options #17783

Draft

gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022

Merge 66fd3b2 into merged_master (Bitcoin PR bitcoin/bitcoin#21732)

0f00842

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Conversation

ryanofsky commented Apr 19, 2021

Uh oh!

ryanofsky commented Apr 19, 2021

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Apr 20, 2021

Conflicts

Uh oh!

practicalswift commented Apr 20, 2021

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theStack commented Apr 21, 2021

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko commented Apr 22, 2021

Uh oh!

practicalswift commented Apr 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants