BIP 174 PSBT Serializations and RPCs #13557

achow101 · 2018-06-28T03:04:17Z

This Pull Request fully implements the updated BIP 174 specification. It is based upon #13425 which implements the majority of the signing logic.

BIP 174 specifies a binary transaction format which contains the information necessary for a signer to produce signatures for the transaction and holds the signatures for an input while the input does not have a complete set of signatures.

This PR contains structs for PSBT, serialization, and deserialzation code. Some changes to SignatureData have been made to support detection of UTXO type and storing public keys.

Many RPCs have been added to handle PSBTs.

walletprocesspsbt takes a PSBT format transaction, updates the PSBT with any inputs related to this wallet, signs, and finalizes the transaction. There is also an option to not sign and just update.

walletcreatefundedpsbt creates a PSBT from user provided data in the same form as createrawtransaction. It also funds the transaction and takes an options argument in the same form as fundrawtransaction. The resulting PSBT is blank with no input or output data filled in. It is analogous to a combination of createrawtransaction and fundrawtransaction

decodepsbt takes a PSBT and decodes it to JSON. It is analogous to decoderawtransaction

combinepsbt takes multiple PSBTs for the same tx and combines them. It is analogous to combinerawtransaction

finalizepsbt takes a PSBT and finalizes the inputs. If all inputs are final, it extracts the network serialized transaction and returns that instead of a PSBT unless instructed otherwise.

createpsbt is like createrawtransaction but for PSBTs instead of raw transactions.

convertpsbt takes a network serialized transaction and converts it into a psbt. The resulting psbt will lose all signature data and an explicit flag must be set to allow transactions with signature data to be converted.

This supersedes #12136

DrahtBot · 2018-06-28T09:28:00Z

Note to reviewers: This pull request conflicts with the following ones:

Directly operate with CMutableTransaction #13359 (wallet: Directly operate with CMutableTransaction by MarcoFalke)
refactor: privatize SignatureExtractorChecker [moveonly] #13266 (refactoring: privatize SignatureExtractorChecker by Empact)
RPC: Improve error messages on RPC endpoints that use GetTransaction #13144 (RPC: Improve error messages on RPC endpoints that use GetTransaction by jimpo)
Skip tx-rehashing on historic blocks #13098 (Skip tx-rehashing on historic blocks by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

promag

Review comments for:

fdcbcb3 - Inline Sign1 and SignN
a86d067 - Make SignatureData able to store signatures and scripts
c5d47a5 - Replace CombineSignatures with ProduceSignature
95f5a38 - Remove CombineSignatures and replace tests

fdcbcb3 is kind of unrelated here no?

promag · 2018-06-28T09:04:02Z

src/script/sign.cpp

