Page MenuHomePhabricator

wallet: Recognize -disablewallet option early
ClosedPublic

Authored by PiRK on Oct 12 2020, 13:55.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC227c1121a755: wallet: Recognize -disablewallet option early
Summary

PR description:

This PR makes early check for the -disablewallet option.
If -disablewallet=1, objects PaymentServer and WalletController are nor created.

Backport of Core PR16436

Test Plan

ninja && ninja check
src/qt/bitcoin-qt -disablewallet

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Owners added a reviewer: Restricted Owners Package.Oct 12 2020, 13:55
PiRK requested review of this revision.Oct 12 2020, 13:55
deadalnix requested changes to this revision.Oct 12 2020, 15:03
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/qt/bitcoin.cpp
356 ↗(On Diff #24564)

This is a not part of the PR. Is there a missing dependency? Considering this is manually managed, if this is initialized twice, this is now leaking.

This revision now requires changes to proceed.Oct 12 2020, 15:03
PiRK requested review of this revision.Oct 12 2020, 16:17
PiRK added inline comments.
src/qt/bitcoin.cpp
356 ↗(On Diff #24564)

The problem is that D2134 change the indentation of the entire BitcoinApplication::initializeResult method in Bitcoin ABC compared to Bitcoin Core. But as far as I can tell, the behavior remained strictly identical.

Basically, D2134 changed

if (success) {
    A
} else {
    B
}

into

if (!success) {
    B
    return;
}
A

As a consequence, the diffs look widely different between here and https://github.com/bitcoin/bitcoin/pull/16436/files.
But as far as I can tell, it's the same code. clientModel was initially on line 375, and was moved above the m_wallet_controller initialization.

To make it clearer, I could first undo the change from D2134 in this piece of code, and then redo this backport.

This revision is now accepted and ready to land.Oct 12 2020, 16:55