Details

Reviewers

deadalnix
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGINGb10e567986f2: Move some static functions out of wallet.h/cpp
rABCb10e567986f2: Move some static functions out of wallet.h/cpp

Summary

Backport of Core PR10976

This change is desirable because wallet.h/cpp are monolithic and hard to
navigate, so pulling things out and grouping together pieces of related
functionality should improve the organization.

Another proximate motivation is the wallet process separation work in
https://github.com/bitcoin/bitcoin/pull/10973, where (at least initially)
parameter parsing and fee estimation are still done in the main process rather
than the wallet process, and having functions that run in different processes
scrambled up throughout wallet.cpp is unnecessarily confusing.

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

schancel created this revision.Sep 15 2018, 02:15

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 15 2018, 02:15

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

Update CMakeLists.txt

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

Harbormaster completed remote builds in B3297: Diff 4910.Sep 15 2018, 02:54

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

Harbormaster completed remote builds in B3298: Diff 4911.Sep 15 2018, 03:10

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

You forgot the changes to src/qt/optionsdialog.cpp

src/qt/sendcoinsdialog.cpp
20 ↗	(On Diff #4911)	Is that required ?

This revision now requires changes to proceed.Sep 15 2018, 12:03

Remove excess header

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

Harbormaster completed remote builds in B3308: Diff 4931.Sep 15 2018, 19:26

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

The changes to src/qt/optionsdialog.cpp are still missing.

This revision now requires changes to proceed.Sep 18 2018, 00:19

Remove unused header from optionsdialog.cpp as well

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

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

Harbormaster completed remote builds in B3333: Diff 4977.Sep 20 2018, 05:09

jasonbcox requested changes to this revision.Sep 20 2018, 21:13

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

src/wallet/fees.cpp
2 ↗	(On Diff #4977)	Add // Copyright (c) 2018 The Bitcoin developers
src/wallet/fees.h
2 ↗	(On Diff #4977)	Add // Copyright (c) 2018 The Bitcoin developers
32 ↗	(On Diff #4977)	The comment is identical to the one above. Please differentiate how targetFee affects this call. Or rather, how the above call uses a default targetFee.
src/wallet/init.cpp
2 ↗	(On Diff #4977)	Add // Copyright (c) 2018 The Bitcoin developers
src/wallet/init.h
2 ↗	(On Diff #4977)	Add // Copyright (c) 2018 The Bitcoin developers
19 ↗	(On Diff #4977)	Nit: Consider fixing this comment style to match the others in fees.h
src/wallet/wallet.h
1093 ↗	(On Diff #4977)	Should be this style: /* * comments... */

This revision now requires changes to proceed.Sep 20 2018, 21:13

schancel added inline comments.Sep 23 2018, 05:41

src/qt/sendcoinsdialog.cpp
20 ↗	(On Diff #4911)	Probably not. Got pulled in with the patch. I'll take it out.
src/wallet/fees.h
2 ↗	(On Diff #4977)	This shit isn't even necessary, why are we fiddling with it so often? Furthermore "Bitcoin developers" has no legal meaning whatsoever.
src/wallet/wallet.h
1093 ↗	(On Diff #4977)	Maybe `clang-format` shouldn't reformat things.