diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b2485651b5..880d339cd3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -5554,6 +5554,12 @@ extern UniValue sendrawtransaction(const JSONRPCRequest& request); template static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef, T_tx& tx_aux, T_merkle_block& merkleBlock) { + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) + return NullUniValue; + if (request.fHelp || request.params.size() < 2 || request.params.size() > 3) throw std::runtime_error( RPCHelpMan{"createrawpegin", @@ -5577,9 +5583,6 @@ static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef }, }.ToString()); - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); @@ -5695,6 +5698,11 @@ UniValue createrawpegin(const JSONRPCRequest& request) UniValue claimpegin(const JSONRPCRequest& request) { + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) + return NullUniValue; if (request.fHelp || request.params.size() < 2 || request.params.size() > 3) throw std::runtime_error( @@ -5716,8 +5724,6 @@ UniValue claimpegin(const JSONRPCRequest& request) }, }.ToString()); - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); CTransactionRef tx_ref; CMutableTransaction mtx; @@ -5728,8 +5734,17 @@ UniValue claimpegin(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Peg-ins cannot be completed during initial sync or reindexing."); } + // NOTE: Making an RPC from within another RPC is not generally a good idea. In particular, it + // is necessary to copy the URI, which contains the wallet if one was given; otherwise + // multi-wallet support will silently break. The resulting request object is still missing a + // bunch of other fields, although they are usually not used by RPC handlers. This is a + // brittle hack, and further examples of this pattern should not be introduced. + // Get raw peg-in transaction - UniValue ret(createrawpegin(request)); + JSONRPCRequest req; + req.URI = request.URI; + req.params = request.params; + UniValue ret(createrawpegin(req)); // See the note above, on why this is a bad idea. // Make sure it can be propagated and confirmed if (!ret["mature"].isNull() && ret["mature"].get_bool() == false) { @@ -5737,11 +5752,12 @@ UniValue claimpegin(const JSONRPCRequest& request) } // Sign it - JSONRPCRequest request2; + JSONRPCRequest req2; + req2.URI = request.URI; UniValue varr(UniValue::VARR); varr.push_back(ret["hex"]); - request2.params = varr; - UniValue result = signrawtransactionwithwallet(request2); + req2.params = varr; + UniValue result = signrawtransactionwithwallet(req2); // See the note above, on why this is a bad idea. if (!DecodeHexTx(mtx, result["hex"].get_str(), false, true)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); diff --git a/test/functional/feature_fedpeg.py b/test/functional/feature_fedpeg.py index d73fb9218f..79a5d176a1 100755 --- a/test/functional/feature_fedpeg.py +++ b/test/functional/feature_fedpeg.py @@ -229,6 +229,14 @@ def run_test(self): raw = parent.gettransaction(txid1)["hex"] + # Create a wallet in order to test that multi-wallet support works correctly for claimpegin + # (Regression test for https://github.com/ElementsProject/elements/issues/812 .) + sidechain.createwallet("throwaway") + # Set up our sidechain RPCs to use the first wallet (with empty name). We do this by + # overriding the RPC object in a hacky way, to avoid breaking a different hack on TestNode + # that enables generate() to work despite the deprecation of the generate RPC. + sidechain.rpc = sidechain.get_wallet_rpc("") + print("Attempting peg-ins") # First attempt fails the consensus check but gives useful result try: