Page MenuHomePhabricator

Do not import private keys to wallets with private keys disabled
ClosedPublic

Authored by deadalnix on Tue, Jan 7, 15:37.

Details

Summary
  • tests: unify RPC argument to cli argument conversion and handle dicts and lists

When running tests with --usecli, unify the conversion from argument objects to
strings using a new function arg_to_cli(). This fixes boolean arguments when
using named arguments.

Also use json.dumps() to get the string values for arguments that are dicts and
lists so that bitcoind's JSON parser does not become confused.

  • Refactor importwallet to extract data from the file and then import

Instead of importing keys and scripts as each line in the file is
read, first extract the data then import them.

  • Do not import private keys to wallets with private keys disabled

This is a backport of Core PR15235

Test Plan
make check
./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Tue, Jan 7, 15:37
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jan 7, 15:37
Fabien requested changes to this revision.Wed, Jan 8, 08:19
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/wallet/rpcdump.cpp
117 ↗(On Diff #15194)

Good catch !

727 ↗(On Diff #15194)

Nit: comment on it's own line

732 ↗(On Diff #15194)

Nit: use c++ constructor

738 ↗(On Diff #15194)

Dito

761 ↗(On Diff #15194)

Braces

769 ↗(On Diff #15194)

Nit: c++ constructor

This revision now requires changes to proceed.Wed, Jan 8, 08:19
deadalnix updated this revision to Diff 15244.Wed, Jan 8, 19:09

address feedback

Fabien accepted this revision.Thu, Jan 9, 09:11
Fabien added inline comments.
src/wallet/rpcdump.cpp
677 ↗(On Diff #15244)

Slightly out of scope, but while you're at it there these casts can also be constructors

This revision is now accepted and ready to land.Thu, Jan 9, 09:11