+        unsigned int num_pubkeys = solutions.size()-2;
+        unsigned int last_success_key = 0;
+        for (const valtype& sig : stack.script) {
+            for (unsigned int i = last_success_key; i < num_pubkeys; i++) {


Commit "Make SignatureData able to store signatures and scripts"

nit, ++i

promag · 2018-06-28T09:09:27Z

src/script/sign.h

@@ -81,4 +91,15 @@ void UpdateInput(CTxIn& input, const SignatureData& data);
 * Solvability is unrelated to whether we consider this output to be ours. */
 bool IsSolvable(const SigningProvider& provider, const CScript& script);

+class SignatureExtractorChecker : public BaseSignatureChecker


Commit "Make SignatureData able to store signatures and scripts"

class SignatureExtractorChecker final?

promag · 2018-06-28T09:15:36Z

src/script/sign.cpp

+{
+    if (checker->CheckSig(scriptSig, vchPubKey, scriptCode, sigversion)) {
+        CPubKey pubkey(vchPubKey);
+        sigdata->signatures.emplace(pubkey.GetID(), SigPair(pubkey, scriptSig));


Commit "Make SignatureData able to store signatures and scripts"

This is called when pubkey.GetID() doesn't exists in signatures, maybe assert it is new:

auto i = sigdata->signatures.emplace(...); assert(i.second);

promag · 2018-06-28T09:16:40Z

src/script/sign.h

+class SignatureExtractorChecker : public BaseSignatureChecker
+{
+private:
+    SignatureData* sigdata;


Commit "Make SignatureData able to store signatures and scripts"

Use references instead of pointers?

promag · 2018-06-28T09:20:06Z

src/script/sign.cpp

@@ -33,14 +33,60 @@ bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provid
    return true;
 }

+static bool GetCScript(const SigningProvider* provider, const SignatureData* sigdata, const CScriptID &scriptid, CScript& script)


Commit "Replace CombineSignatures with ProduceSignature"

Looks like these could be references:

static bool GetCScript(const SigningProvider& provider, const SignatureData& sigdata, const CScriptID &scriptid, CScript& script)

and remove provider != nullptr below.

Otherwise fix space before scriptid argument. Same below in the definition.

promag · 2018-06-28T09:27:04Z

src/script/sign.cpp

+        return true;
+    }
+    // Look for pubkey in all partial sigs
+    const auto& it = sigdata->signatures.find(address);


Commit "Replace CombineSignatures with ProduceSignature"

As pointed by @MarcoFalke, don't use references to iterators. Check throughout too.

achow101 · 2018-06-28T17:52:59Z

@promag Just so you know, those commits are part of #13425

promag · 2018-06-28T18:01:00Z

Ops @achow101 do you want me to comment there?

achow101 · 2018-06-28T18:05:52Z

No need to comment twice. I will update that PR and then rebase this onto that.

sipa

Just comments on the serialization commit.

sipa · 2018-06-28T05:01:33Z

src/script/sign.h

+};
+
+/** A structure for PSBTs which contains per output information */
+struct PSBTOutput


Inconsistent class name (PartiallySignedInput vs PSBTOutput).

Done. Renamed PartiallySignedInput to PSBTInput

sipa · 2018-06-28T05:02:39Z

src/script/sign.h

+static const uint8_t PSBT_SEPARATOR = 0x00;
+
+template<typename Stream, typename... X>
+void SerializeToVector(Stream& s, const X&... args)


Perhaps add a comment to explain this function and the one below.

sipa · 2018-06-28T18:01:31Z

src/script/sign.h

+                    }
+
+                    // Read in the signature from value
+                    uint64_t value_len = ReadCompactSize(s);


I think this can be written as just std::vector<unsigned char> sig; s >> sig.

sipa · 2018-06-28T18:06:09Z

src/script/sign.h

+
+            // Do stuff based on type
+            switch(type) {
+                case PSBT_IN_NON_WITNESS_UTXO:


For this one and the next, check whether you don't already have the other UTXO type?

sipa · 2018-06-28T18:15:21Z

src/script/sign.h

+                        throw std::ios_base::failure("Duplicate Key, key for unknown value already provided");
+                    }
+                    // Read in the value
+                    uint64_t value_len = ReadCompactSize(s);


Here you can again just use std::vector<unsigned char> val_bytes; s >> val_bytes;.

sipa · 2018-06-28T18:16:37Z

src/script/sign.h

+                    s >> witness_script;
+                    break;
+                }
+                case PSBT_OUT_BIP32_DERIVATION:


Can you abstract out the serialization/deserialization code for derivation paths and scripts into separate functions? Otherwise it is needlessly duplicated across input and outputs.

sipa · 2018-06-28T18:44:44Z

src/script/sign.h

+    std::vector<PartiallySignedInput> inputs;
+    std::vector<PSBTOutput> outputs;
+    std::map<std::vector<unsigned char>, std::vector<unsigned char>> unknown;
+    uint64_t num_ins = 0;


Do you still need num_ins and use_in_index?

sipa · 2018-06-28T18:46:56Z

src/script/sign.h

