Update createmultisig RPC to support segwit #13072
Conversation
|
I think this would resolve #12502 Thanks for working on this! |
7a43ed7 to
894da17
Compare
|
Nice! |
|
|
|
@luke-jr The existing "legacy" address type (argument to |
|
Ping @instagibbs @jonasschnelli ; Care to review code? |
src/outputtype.cpp
Outdated
| @@ -75,6 +75,8 @@ std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key) | |||
|
|
|||
| CTxDestination AddAndGetDestinationForScript(CKeyStore& keystore, const CScript& script, OutputType type) | |||
| { | |||
| keystore.AddCScript(script); | |||
There was a problem hiding this comment.
It moves the AddCScript() call from the function invoking AddAndGetDestinationForScript() into AAGDFS(); see corresponding change in src/wallet/rpcwallet.cpp
| CScriptID innerID(inner); | ||
| const CScript inner = CreateMultisigRedeemscript(required, pubkeys); | ||
| CBasicKeyStore keystore; | ||
| const CTxDestination dest = AddAndGetDestinationForScript(keystore, inner, output_type); |
There was a problem hiding this comment.
I didn't think this rpc call changed wallet state previously?
There was a problem hiding this comment.
This shouldn't change wallet state either -- keystore is a dummy variable that's just declared here and discarded. The functional tests don't work without wallet support compiled in, but I think I gave it a quick test by hand and it was okay.
|
|
||
| UniValue result(UniValue::VOBJ); | ||
| result.pushKV("address", EncodeDestination(innerID)); | ||
| result.pushKV("address", EncodeDestination(dest)); | ||
| result.pushKV("redeemScript", HexStr(inner.begin(), inner.end())); |
There was a problem hiding this comment.
This field becomes essentially useless for new address types. I'm not sure what to do about that.
There was a problem hiding this comment.
It could be changed to script and have redeemScript behind a deprecation switch.
| Needs rebase |
Moves OutputType into its own 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.
|
utACK f40b3b8 |
1 similar comment
|
utACK f40b3b8 |
|
utACK f40b3b8. |
f40b3b8 [tests] functional test for createmultisig RPC (Anthony Towns) b9024fd segwit support for createmultisig RPC (Anthony Towns) d58055d Move AddAndGetDestinationForScript from wallet to outputype module (Anthony Towns) 9a44db2 Add outputtype module (Anthony Towns) Pull request description: Adds an "address_type" parameter that accepts "legacy", "p2sh-segwit", and "bech32" to choose the type of address created. Defaults to "legacy" rather than the value of the `-address-type` option for backwards compatibility. As part of implementing this, OutputType is moved from wallet into its own module, and `AddAndGetDestinationForScript` is changed to apply to a `CKeyStore` rather than a wallet, and to invoke `keystore.AddCScript(script)` itself rather than expecting the caller to have done that. Fixes #12502 Tree-SHA512: a08c1cfa89976e4fd7d29caa90919ebd34a446354d17abb862e99f2ee60ed9bc19d8a21a18547c51dc3812cb9fbed86af0bef2f1e971f62bf95cade4a7d86237
Adds an "address_type" parameter that accepts "legacy", "p2sh-segwit", and "bech32" to choose the type of address created. Defaults to "legacy" rather than the value of the
-address-typeoption for backwards compatibility.As part of implementing this, OutputType is moved from wallet into its own module, and
AddAndGetDestinationForScriptis changed to apply to aCKeyStorerather than a wallet, and to invokekeystore.AddCScript(script)itself rather than expecting the caller to have done that.Fixes #12502