Page MenuHomePhabricator
Differential D2002

Disallow using addresses in createmultisig
ClosedPublic
Actions

Authored by deadalnix on Nov 2 2018, 22:36.

Details

Reviewers
schancel
Fabien
Group Reviewers
Restricted Project
Commits
rSTAGING0b84b5e6f32a: Disallow using addresses in createmultisig
rABC0b84b5e6f32a: Disallow using addresses in createmultisig
Summary

Make createmultisig only accept public keys with the old functionality
marked as deprecated.

Splits _createmultisig_redeemscript into two functions, one for
getting public keys from UniValue and one for getting addresses
from UniValue and then their respective public keys. The one for
retrieving address's public keys is located in rpcwallet.cpp

Adds a test to deprecated_rpc.py for testing the deprecation.

Update the tests to give only public keys to createmultisig.

Depends on D1967 and D2001

Backport of Core PR11415

Test Plan
make VERBOSE=1 check && ./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
D2002
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 4428
Build 6921: Bitcoin ABC Buildbot (legacy)
Build 6920: arc lint + arc unit

Event Timeline

schancel created this revision.Nov 2 2018, 22:36
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 2 2018, 22:36
teamcity added a subscriber: teamcity.Nov 2 2018, 22:37

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

teamcity added a comment.Nov 2 2018, 22:39

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

Harbormaster failed remote builds in B3756: Diff 5635!Nov 2 2018, 22:39
schancel updated this revision to Diff 5682.Nov 6 2018, 23:58

Rebase on IsDeprecatedRPCEnabled parameter update

teamcity added a comment.Nov 6 2018, 23:58

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

teamcity added a comment.Nov 6 2018, 23:59

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

Harbormaster failed remote builds in B3796: Diff 5682!Nov 6 2018, 23:59
schancel edited the summary of this revision. (Show Details)Nov 7 2018, 00:04
deadalnix commandeered this revision.Jan 4 2019, 14:31
deadalnix added a reviewer: schancel.
deadalnix updated this revision to Diff 6499.Jan 4 2019, 14:33

Rebase, fix build, fix tests

teamcity added a comment.Jan 4 2019, 14:33

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

teamcity added a comment.Jan 4 2019, 14:42

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

Harbormaster completed remote builds in B4409: Diff 6499.Jan 4 2019, 14:43
deadalnix updated this revision to Diff 6529.Jan 6 2019, 22:57

rebase

teamcity added a comment.Jan 6 2019, 23:00

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

teamcity added a comment.Jan 6 2019, 23:08

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

Harbormaster completed remote builds in B4428: Diff 6529.Jan 6 2019, 23:08
Fabien requested changes to this revision.Jan 9 2019, 16:55
Fabien added a subscriber: Fabien.

Update version in the comments:

  • Deprecate from v0.19
  • Remove from v0.20

Also add an entry in the release note

This revision now requires changes to proceed.Jan 9 2019, 16:55
deadalnix updated this revision to Diff 8094.Apr 17 2019, 15:12

Put that diff back in shape

teamcity added a comment.Apr 17 2019, 15:13

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

teamcity added a comment.Apr 17 2019, 15:27

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

Harbormaster completed remote builds in B5479: Diff 8094.Apr 17 2019, 15:27
Fabien requested changes to this revision.Apr 17 2019, 18:18

Can you rename deprecated_rpc.py to rpc_deprecated.py to follow the new naming convention ?

