Fix claimpegin and createrawpegin support for multiwallet.

Fix multiple issues with claimpegin and createrawpegin support for
multiple loaded wallets:

- Failure to call EnsureWalletIsAvailable would cause a crash when multiple
  wallets were loaded, but one was not specified in the RPC call.
- Failure to propagate the request URL from claimpegin when making an
  internal call to another RPC would result in failure when multiple
  wallets were loaded, by failing to specify one for that call.

Add a regression test to the feature_fedpeg.py test: at a critical point,
create a second wallet on the sidechaind, and set up the RPC client to
use the first one, to check that it still works correctly.
This commit is contained in:
Glenn Willen 2020-02-06 23:08:56 -08:00
parent da3611a580
commit fc841c8047
2 changed files with 33 additions and 9 deletions

View file

@ -5554,6 +5554,12 @@ extern UniValue sendrawtransaction(const JSONRPCRequest& request);
template<typename T_tx_ref, typename T_tx, typename T_merkle_block>
static UniValue createrawpegin(const JSONRPCRequest& request, T_tx_ref& txBTCRef, T_tx& tx_aux, T_merkle_block& merkleBlock)
{
std::shared_ptr<CWallet> 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<CWallet> 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<CWallet> 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<CWallet> 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");

View file

@ -229,6 +229,14 @@ class FedPegTest(BitcoinTestFramework):
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: