Page MenuHomePhabricator

Add outputtype module
ClosedPublic

Authored by deadalnix on Nov 11 2019, 01:03.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCc453a23e9675: Add outputtype module
Summary

Moves OutputType into its own module

  • Move AddAndGetDestinationForScript from wallet to outputype module

Makes AddAndGetDestinationForScript use a generic CKeyStore rather than
the wallet, and makes it always add the script to the keystore, rather
than only adding related (redeem) scripts.

  • segwit support for createmultisig RPC
  • [tests] functional test for createmultisig RPC

This is a backport of Core PR13072

Depends on D4423

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.Nov 11 2019, 01:03
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 11 2019, 01:03
Fabien added a subscriber: Fabien.Nov 12 2019, 07:33

What is the purpose of the OutputType for us, intended that we don't do segwit ?

It's not super useful, ut helps backports.

Fabien requested changes to this revision.Thu, Nov 14, 15:58
Fabien added inline comments.
src/outputtype.h
21 ↗(On Diff #14076)

Despite somewhat documented in a couple places, there is no such option in our codebase.

test/functional/rpc_createmultisig.py
26 ↗(On Diff #14076)

BTC => BCH

49 ↗(On Diff #14076)

Nit: spaces around operators

52 ↗(On Diff #14076)

Dito

This revision now requires changes to proceed.Thu, Nov 14, 15:58
deadalnix updated this revision to Diff 14160.Sat, Nov 16, 13:37

Address comments

deadalnix added inline comments.Sat, Nov 16, 14:46
test/functional/rpc_createmultisig.py
49 ↗(On Diff #14076)

I'm not sure what you want me to do here.

Fabien added inline comments.Sat, Nov 16, 22:10
test/functional/rpc_createmultisig.py
49 ↗(On Diff #14076)

It's just style nit, I want you to surround the operators with spaces, e.g. here:
total = 149 * 50 + (height - 149 - 100) * 25

I'll see if there is a way to tell autopep8 to do it automatically, but I think I did already search for this feature with no success.

deadalnix updated this revision to Diff 14165.Sun, Nov 17, 22:19

Add spaces

Fabien accepted this revision.Mon, Nov 18, 08:11
This revision is now accepted and ready to land.Mon, Nov 18, 08:11
This revision was automatically updated to reflect the committed changes.