+
+        // Read input data
+        unsigned int i = 0;
+        while (!s.empty() && i < tx.vin.size()) {


Why the !s.empty() check here? If we reach EOF in the stream an error should be raised, not ignore it.

If we reach EOF in the stream, this loop will exit and the if block below will throw the error as the number of inputs/outputs will differ from the number of inputs/outputs in the unsigned tx.

Ah, I see. That will probably give a more useful error.

sipa · 2018-06-28T21:28:24Z

src/script/sign.h

+
+
+// Note: These constants are in reverse byte order because serialization uses LSB
+static constexpr uint32_t PSBT_MAGIC_BYTES = 0x74627370;


You could also write it as uint8_t PSBT_MAGIC_BYTES[4] = "PSBT";. Byte arrays can be serialized directly now.

sipa · 2018-06-28T21:28:47Z

src/script/sign.h

+
+// The separator is 0x00. Reading this in means that the unserializer can interpret it
+// as a 0 length key. which indicates that this is the separator. The separator has no value.
+static const uint8_t PSBT_SEPARATOR = 0x00;


constexpr

sipa · 2018-06-28T21:29:28Z

src/script/sign.h

+}
+
+// Deserialize HD keypaths into a map
+static void DeserializeHDKeypaths(const std::vector<unsigned char>& key, std::map<CPubKey, std::vector<uint32_t>>& hd_keypaths, Stream& s)


Add template <typename Stream> before.

sipa · 2018-06-28T21:29:51Z

src/script/sign.h

+}
+
+// Serialize HD keypaths to a stream from a map
+static void SerializeHDKeypaths(std::vector<uint32_t>>& hd_keypaths, Stream& s)


Add template<typename Stream> before.

sipa · 2018-06-28T21:32:56Z

src/script/sign.h

+static void SerializeHDKeypaths(std::vector<uint32_t>>& hd_keypaths, Stream& s)
+{
+    for (auto keypath_pair : hd_keypaths) {
+        SerializeToVector(s, PSBT_IN_BIP32_DERIVATION, MakeSpan(keypath_pair.first));


Make the field type a function argument; it differs between input and output types.

sipa · 2018-06-28T21:36:45Z

src/script/sign.h

+
+        // Read input data
+        unsigned int i = 0;
+        while (!s.empty() && i < tx.vin.size()) {


Ah, I see. That will probably give a more useful error.

sipa

Comments on rawtransaction.cpp RPCs.

sipa · 2018-06-28T22:25:51Z

src/rpc/rawtransaction.cpp

+
+    RPCTypeCheck(request.params, {UniValue::VSTR});
+
+    // Unserialize the transactions


Would it make sense to abstract out this convert-base64-to-psbt RPC routine into a separate function? It's probably called from many RPCs.

sipa · 2018-06-28T22:26:40Z

src/rpc/rawtransaction.cpp

+            if (!input.final_script_witness.IsNull()) {
+                in.pushKV("final_scripWitness", input.final_script_witness.ToString());
+            }
+        }


Also add unknowns to the decode?

sipa · 2018-06-28T22:27:23Z

src/rpc/rawtransaction.cpp

+            "  \"bip32_derivs\" : [          (array of json objects, optional)\n"
+            "    {\n"
+            "      \"pubkey\" : \"pubkey\",                     (string) The public key this path corresponds to\n"
+            "      \"master_fingerprint\" : \"fingerprint\"     (string) The fingerprint of the master key\n"


What do you think about combing fingerprint and path into one string (like proposed in my descriptor language)?

I think it is clearer to list the fingerprint separately.

sipa · 2018-06-28T22:28:30Z

src/rpc/rawtransaction.cpp

+            }
+        }
+
+        for (const PartiallySignedTransaction& psbtx : psbtxs) {


It feels like this should be written as a method or function operating on PartiallySignedTransaction objects, rather than inside the RPC code.

Because it's not RPC specific. It's cleaner to put the operations on the lowest level they can go.

sipa · 2018-06-28T22:29:48Z

src/rpc/rawtransaction.cpp

+    for (unsigned int i = 0; i < merged_psbtx.tx.vin.size(); ++i) {
+        CTxIn& txin = merged_psbtx.tx.vin[i];
+
+        // Find the utxo from one of the psbtxs


I'm not sure what this entire UTXO handling section does. Is it sufficient to (a) rely on a generic combine PSBT function somewhere (suggested below) which also combined the utxo data, and then perhaps convert the logic in this section to a sanity check function?

I have refactored the merging stuff into a Merge() method for each PSBT struct. I also added a sanity checking method for each.

sipa · 2018-06-28T22:49:48Z

src/rpc/rawtransaction.cpp

+            "\nResult:\n"
+            "{\n"
+            "  \"base64\" : \"value\",        (string) The base64-encoded network transaction\n"
+            "  \"hex\" : \"value\",           (string) The hex-encoded partially signed transaction if extracted\n"


Fully signed transaction, no? It seems the explanation of this and the field above are swapped.

sipa · 2018-06-28T22:51:30Z

src/rpc/rawtransaction.cpp

+            continue;
+        }
+
+        // Fill a SignatureData with input info


This whole section that extracts UTXOs and data from a PSBT input and invokes ProduceSignature on it seems like something that can be abstracted out.

sipa · 2018-06-28T22:52:26Z

src/rpc/rawtransaction.cpp

+    if (request.fHelp || request.params.size() < 2 || request.params.size() > 4)
+        throw std::runtime_error(
+                            "createpsbt [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime ) ( replaceable )\n"
+                            "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n"


This RPC does not do any funding, I think.

Explain that this implements a Creator role.

sipa · 2018-06-28T23:04:06Z

src/uint256.h

@@ -91,6 +91,15 @@ class base_blob
               ((uint64_t)ptr[7]) << 56;
    }

+    uint32_t GetUint32LE(int pos) const


You can use ReadLE32 for this, it's more efficient.

If instead this would be BE (ReadBE32 also exists), you can get rid of Uint32ToUint8VectorLE and instead print using strprintf("%08x", fingerprint).

sipa · 2018-06-28T23:06:42Z

src/rpc/rawtransaction.cpp

+    if (request.fHelp || request.params.size() < 1 || request.params.size() > 3)
+        throw std::runtime_error(
+                            "converttopsbt \"hexstring\" ( permitsigdata iswitness )\n"
+                            "\nConverts a network serialized transaction to a PSBT.\n"


Perhaps clarify that it's intended to work with createrawtransaction and fundrawtransaction, and createpsbt/createfundedpsbt should be used for new applications.

sipa · 2018-06-28T23:13:58Z

General comment on naming of RPC field names and input arguments, "base64" isn't very informative I think. I suggest using "psbt" anywhere you have an input or output in base64. There isn't any other encoding used, so there should never be any confusion. Using "hex" for fully signed or legacy partially signed transaction makes sense for consistency with the existing RPCs.

achow101 · 2018-06-29T02:26:44Z

I did a bit of commit splitting and have reduced the size of the RPCs commit by splitting it up. Hopefully this will make review easier.

sipa · 2018-06-29T06:37:29Z

src/script/sign.cpp

@@ -235,6 +235,36 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
    return sigdata.complete;
 }

+bool SignPSBTInput(const SigningProvider& provider, const CTxIn& txin, const CMutableTransaction& tx, PSBTInput& input, SignatureData& sigdata, int index, int sighash, bool sign)


No need to pass txin; it's always equal to tx.vin[index] I think.

sipa · 2018-06-29T06:37:58Z

src/script/sign.cpp

+        utxo = input.non_witness_utxo->vout[txin.prevout.n];
+    }
+    // Now find the witness utxo if the non witness doesn't exist
+    else if (!input.witness_utxo.IsNull()) {


Nit: else on the same line as }.

sipa · 2018-06-29T06:41:26Z

src/wallet/rpcwallet.cpp

@@ -4388,6 +4391,333 @@ UniValue sethdseed(const JSONRPCRequest& request)
    return NullUniValue;
 }

+bool parse_hd_keypath(std::string keypath_str, std::vector<uint32_t>& keypath)


Nit: function naming style

sipa · 2018-06-29T06:41:38Z

src/wallet/rpcwallet.cpp

+    return true;
+}
+
+void add_keypath_to_map(const CWallet* pwallet, const CKeyID& keyID, std::map<CPubKey, std::vector<uint32_t>>& hd_keypaths)


Nit: function naming style

sipa · 2018-06-29T06:43:40Z

src/wallet/rpcwallet.cpp

+    hd_keypaths.emplace(vchPubKey, keypath);
+}
+
+bool fill_psbt(const CWallet* pwallet, PartiallySignedTransaction& psbtx, const CTransaction* txConst, int sighash_type, bool sign)


