From 7103471fd5008be82e7790ef3d5608899967d942 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 28 Sep 2021 21:48:15 +0000 Subject: [PATCH 1/8] walletcreatefundedpsbt: signal blinding data correctly to `FundTransaction` --- src/wallet/rpcwallet.cpp | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c4f30b9973..b234c37d02 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4957,8 +4957,16 @@ static RPCHelpMan walletcreatefundedpsbt() // Automatically select coins, unless at least one is manually selected. Can // be overridden by options.add_inputs. coin_control.m_add_inputs = rawTx.vin.size() == 0; + // FundTransaction expects blinding keys, if present, to appear in the output nonces + for (CTxOut& txout : rawTx.vout) { + auto search_it = psbt_outs.find(txout); + assert (search_it != psbt_outs.end()); + CPubKey& blind_pub = search_it->second.m_blinding_pubkey; + if (blind_pub.IsFullyValid()) { + txout.nNonce.vchCommitment = std::vector(blind_pub.begin(), blind_pub.end()); + } + } FundTransaction(pwallet, rawTx, fee, change_position, request.params[3], coin_control, /* solving_data */ request.params[5], /* override_min_fee */ true); - PartiallySignedTransaction psbtx(rawTx, psbt_version); // Find an input that is ours unsigned int blinder_index = 0; { @@ -4971,6 +4979,17 @@ static RPCHelpMan walletcreatefundedpsbt() } } assert(blinder_index < rawTx.vin.size()); // We added inputs, or existing inputs are ours, we should have a blinder index at this point. + // It may add outputs (change, and in some edge case OP_RETURN) which need to be + // blinded. So pull these into `psbt_outs`. + for (const CTxOut& txout : rawTx.vout) { + if (!txout.nNonce.IsNull() && !psbt_outs.count(txout)) { + PSBTOutput new_out{2}; // psbtv2 output + new_out.m_blinding_pubkey.Set(txout.nNonce.vchCommitment.begin(), txout.nNonce.vchCommitment.end()); + new_out.m_blinder_index = blinder_index; + psbt_outs.insert(std::make_pair(txout, new_out)); + } + } + PartiallySignedTransaction psbtx(rawTx, psbt_version); for (unsigned int i = 0; i < rawTx.vout.size(); ++i) { PSBTOutput& output = psbtx.outputs[i]; auto it = psbt_outs.find(rawTx.vout.at(i)); From 9813c3e74a7e9472bff9f44e6bd67c42a216244e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 29 Sep 2021 20:24:43 +0000 Subject: [PATCH 2/8] wallet: fix "cannot unblind IsMine output" check in SignPSBT --- src/util/error.cpp | 4 ++++ src/util/error.h | 2 ++ src/wallet/wallet.cpp | 7 ++++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/util/error.cpp b/src/util/error.cpp index 18131f2cdf..5048ac5dff 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -41,6 +41,10 @@ bilingual_str TransactionErrorString(const TransactionError err) return Untranslated("Proof of blinded value is invalid"); case TransactionError::INVALID_ASSET_PROOF: return Untranslated("Proof of blinded asset is invalid"); + case TransactionError::MISSING_BLINDING_KEY: + return Untranslated("Wallet does not have necessary blinding key"); + case TransactionError::MISSING_SIDECHANNEL_DATA: + return Untranslated("A rangeproof did not encode necessary blinding data"); // no default case, so the compiler can warn about missing cases } assert(false); diff --git a/src/util/error.h b/src/util/error.h index 831d1a8df5..d2268872f4 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -35,6 +35,8 @@ enum class TransactionError { UTXOS_MISSING_BALANCE_CHECK, INVALID_VALUE_PROOF, INVALID_ASSET_PROOF, + MISSING_BLINDING_KEY, + MISSING_SIDECHANNEL_DATA, }; bilingual_str TransactionErrorString(const TransactionError error); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 08b545454a..dfb7f63110 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2859,7 +2859,7 @@ TransactionError CWallet::SignPSBT(PartiallySignedTransaction& psbtx, bool& comp CConfidentialNonce nonce; nonce.vchCommitment.insert(nonce.vchCommitment.end(), o.m_ecdh_pubkey.begin(), o.m_ecdh_pubkey.end()); - if (!UnblindConfidentialPair(blinding_key, o.m_value_commitment, o.m_asset_commitment, nonce, o.script.get(), o.m_value_rangeproof, value, value_factor, asset, asset_factor)) { + if (UnblindConfidentialPair(blinding_key, o.m_value_commitment, o.m_asset_commitment, nonce, o.script.get(), o.m_value_rangeproof, value, value_factor, asset, asset_factor)) { // These assertions are cryptographically impossible to trigger, as we // checked the proofs above, and then `UnblindConfidentialPair` checks // the extracted value/asset against the commitments. @@ -2869,10 +2869,11 @@ TransactionError CWallet::SignPSBT(PartiallySignedTransaction& psbtx, bool& comp if (!o.m_asset.IsNull()) { assert(CAsset(o.m_asset) == asset); } - return TransactionError::INVALID_ASSET_PROOF; // FIXME + } else { + return TransactionError::MISSING_SIDECHANNEL_DATA; } } else { - return TransactionError::INVALID_ASSET_PROOF; // FIXME + return TransactionError::MISSING_BLINDING_KEY; } } } From 9afcb83bafaf2a532554e4c3098cf6ff84d3334a Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 29 Sep 2021 20:26:07 +0000 Subject: [PATCH 3/8] wallet: correctly handle blinding of manually-set change addresses When the user specifies a change address manually, use the change address to obtain blinding parameters (either extract the blinding key from the address or don't blind the change). The previous behavior would assume that the change address was owned by the wallet and always generate a blinding key internally. If the user were to pass a non-wallet-owned change address, the result would be an output that could not be unblinded by its owner. --- src/wallet/wallet.cpp | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dfb7f63110..02b4805225 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -19,6 +19,7 @@ #include #include #include +#include // for GetDestinationBlindingKey and IsBlindDestination #include