diff options
author | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-08-21 08:59:35 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2018-08-21 09:44:26 +0200 |
commit | 8aa9badf5ea3ea98980f50d924e88e46c4d5ee38 (patch) | |
tree | b96ab86c1c66db8b4b49d92a510e1f353b040f2f | |
parent | 4732fa133abdf374790825abbf1bbf56819cc806 (diff) | |
parent | faaac5caaab4d5131040292f4ef2404074ad268b (diff) | |
download | bitcoin-8aa9badf5ea3ea98980f50d924e88e46c4d5ee38.tar.xz |
Merge #13968: [wallet] couple of walletcreatefundedpsbt fixes
faaac5caaab4d5131040292f4ef2404074ad268b RPCTypeCheck bip32derivs arg in walletcreatefunded (Gregory Sanders)
1f0c4282e961baea85d5f74d7493bd7459784391 QA: add basic walletcreatefunded optional arg test (Gregory Sanders)
1f18d7b591ffcc8bb9422a9b728bd9a0d8da6a2a walletcreatefundedpsbt: remove duplicate replaceable arg (Gregory Sanders)
2252ec50085c151e7998ca9a30cda6a33ee862b6 Allow ConstructTransaction to not throw error with 0-input txn (Gregory Sanders)
Pull request description:
1) Previously an empty input argument transaction that is marked for replaceability fails to pass the `SignalsOptInRBF` check right before funding it. Explicitly check for that condition before throwing an error.
2) The rpc call had two separate `replaceable` arguments, each of which being used in mutually exclusive places. I preserved the `options` version to retain compatability with `fundtransaction`.
Tree-SHA512: 26eb0c9e2d38ea51d11f741d61100223253271a084adadeb7e78c6d4e9004636f089e4273c5bf64a41bd7e9ff795317acf30531cb36aeb0d8db9304b3c8270c3
-rw-r--r-- | src/rpc/client.cpp | 5 | ||||
-rw-r--r-- | src/rpc/rawtransaction.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 19 | ||||
-rwxr-xr-x | test/functional/rpc_psbt.py | 29 |
4 files changed, 41 insertions, 14 deletions
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 6738b206c9..c7f3e38ac0 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -113,9 +113,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "walletcreatefundedpsbt", 0, "inputs" }, { "walletcreatefundedpsbt", 1, "outputs" }, { "walletcreatefundedpsbt", 2, "locktime" }, - { "walletcreatefundedpsbt", 3, "replaceable" }, - { "walletcreatefundedpsbt", 4, "options" }, - { "walletcreatefundedpsbt", 5, "bip32derivs" }, + { "walletcreatefundedpsbt", 3, "options" }, + { "walletcreatefundedpsbt", 4, "bip32derivs" }, { "walletprocesspsbt", 1, "sign" }, { "walletprocesspsbt", 3, "bip32derivs" }, { "createpsbt", 0, "inputs" }, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 608a1b5da2..314184ab06 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -436,7 +436,7 @@ CMutableTransaction ConstructTransaction(const UniValue& inputs_in, const UniVal } } - if (!rbf.isNull() && rbfOptIn != SignalsOptInRBF(rawTx)) { + if (!rbf.isNull() && rawTx.vin.size() > 0 && rbfOptIn != SignalsOptInRBF(rawTx)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter combination: Sequence number(s) contradict replaceable option"); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 281fd46146..0966334b82 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4643,7 +4643,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() < 2 || request.params.size() > 6) + if (request.fHelp || request.params.size() < 2 || request.params.size() > 5) throw std::runtime_error( "walletcreatefundedpsbt [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime ) ( replaceable ) ( options bip32derivs )\n" "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n" @@ -4670,9 +4670,8 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) " accepted as second parameter.\n" " ]\n" "3. locktime (numeric, optional, default=0) Raw locktime. Non-0 value also locktime-activates inputs\n" - "4. replaceable (boolean, optional, default=false) Marks this transaction as BIP125 replaceable.\n" " Allows this transaction to be replaced by a transaction with higher fees. If provided, it is an error if explicit sequence numbers are incompatible.\n" - "5. options (object, optional)\n" + "4. options (object, optional)\n" " {\n" " \"changeAddress\" (string, optional, default pool address) The bitcoin address to receive the change\n" " \"changePosition\" (numeric, optional, default random) The index of the change output\n" @@ -4694,7 +4693,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) " \"ECONOMICAL\"\n" " \"CONSERVATIVE\"\n" " }\n" - "6. bip32derivs (boolean, optiona, default=false) If true, includes the BIP 32 derivation paths for public keys if we know them\n" + "5. bip32derivs (boolean, optiona, default=false) If true, includes the BIP 32 derivation paths for public keys if we know them\n" "\nResult:\n" "{\n" " \"psbt\": \"value\", (string) The resulting raw transaction (base64-encoded string)\n" @@ -4710,15 +4709,15 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) UniValue::VARR, UniValueType(), // ARR or OBJ, checked later UniValue::VNUM, - UniValue::VBOOL, - UniValue::VOBJ + UniValue::VOBJ, + UniValue::VBOOL }, true ); CAmount fee; int change_position; - CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]); - FundTransaction(pwallet, rawTx, fee, change_position, request.params[4]); + CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], request.params[3]["replaceable"]); + FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]); // Make a blank psbt PartiallySignedTransaction psbtx; @@ -4735,7 +4734,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) const CTransaction txConst(*psbtx.tx); // Fill transaction with out data but don't sign - bool bip32derivs = request.params[5].isNull() ? false : request.params[5].get_bool(); + bool bip32derivs = request.params[4].isNull() ? false : request.params[5].get_bool(); FillPSBT(pwallet, psbtx, &txConst, 1, false, bip32derivs); // Serialize the PSBT @@ -4765,7 +4764,7 @@ static const CRPCCommand commands[] = // --------------------- ------------------------ ----------------------- ---------- { "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options","iswitness"} }, { "wallet", "walletprocesspsbt", &walletprocesspsbt, {"psbt","sign","sighashtype","bip32derivs"} }, - { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","replaceable","options","bip32derivs"} }, + { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} }, { "hidden", "resendwallettransactions", &resendwallettransactions, {} }, { "wallet", "abandontransaction", &abandontransaction, {"txid"} }, { "wallet", "abortrescan", &abortrescan, {} }, diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 99c4131d61..f847a01e59 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -11,6 +11,8 @@ from test_framework.util import assert_equal, assert_raises_rpc_error, find_outp import json import os +MAX_BIP125_RBF_SEQUENCE = 0xfffffffd + # Create one-input, one-output, no-fee transaction: class PSBTTest(BitcoinTestFramework): @@ -135,6 +137,33 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].generate(6) self.sync_all() + # Test additional args in walletcreatepsbt + # Make sure both pre-included and funded inputs + # have the correct sequence numbers based on + # replaceable arg + block_height = self.nodes[0].getblockcount() + unspent = self.nodes[0].listunspent()[0] + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height+2, {"replaceable":True}) + decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) + for tx_in in decoded_psbt["tx"]["vin"]: + assert_equal(tx_in["sequence"], MAX_BIP125_RBF_SEQUENCE) + assert_equal(decoded_psbt["tx"]["locktime"], block_height+2) + + # Same construction with only locktime set + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}], block_height) + decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) + for tx_in in decoded_psbt["tx"]["vin"]: + assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE + assert_equal(decoded_psbt["tx"]["locktime"], block_height) + + # Same construction without optional arguments + psbtx_info = self.nodes[0].walletcreatefundedpsbt([{"txid":unspent["txid"], "vout":unspent["vout"]}], [{self.nodes[2].getnewaddress():unspent["amount"]+1}]) + decoded_psbt = self.nodes[0].decodepsbt(psbtx_info["psbt"]) + for tx_in in decoded_psbt["tx"]["vin"]: + assert tx_in["sequence"] > MAX_BIP125_RBF_SEQUENCE + assert_equal(decoded_psbt["tx"]["locktime"], 0) + + # BIP 174 Test Vectors # Check that unknown values are just passed through |