Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix claimpegin and createrawpegin support for multiwallet. #813

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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);

Expand Down Expand Up @@ -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(
Expand All @@ -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;

Expand All @@ -5728,20 +5734,30 @@ 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) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Peg-in Bitcoin transaction needs more confirmations to be sent.");
}

// 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");
Expand Down
8 changes: 8 additions & 0 deletions test/functional/feature_fedpeg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down