Nit: function naming style

nvk · 2018-06-29T18:16:28Z

We will start implementing as soon as this gets merged.
Thanks for sorting it out so fast 👍

sipa

Some more nits on serialization code.

sipa · 2018-06-30T00:34:06Z

src/script/sign.h

+    inline void Unserialize(Stream& s) {
+        // Read loop
+        while(!s.empty()) {
+            // read size of key


This section (up to s >> MakeSpan(key)) can be written more concisely as:

std::vector<unsigned char> key; s >> key; if (key.empty()) return;

sipa · 2018-06-30T00:39:29Z

src/script/sign.h

+                    if (non_witness_utxo) {
+                        throw std::ios_base::failure("Duplicate Key, input non-witness utxo already provided");
+                    }
+                    if (!witness_utxo.IsNull()) {


I guess this is now done by the IsSane() function, and doesn't need to be repeated here?

sipa · 2018-06-30T00:39:40Z

src/script/sign.h

+                        throw std::ios_base::failure("Duplicate Key, input witness utxo already provided");
+                    }
+                    if (non_witness_utxo) {
+                        throw std::ios_base::failure("A non-witness utxo was already provided, cannot provide a witness utxo");


Same thing here.

sipa · 2018-06-30T00:40:27Z

src/script/sign.h

+                    s >> sig;
+
+                    // Add to list
+                    partial_sigs.emplace(pubkey.GetID(), SigPair(pubkey, sig));


Can be std::move(sig).

sipa · 2018-06-30T00:44:10Z

src/script/sign.h

+    inline void Unserialize(Stream& s) {
+        // Read loop
+        while(!s.empty()) {
+            // read size of key


The same shorter read routine is possible here.

sipa · 2018-06-30T00:44:33Z

src/script/sign.h

+        // Read global data
+        while(!s.empty()) {
+            // read size of key
+            uint64_t key_len = ReadCompactSize(s);


And again here.

sipa · 2018-06-30T01:53:50Z

src/core_read.cpp

+            error = "extra data after PSBT";
+            return false;
+        }
+    } catch (const std::exception& e) {


Perhaps check for ssData.eof() here (so that adding garbage at the end of the base64 string is rejected).

This is checked for above with if (!ssData.empty()) {

I should learn to read.

sipa · 2018-06-30T01:54:04Z

src/core_read.cpp

@@ -160,6 +161,23 @@ bool DecodeHexBlk(CBlock& block, const std::string& strHexBlk)
    return true;
 }

+bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error)
+{
+    std::vector<unsigned char> txData = DecodeBase64(base64_tx.c_str());


Nit: variable naming style.

sipa · 2018-06-30T01:54:36Z

src/core_read.cpp

@@ -160,6 +161,23 @@ bool DecodeHexBlk(CBlock& block, const std::string& strHexBlk)
    return true;
 }

+bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error)
+{
+    std::vector<unsigned char> txData = DecodeBase64(base64_tx.c_str());


You can drop the .c_str().

DecodeBase64 takes an unsigned char* while base64_tx is a std::string. It fails to compile without c_str()

There is also a version of DecodeBase64 which takes a const std::string&.

Yes, but not to a std::vector<unsigned char>. It decodes to a std::string, but I need a std::vector<unsigned char>

sipa · 2018-06-30T01:55:14Z

src/rpc/rawtransaction.cpp

@@ -1262,6 +1263,551 @@ static UniValue testmempoolaccept(const JSONRPCRequest& request)
    return result;
 }

+static std::string WriteHDKeypath(std::vector<uint32_t>& keypath)


This adds a superfluous / at the end of the string.

sipa · 2018-06-30T01:57:01Z

src/rpc/rawtransaction.cpp

+{
+    if (request.fHelp || request.params.size() != 1)
+        throw std::runtime_error(
+            "decodepsbt \"base64string\"\n"


What do you think about renaming input psbt arguments from "base64" or "base64string" to "psbt" (and similar for output object field names)?

sipa · 2018-06-30T01:58:37Z

src/rpc/rawtransaction.cpp

+            "\nCombine multiple partially signed Bitcoin transactions into one transaction.\n"
+            "Implements the Combiner and Finalizer roles.\n"
+            "\nArguments:\n"
+            "1. \"txs\"                   (string) A json array of hex strings of partially signed transactions\n"


sipa · 2018-06-30T01:59:24Z

src/rpc/rawtransaction.cpp

+
+    PartiallySignedTransaction merged_psbt(psbtxs[0]); // Copy the first one
+    // Merge them
+    for (const PartiallySignedTransaction& psbtx : psbtxs) {


No need to merge in the first one again.

Changed to erase the first element from the vector.

sipa · 2018-06-30T01:59:58Z

src/rpc/rawtransaction.cpp

+        merged_psbt.Merge(psbtx);
+    }
+    if (!merged_psbt.IsSane()) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Merged PSBT is not sane");


"Not sane" sounds funny. What about "Resulting PSBT would be inconsistent"?

sipa · 2018-06-30T02:00:37Z

src/rpc/rawtransaction.cpp

+    UniValue result(UniValue::VOBJ);
+    CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
+    ssTx << merged_psbt;
+    return EncodeBase64((unsigned char*)ssTx.data(), ssTx.size());


Is the cast necessary here?

Yes. EncodeBase64 takes an unsigned char* but ssTx.data() is a char *.

sipa · 2018-06-30T02:05:06Z

src/rpc/rawtransaction.cpp

+                            "createpsbt and walletcreatefundedpsbt should be used for new applications.\n"
+                            "\nArguments:\n"
+                            "1. \"hexstring\"              (string, required) The hex string of a raw transaction\n"
+                            "2. permitsigdata           (boolean, optional) Whether the transaction hex contains any data in the scriptSigs or scriptWitnesses.\n"


This explanation is confusing. It's an input argument, right? How about "If true, the RPC will ignore any signatures present in the input. When this argument is false, it will fail if signatures are present."

5bad792 [test] PSBT RPC: check that bip32_derivs are present by default (Sjors Provoost) 29a21c9 [rpc] set default bip32derivs to true for psbt methods (Sjors Provoost) Pull request description: In #13557 (review) I recommended not including bip32 deriviation by default in PSBTs: > _Bit of a privacy issue_: let's say person A and B are about to spend from a multisig address, sending everything to person A. Person A gives their address to person B, their wallet wallet creates a PSBT, but doesn't sign it. Wallet A then calls `walletprocesspsbt` which signs it and _spontaneously adds the master_fingerprint and bip32 path_. Same issue with `walletcreatefundedpsbt`. > > Adding `bip32_derivs` should probably be opt-in. In practice I find this default quite annoying because I forget it and end up with a confused hardware wallet. More importantly, in the multisig example I provided, it's actually essential for the other side to know the derivation details (in addition to an xpub). This allows them to check that change is going to an address you can still co-sign for (because the spending policy is unchanged except for an index). ACKs for top commit: instagibbs: utACK 5bad792 jonatack: ACK 5bad792 code review, built, ran tests, inspected/messed around with/pprinted values from the new tests. Thanks for adding the tests. meshcollider: utACK 5bad792 Tree-SHA512: 22ad71dda96856060a96758c4ae7aafa22d5e9efba30e0c8287c711e7579849bd72593cbc0f41a2e9e8821315d78bda04e848dbb006283b841b2795e2faebcfd

…thods 5bad792 [test] PSBT RPC: check that bip32_derivs are present by default (Sjors Provoost) 29a21c9 [rpc] set default bip32derivs to true for psbt methods (Sjors Provoost) Pull request description: In bitcoin#13557 (review) I recommended not including bip32 deriviation by default in PSBTs: > _Bit of a privacy issue_: let's say person A and B are about to spend from a multisig address, sending everything to person A. Person A gives their address to person B, their wallet wallet creates a PSBT, but doesn't sign it. Wallet A then calls `walletprocesspsbt` which signs it and _spontaneously adds the master_fingerprint and bip32 path_. Same issue with `walletcreatefundedpsbt`. > > Adding `bip32_derivs` should probably be opt-in. In practice I find this default quite annoying because I forget it and end up with a confused hardware wallet. More importantly, in the multisig example I provided, it's actually essential for the other side to know the derivation details (in addition to an xpub). This allows them to check that change is going to an address you can still co-sign for (because the spending policy is unchanged except for an index). ACKs for top commit: instagibbs: utACK bitcoin@5bad792 jonatack: ACK 5bad792 code review, built, ran tests, inspected/messed around with/pprinted values from the new tests. Thanks for adding the tests. meshcollider: utACK 5bad792 Tree-SHA512: 22ad71dda96856060a96758c4ae7aafa22d5e9efba30e0c8287c711e7579849bd72593cbc0f41a2e9e8821315d78bda04e848dbb006283b841b2795e2faebcfd

The fix for CPubKey is a part of `Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet

The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet

Comment from 6f7b52a: "The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"

backported achow101 psbt implementation (barrystyle)

…coin#13666, bitcoin#13723: BIP 174 PSBT Serializations and RPCs (PR#4186)

merge bitcoin#13269, bitcoin#13425, bitcoin#13557, bitcoin#13721, bitcoin#13666, bitcoin#13723: from kittywhiskers:psbt

merge bitcoin#13269, bitcoin#13425, bitcoin#13557, bitcoin#13721, bitcoin#13666, bitcoin#13723: BIP 174 PSBT Serializations and RPCs

Comment from 6f7b52a: "The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"

[bitcoin#14021](bitcoin/bitcoin#14021) - Import key origin data through descriptors in importmulti [bitcoin#13557](bitcoin/bitcoin#13557) - BIP174 Serialization and logic only (partial merge) additional extensions to utilities.

promag reviewed Jun 28, 2018

View reviewed changes

This was referenced Jun 28, 2018

[WIP] support new multisig template in wallet for Solver, signing, and signature combining #13449

Closed

Return the script type from Solver #13429

Merged

sipa reviewed Jun 28, 2018

View reviewed changes

achow101 mentioned this pull request Jun 28, 2018

Moving final scriptSig construction from CombineSignatures to ProduceSignature (PSBT signer logic) #13425

Merged

achow101 force-pushed the psbt branch 3 times, most recently from ad1c9ce to 2eb4f61 Compare June 28, 2018 19:57

sipa reviewed Jun 28, 2018

View reviewed changes

achow101 force-pushed the psbt branch 4 times, most recently from 5162bb4 to ed7a484 Compare June 28, 2018 22:22

sipa reviewed Jun 28, 2018

View reviewed changes

achow101 force-pushed the psbt branch from ed7a484 to 00d2d6b Compare June 29, 2018 02:23

sipa reviewed Jun 29, 2018

View reviewed changes

achow101 force-pushed the psbt branch from 00d2d6b to d7422ea Compare June 29, 2018 18:15

sipa reviewed Jun 30, 2018

View reviewed changes

achow101 force-pushed the psbt branch 2 times, most recently from 5f22b5b to 3a0ca94 Compare June 30, 2018 01:14

sipa reviewed Jun 30, 2018

View reviewed changes

achow101 force-pushed the psbt branch 2 times, most recently from fe71185 to a871194 Compare June 30, 2018 02:46

Sjors mentioned this pull request Oct 26, 2019

rpc: set default bip32derivs to true for psbt methods #17264

Merged

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request May 19, 2021

Partial revert 478435d

47f2046

The fix for CPubKey is a part of `Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request May 19, 2021

Fix 19660: partial revert

1d240e7

The fix for CPubKey is a part of `Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request May 19, 2021

Fix 19660: partial revert

6f7b52a

The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet

kwvg mentioned this pull request May 20, 2021

Merge #19660, #19373, #19841, #13862, #13866, #17280, #17682 and partial #19326, #14978: Auxiliary Backports dashpay/dash#4164

Merged

kwvg added a commit to kwvg/dash that referenced this pull request May 20, 2021

Partial bitcoin#19660: Make HexStr take a span

5a4e75c

Comment from 6f7b52a: "The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"

kwvg added a commit to kwvg/dash that referenced this pull request May 20, 2021

Partial bitcoin#19660: Make HexStr take a span

dfd2a19

Comment from 6f7b52a: "The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"

kwvg added a commit to kwvg/dash that referenced this pull request May 20, 2021

Partial bitcoin#19660: Make HexStr take a span

bc2f11f

Comment from 6f7b52a: "The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"

barrystyle added a commit to pacprotocol/pacprotocol that referenced this pull request May 27, 2021

BIP 174 PSBT Serializations and RPCs bitcoin#13557

8e74396

backported achow101 psbt implementation (barrystyle)

barrystyle added a commit to pacprotocol/pacprotocol that referenced this pull request May 27, 2021

BIP 174 PSBT Serializations and RPCs bitcoin#13557

a8669d0

backported achow101 psbt implementation (barrystyle)

barrystyle added a commit to pacprotocol/pacprotocol that referenced this pull request May 31, 2021

BIP 174 PSBT Serializations and RPCs bitcoin#13557

4a6b6f2

backported achow101 psbt implementation (barrystyle)

kwvg pushed a commit to kwvg/dash that referenced this pull request Jun 4, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

bcfec0b

kwvg added a commit to kwvg/dash that referenced this pull request Jun 4, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

dbb1546

kwvg mentioned this pull request Jun 4, 2021

merge #13269, #13425, #13557, #13721, #13666, #13723: BIP 174 PSBT Serializations and RPCs dashpay/dash#4186

Merged

UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Jun 7, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

6964099

barrystyle added a commit to pacprotocol/pacprotocol that referenced this pull request Jun 8, 2021

merge bitcoin#13269, bitcoin#13425, bitcoin#13557, bitcoin#13721, bit…

acb122e

…coin#13666, bitcoin#13723: BIP 174 PSBT Serializations and RPCs (PR#4186)

barrystyle added a commit to pacprotocol/pacprotocol that referenced this pull request Jun 8, 2021

BIP 174 PSBT Serializations and RPCs (PR#4186)

9166796

merge bitcoin#13269, bitcoin#13425, bitcoin#13557, bitcoin#13721, bitcoin#13666, bitcoin#13723: from kittywhiskers:psbt

kwvg added a commit to kwvg/dash that referenced this pull request Jun 9, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

2b7d4a3

kwvg added a commit to kwvg/dash that referenced this pull request Jul 9, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

2799f70

kwvg added a commit to kwvg/dash that referenced this pull request Jul 13, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

bf9ae7c

kwvg added a commit to kwvg/dash that referenced this pull request Jul 13, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

04f913f

kwvg added a commit to kwvg/dash that referenced this pull request Jul 13, 2021

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

c00b3e9

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Jul 13, 2021

Merge pull request #4186 from kittywhiskers/psbt

e98241d

merge bitcoin#13269, bitcoin#13425, bitcoin#13557, bitcoin#13721, bitcoin#13666, bitcoin#13723: BIP 174 PSBT Serializations and RPCs

gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 26, 2022

Partial bitcoin#19660: Make HexStr take a span

58d7b3b

Comment from 6f7b52a: "The fix for CPubKey is a part of `bitcoin#13557: BIP 174 PSBT Serializations and RPCs` which wasn't backported yet"

gades pushed a commit to cosanta/cosanta-core that referenced this pull request Apr 30, 2022

Merge bitcoin#13557: BIP 174 PSBT Serializations and RPCs

6beff1d

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022



		// Note: These constants are in reverse byte order because serialization uses LSB
		static constexpr uint32_t PSBT_MAGIC_BYTES = 0x74627370;


		RPCTypeCheck(request.params, {UniValue::VSTR});

		// Unserialize the transactions

BIP 174 PSBT Serializations and RPCs #13557

BIP 174 PSBT Serializations and RPCs #13557

Conversation

achow101 commented Jun 28, 2018 • edited

DrahtBot commented Jun 28, 2018 • edited

promag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

promag Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

promag Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achow101 commented Jun 28, 2018

promag commented Jun 28, 2018

achow101 commented Jun 28, 2018

sipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achow101 Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achow101 commented Jun 28, 2018 •

edited

DrahtBot commented Jun 28, 2018 •

edited

promag Jun 28, 2018 •

edited

promag Jun 28, 2018 •

edited

achow101 Jun 28, 2018 •

edited

sipa Jun 28, 2018 •

edited