doc/release-notes.md
12 ↗(On Diff #8094)

Nit: Using addresses in createmultisig is now deprectated. Use -deprecatedrpc=createmultisig to get the old behavior.

src/rpc/misc.cpp
18 ↗(On Diff #8094)

Can you use rpc/util.h here to avoid confusion ?

311 ↗(On Diff #8094)

The removal of the addresses mention in the help is missing from the PR

358 ↗(On Diff #8094)

Should braces be added for the else above in this case ?

src/util.cpp
13 ↗(On Diff #8094)

This seems unrelated ?

src/wallet/rpcwallet.cpp
1397 ↗(On Diff #8094)

v0.16 => v0.19
Also missing an entry in the release notes for this deprecation, and maybe a test case in rpc_deprecated.py.

This revision now requires changes to proceed.Apr 17 2019, 18:18
deadalnix updated this revision to Diff 8413.May 6 2019, 12:16

Update with whatever I have

deadalnix planned changes to this revision.May 6 2019, 12:16

This was in pretty bad shape, I'll run a round of review myself before asking other to do so.

teamcity added a comment.May 6 2019, 12:17

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

teamcity added a comment.May 6 2019, 12:31

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

Harbormaster completed remote builds in B5695: Diff 8413.May 6 2019, 12:31
deadalnix updated this revision to Diff 8414.May 6 2019, 13:15

Update version numbers as expected and address other comments.

teamcity added a comment.May 6 2019, 13:16

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

teamcity added a comment.May 6 2019, 13:30

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

Harbormaster completed remote builds in B5696: Diff 8414.May 6 2019, 13:31
Fabien requested changes to this revision.May 6 2019, 13:53

The original PR contains tabs alignment fixes on the rpc helps which are not ported here, I'm not sure if this is deliberate or not. I leave this up to you.

doc/release-notes.md
6 ↗(On Diff #8414)

Nit: o => to, add some backquotes

src/rpc/misc.cpp
352 ↗(On Diff #8414)

unsigned int => size_t

src/wallet/rpcwallet.cpp
1413 ↗(On Diff #8414)

unsigned int => size_t

This revision now requires changes to proceed.May 6 2019, 13:53
deadalnix updated this revision to Diff 8422.May 6 2019, 20:43

Update as per request

teamcity added a comment.May 6 2019, 20:44

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

teamcity added a comment.May 6 2019, 20:58

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

Harbormaster completed remote builds in B5698: Diff 8422.May 6 2019, 20:58
Fabien accepted this revision.May 6 2019, 21:05
This revision is now accepted and ready to land.May 6 2019, 21:05
Closed by commit rABC0b84b5e6f32a: Disallow using addresses in createmultisig (authored by Andrew Chow <achow101-github@achow101.com>, committed by deadalnix). · Explain WhyMay 6 2019, 22:34
This revision was automatically updated to reflect the committed changes.
nakihito mentioned this in D4022: Merge #12503: [RPC] createmultisig no longer takes addresses.Sep 6 2019, 18:35
deadalnix added a commit: rSTAGING0b84b5e6f32a: Disallow using addresses in createmultisig.Sep 18 2019, 23:17
Fabien mentioned this in D5526: Remove unused misc.h.Mar 20 2020, 17:28
Fabien mentioned this in rABC17c73ea83a2f: Remove unused misc.h.Mar 20 2020, 19:50
Fabien mentioned this in rSTAGING17c73ea83a2f: Remove unused misc.h.Apr 7 2020, 23:24

Revision Contents
Changeset List

PathSize
src/
1 line
2 lines
rpc/
124 lines
2 lines
2 lines
2 lines
22 lines
87 lines
1 line
wallet/
48 lines
test/
functional/
26 lines
6 lines
8 lines
5 lines
3 lines
18 lines
170 lines
2 lines

Diff 6529

View Options

src/CMakeLists.txt

Loading...
View Options

src/Makefile.am

Loading...
View Options

src/rpc/misc.cpp

Loading...
View Options

src/rpc/protocol.cpp

Loading...
View Options

src/rpc/safemode.cpp

Loading...
View Options

src/rpc/server.h

Loading...
View Options

src/rpc/util.h

Loading...
View Options

src/rpc/util.cpp

Loading...
View Options

src/util.cpp

Loading...
View Options

src/wallet/rpcwallet.cpp

Loading...
View Options

test/functional/deprecated_rpc.py

Loading...
View Options

test/functional/fundrawtransaction.py

Loading...
View Options

test/functional/importmulti.py

Loading...
View Options

test/functional/listtransactions.py

Loading...
View Options

test/functional/nulldummy.py

Loading...
View Options

test/functional/rawtransactions.py

Loading...
View Options

test/functional/timing.json

Loading...
View Options

test/functional/wallet_accounts.py

Loading...