Page MenuHomePhabricator
Differential D7859

[backport#17261 10/13] Box the wallet: Add multiple keyman maps and loops
ClosedPublic
Actions

Authored by majcosta on Oct 9 2020, 18:07.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABCe37acaefee4a: [backport#17261 10/13] Box the wallet: Add multiple keyman maps and loops
Summary

Add wallet logic for dealing with multiple ScriptPubKeyMan instances. This
doesn't change current behavior because there is still only a single
LegacyScriptPubKeyMan. But in the future the new logic will be used to support
descriptor wallets.

https://github.com/bitcoin/bitcoin/pull/17261/commits/c729afd0a3b74a3943e4c359270beaf3e6ff8a7b

Note: Had to bring in PR18052 to fix an instance of -Wmaybe-uninitialized

Depends on D7858

Partial backport of Core PR17261 and PR18052

Test Plan
ninja all check check-functional

Diff Detail

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

Event Timeline

majcosta created this revision.Oct 9 2020, 18:07
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 9 2020, 18:07
majcosta requested review of this revision.Oct 9 2020, 18:08
teamcity edited the summary of this revision. (Show Details)Oct 9 2020, 18:08
Harbormaster completed remote builds in B13318: Diff 24481.Oct 9 2020, 18:19
majcosta added a child revision: D7860: [backport#17261 11/13] Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyMan.Oct 9 2020, 18:20
deadalnix requested changes to this revision.Oct 9 2020, 22:23
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/wallet/scriptpubkeyman.cpp
441 ↗(On Diff #24481)

The very famous if/return/else pattern :)

src/wallet/scriptpubkeyman.h
242 ↗(On Diff #24481)

reformat.

This revision now requires changes to proceed.Oct 9 2020, 22:23
majcosta updated this revision to Diff 24490.Oct 10 2020, 03:40
majcosta marked 2 inline comments as done.

condensed wonky if-elseif-else conditional and fixed formatting

Harbormaster completed remote builds in B13326: Diff 24490.Oct 10 2020, 04:01
teamcity added a comment.Oct 10 2020, 04:02
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Snippet of first build failure:

rpc_getblockstats.py                             | ✓ Passed  | 1 s
rpc_getchaintips.py                              | ✓ Passed  | 2 s
rpc_help.py                                      | ✓ Passed  | 1 s
rpc_invalidateblock.py                           | ✓ Passed  | 6 s
rpc_misc.py                                      | ✓ Passed  | 1 s
rpc_named_arguments.py                           | ✓ Passed  | 1 s
rpc_net.py                                       | ✓ Passed  | 1 s
rpc_preciousblock.py                             | ✓ Passed  | 1 s
rpc_psbt.py                                      | ✓ Passed  | 29 s
rpc_rawtransaction.py                            | ✓ Passed  | 44 s
rpc_scantxoutset.py                              | ✓ Passed  | 4 s
rpc_setban.py                                    | ✓ Passed  | 2 s
rpc_signmessage.py                               | ✓ Passed  | 1 s
rpc_signrawtransaction.py                        | ✓ Passed  | 1 s
rpc_txoutproof.py                                | ✓ Passed  | 2 s
rpc_uptime.py                                    | ✓ Passed  | 1 s
rpc_users.py                                     | ✓ Passed  | 5 s
rpc_whitelist.py                                 | ✓ Passed  | 1 s
tool_wallet.py                                   | ✓ Passed  | 3 s
wallet_abandonconflict.py                        | ✓ Passed  | 4 s
wallet_address_types.py                          | ✓ Passed  | 13 s
wallet_avoidreuse.py                             | ✓ Passed  | 5 s
wallet_balance.py                                | ✓ Passed  | 14 s
wallet_basic.py                                  | ✓ Passed  | 33 s
wallet_coinbase_category.py                      | ✓ Passed  | 1 s
wallet_create_tx.py                              | ✓ Passed  | 6 s
wallet_createwallet.py                           | ✓ Passed  | 2 s
wallet_createwallet.py --usecli                  | ✓ Passed  | 3 s
wallet_disable.py                                | ✓ Passed  | 1 s
wallet_dump.py                                   | ✓ Passed  | 3 s
wallet_encryption.py                             | ✓ Passed  | 5 s
wallet_groups.py                                 | ✓ Passed  | 39 s
wallet_hd.py                                     | ✓ Passed  | 5 s
wallet_import_rescan.py                          | ✓ Passed  | 5 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importmulti.py                            | ✓ Passed  | 3 s
wallet_importprunedfunds.py                      | ✓ Passed  | 1 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 3 s
wallet_labels.py                                 | ✓ Passed  | 1 s
wallet_listreceivedby.py                         | ✓ Passed  | 19 s
wallet_listsinceblock.py                         | ✓ Passed  | 5 s
wallet_listtransactions.py                       | ✓ Passed  | 7 s
wallet_multiwallet.py                            | ✓ Passed  | 12 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 14 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 3 s
wallet_txn_clone.py                              | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
wallet_txn_doublespend.py                        | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
wallet_zapwallettxes.py                          | ✓ Passed  | 3 s

ALL                                              | ✓ Passed  | 800 s (accumulated) 
Runtime: 160 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1

Each failure log is accessible here:
Bitcoin ABC functional tests: feature_abortnode.py
Bitcoin ABC functional tests: p2p_feefilter.py
Bitcoin ABC functional tests: rpc_bind.py
Bitcoin ABC functional tests: rpc_fundrawtransaction.py
Bitcoin ABC functional tests: rpc_rawtransaction.py

deadalnix requested changes to this revision.Oct 10 2020, 13:32

You need to figure out why these tests are failing.

src/wallet/scriptpubkeyman.cpp
441 ↗(On Diff #24490)

Not only does this comment now does not make sense anymore, but you also kept the if/return/else pattern.

This revision now requires changes to proceed.Oct 10 2020, 13:32
majcosta updated this revision to Diff 24499.Oct 10 2020, 18:06

ha, yes. I think I understand what you mean. I think it looks better now.

Harbormaster completed remote builds in B13334: Diff 24499.Oct 10 2020, 18:25
deadalnix accepted this revision.Oct 10 2020, 19:27
This revision is now accepted and ready to land.Oct 10 2020, 19:27
Closed by commit rABCe37acaefee4a: [backport#17261 10/13] Box the wallet: Add multiple keyman maps and loops (authored by Andrew Chow <achow101-github@achow101.com>, committed by majcosta). · Explain WhyOct 10 2020, 19:46
This revision was automatically updated to reflect the committed changes.
majcosta added a commit: rABCe37acaefee4a: [backport#17261 10/13] Box the wallet: Add multiple keyman maps and loops.

Revision Contents
Changeset List

PathSize
src/
wallet/
24 lines
35 lines
28 lines
166 lines

Diff 24508

View Options

src/wallet/scriptpubkeyman.h

Loading...
View Options

src/wallet/scriptpubkeyman.cpp

Loading...
View Options

src/wallet/wallet.h

Loading...
View Options

src/wallet/wallet.cpp

Loading...