Skip to content

Commit

Permalink
Merge 526e802 into merged_master (Elements PR #755)
Browse files Browse the repository at this point in the history 
This Elements PR includes components of Core PR #17211, which since the
refactors to use effective value landed, no longer provides the right
error message when a user provides an unowned input from a wallet tx.
See bitcoin/bitcoin#17211 (review)

This breaks a functional test which was included in this PR, but which
conveniently has been changed in the current version of the Core PR. I
fixed the behavior (commented, in SelectCoins) rather than updating the
test to the most recent version.
  • Loading branch information
@apoelstra
apoelstra committed Nov 14, 2020
2 parents 5a48ca1 + 526e802 commit c7bf5ba
Show file tree
Hide file tree
Showing 11 changed files with 362 additions and 68 deletions.
2 changes: 2 additions & 0 deletions src/rpc/client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,13 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "combinerawtransaction", 0, "txs" },
{ "fundrawtransaction", 1, "options" },
{ "fundrawtransaction", 2, "iswitness" },
{ "fundrawtransaction", 3, "solving_data" },
{ "walletcreatefundedpsbt", 0, "inputs" },
{ "walletcreatefundedpsbt", 1, "outputs" },
{ "walletcreatefundedpsbt", 2, "locktime" },
{ "walletcreatefundedpsbt", 3, "options" },
{ "walletcreatefundedpsbt", 4, "bip32derivs" },
{ "walletcreatefundedpsbt", 5, "solving_data" },
{ "walletprocesspsbt", 1, "sign" },
{ "walletprocesspsbt", 3, "bip32derivs" },
{ "walletfillpsbtdata", 1, "bip32derivs" },
Expand Down
3 changes: 1 addition & 2 deletions src/script/signingprovider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@

#include <key.h>
#include <pubkey.h>
#include <script/keyorigin.h>
#include <script/script.h>
#include <script/standard.h>
#include <sync.h>

struct KeyOriginInfo;

/** An interface to be implemented by keystores that support signing. */
class SigningProvider
{
Expand Down
2 changes: 2 additions & 0 deletions src/wallet/coincontrol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ void CCoinControl::SetNull()
m_confirm_target.reset();
m_signal_bip125_rbf.reset();
m_fee_mode = FeeEstimateMode::UNSET;
m_external_txouts.clear();
m_external_provider = FlatSigningProvider();
m_min_depth = DEFAULT_MIN_DEPTH;
m_max_depth = DEFAULT_MAX_DEPTH;
}
Expand Down
36 changes: 36 additions & 0 deletions src/wallet/coincontrol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
#define BITCOIN_WALLET_COINCONTROL_H

#include <asset.h>
#include <chainparams.h>
#include <optional.h>
#include <outputtype.h>
#include <policy/feerate.h>
#include <policy/fees.h>
#include <primitives/bitcoin/transaction.h>
#include <primitives/transaction.h>
#include <script/standard.h>

Expand Down Expand Up @@ -45,6 +47,8 @@ class CCoinControl
bool m_avoid_address_reuse;
//! Fee estimation mode to control arguments to estimateSmartFee
FeeEstimateMode m_fee_mode;
//! SigningProvider that has pubkeys and scripts to do spend size estimation for external inputs
FlatSigningProvider m_external_provider;
//! Minimum chain depth value for coin availability
int m_min_depth = DEFAULT_MIN_DEPTH;
//! Maximum chain depth value for coin availability
Expand All @@ -67,11 +71,42 @@ class CCoinControl
return (setSelected.count(output) > 0);
}

bool IsExternalSelected(const COutPoint& output) const
{
return (m_external_txouts.count(output) > 0);
}

bool GetExternalOutput(const COutPoint& outpoint, CTxOut& txout) const
{
const auto ext_it = m_external_txouts.find(outpoint);
if (ext_it == m_external_txouts.end()) {
return false;
}
txout = ext_it->second;
return true;
}

void Select(const COutPoint& output)
{
setSelected.insert(output);
}

void SelectExternal(const COutPoint& outpoint, const CTxOut& txout)
{
setSelected.insert(outpoint);
m_external_txouts.emplace(outpoint, txout);
}

void Select(const COutPoint& outpoint, const Sidechain::Bitcoin::CTxOut& txout_in)
{
setSelected.insert(outpoint);
CTxOut txout;
txout.scriptPubKey = txout_in.scriptPubKey;
txout.nValue.SetToAmount(txout_in.nValue);
txout.nAsset.SetToAsset(Params().GetConsensus().pegged_asset);
m_external_txouts.emplace(outpoint, txout);
}

void UnSelect(const COutPoint& output)
{
setSelected.erase(output);
Expand All @@ -89,6 +124,7 @@ class CCoinControl

private:
std::set<COutPoint> setSelected;
std::map<COutPoint, CTxOut> m_external_txouts;
};

#endif // BITCOIN_WALLET_COINCONTROL_H
37 changes: 37 additions & 0 deletions src/wallet/coinselection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#define BITCOIN_WALLET_COINSELECTION_H

#include <amount.h>
#include <chainparams.h>
#include <primitives/transaction.h>
#include <primitives/bitcoin/transaction.h>
#include <random.h>

//! target minimum change amount
Expand All @@ -26,6 +28,41 @@ class CInputCoin {
m_input_bytes = input_bytes;
}

CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in)
{
outpoint = outpoint_in;
txout = txout_in;
if (txout.nValue.IsExplicit()) {
effective_value = txout_in.nValue.GetAmount();
value = txout.nValue.GetAmount();
asset = txout.nAsset.GetAsset();
} else {
effective_value = 0;
}
}

CInputCoin(const COutPoint& outpoint_in, const CTxOut& txout_in, int input_bytes) : CInputCoin(outpoint_in, txout_in)
{
m_input_bytes = input_bytes;
}

CInputCoin(const COutPoint& outpoint_in, const Sidechain::Bitcoin::CTxOut& txout_in)
{
outpoint = outpoint_in;
effective_value = txout_in.nValue;
txout.SetNull();
txout.scriptPubKey = txout_in.scriptPubKey;
txout.nValue.SetToAmount(txout_in.nValue);
txout.nAsset.SetToAsset(Params().GetConsensus().pegged_asset);
asset = Params().GetConsensus().pegged_asset;
value = txout_in.nValue;
}

CInputCoin(const COutPoint& outpoint_in, const Sidechain::Bitcoin::CTxOut& txout_in, int input_bytes) : CInputCoin(outpoint_in, txout_in)
{
m_input_bytes = input_bytes;
}

COutPoint outpoint;
CTxOut txout;
CAmount effective_value;
Expand Down
160 changes: 154 additions & 6 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3275,7 +3275,7 @@ static UniValue listunspent(const JSONRPCRequest& request)
return results;
}

void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options)
void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, const UniValue& solving_data)
{
// Make sure the results are valid at least up to the most recent block
// the user could have gotten from another RPC command prior to now
Expand Down Expand Up @@ -3395,6 +3395,41 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
coinControl.fAllowWatchOnly = ParseIncludeWatchonly(NullUniValue, *pwallet);
}

if (!solving_data.isNull()) {
if (solving_data.exists("pubkeys")) {
UniValue pubkey_strs = solving_data["pubkeys"].get_array();
for (unsigned int i = 0; i < pubkey_strs.size(); ++i) {
std::vector<unsigned char> data(ParseHex(pubkey_strs[i].get_str()));
CPubKey pubkey(data.begin(), data.end());
if (!pubkey.IsFullyValid()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s is not a valid public key", pubkey_strs[i].get_str()));
}
coinControl.m_external_provider.pubkeys.emplace(pubkey.GetID(), pubkey);
// Add witnes script for pubkeys
CScript wit_script = GetScriptForDestination(WitnessV0KeyHash(pubkey.GetID()));
coinControl.m_external_provider.scripts.emplace(CScriptID(wit_script), wit_script);
}
}

if (solving_data.exists("scripts")) {
UniValue script_strs = solving_data["scripts"].get_array();
for (unsigned int i = 0; i < script_strs.size(); ++i) {
CScript script = ParseScript(script_strs[i].get_str());
coinControl.m_external_provider.scripts.emplace(CScriptID(script), script);
}
}

if (solving_data.exists("descriptors")) {
UniValue desc_strs = solving_data["descriptors"].get_array();
for (unsigned int i = 0; i < desc_strs.size(); ++i) {
FlatSigningProvider desc_out;
std::string error;
std::unique_ptr<Descriptor> desc = Parse(desc_strs[i].get_str(), desc_out, error, true);
coinControl.m_external_provider = Merge(coinControl.m_external_provider, desc_out);
}
}
}

if (tx.vout.size() == 0)
throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output");

Expand All @@ -3412,6 +3447,42 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
setSubtractFeeFromOutputs.insert(pos);
}

// Check any existing inputs for peg-in data and add to external txouts if so
// Fetch specified UTXOs from the UTXO set
const auto& fedpegscripts = GetValidFedpegScripts(::ChainActive().Tip(), Params().GetConsensus(), true /* nextblock_validation */);
std::map<COutPoint, Coin> coins;
for (unsigned int i = 0; i < tx.vin.size(); ++i ) {
const CTxIn& txin = tx.vin[i];
coins[txin.prevout]; // Create empty map entry keyed by prevout.
if (txin.m_is_pegin) {
std::string err;
if (tx.witness.vtxinwit.size() != tx.vin.size() || !IsValidPeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, fedpegscripts, txin.prevout, err, false)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Transaction contains invalid peg-in input: %s", err));
}
CScriptWitness& pegin_witness = tx.witness.vtxinwit[i].m_pegin_witness;
CTxOut txout = GetPeginOutputFromWitness(pegin_witness);
coinControl.SelectExternal(txin.prevout, txout);
}
}
CCoinsView viewDummy;
CCoinsViewCache view(&viewDummy);
{
LOCK2(cs_main, mempool.cs);
CCoinsViewCache& chain_view = ::ChainstateActive().CoinsTip();
CCoinsViewMemPool mempool_view(&chain_view, mempool);
for (auto& coin : coins) {
if (!mempool_view.GetCoin(coin.first, coin.second)) {
// Either the coin is not in the CCoinsViewCache or is spent. Clear it.
coin.second.Clear();
}
}
}
for (const auto& coin : coins) {
if (!coin.second.out.IsNull()) {
coinControl.SelectExternal(coin.first, coin.second.out);
}
}

std::string strFailReason;

if (!pwallet->FundTransaction(tx, fee_out, change_position, strFailReason, lockUnspents, setSubtractFeeFromOutputs, coinControl)) {
Expand Down Expand Up @@ -3476,6 +3547,25 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request)
"This boolean should reflect whether the transaction has inputs\n"
"(e.g. fully valid, or on-chain transactions), if known by the caller."
},
{"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature. Used for fee estimation during coin selection.\n",
{
{"pubkeys", RPCArg::Type::ARR, /* default */ "empty array", "A json array of public keys.\n",
{
{"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"},
},
},
{"scripts", RPCArg::Type::ARR, /* default */ "empty array", "A json array of scripts.\n",
{
{"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"},
},
},
{"descriptors", RPCArg::Type::ARR, /* default */ "empty array", "A json array of descriptors.\n",
{
{"descriptor", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A descriptor"},
},
}
}
},
},
RPCResult{
"{\n"
Expand Down Expand Up @@ -3508,7 +3598,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request)

CAmount fee;
int change_position;
FundTransaction(pwallet, tx, fee, change_position, request.params[1]);
FundTransaction(pwallet, tx, fee, change_position, request.params[1], request.params[3]);

UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(CTransaction(tx)));
Expand Down Expand Up @@ -4658,6 +4748,9 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
{"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"},
{"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"},
{"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"},
{"pegin_bitcoin_tx", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The raw bitcoin transaction (in hex) depositing bitcoin to the mainchain_address generated by getpeginaddress"},
{"pegin_txout_proof", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A rawtxoutproof (in hex) generated by the mainchain daemon's `gettxoutproof` containing a proof of only bitcoin_tx"},
{"pegin_claim_script", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The witness program generated by getpeginaddress."},
},
},
},
Expand Down Expand Up @@ -4706,6 +4799,25 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
},
"options"},
{"bip32derivs", RPCArg::Type::BOOL, /* default */ "false", "If true, includes the BIP 32 derivation paths for public keys if we know them"},
{"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature. Used for fee estimation during coin selection.\n",
{
{"pubkeys", RPCArg::Type::ARR, /* default */ "empty array", "A json array of public keys.\n",
{
{"pubkey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A public key"},
},
},
{"scripts", RPCArg::Type::ARR, /* default */ "empty array", "A json array of scripts.\n",
{
{"script", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A script"},
},
},
{"descriptors", RPCArg::Type::ARR, /* default */ "empty array", "A json array of descriptors.\n",
{
{"descriptor", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A descriptor"},
},
}
}
},
},
RPCResult{
"{\n"
Expand Down Expand Up @@ -4740,8 +4852,8 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
// It's hard to control the behavior of FundTransaction, so we will wait
// until after it's done, then extract the blinding keys from the output
// nonces.
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf, NullUniValue /* CA: assets_in */, nullptr /* output_pubkeys_out */, false /* allow_peg_in */);
FundTransaction(pwallet, rawTx, fee, change_position, request.params[3]);
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf, NullUniValue /* CA: assets_in */, nullptr /* output_pubkeys_out */, true /* allow_peg_in */);
FundTransaction(pwallet, rawTx, fee, change_position, request.params[3], request.params[5]);

// Make a blank psbt
PartiallySignedTransaction psbtx(rawTx);
Expand All @@ -4761,6 +4873,42 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
throw JSONRPCTransactionError(err);
}

// Add peg-in stuff if it's there
for (unsigned int i = 0; i < rawTx.vin.size(); ++i) {
if (psbtx.tx->vin[i].m_is_pegin) {
CScriptWitness& pegin_witness = psbtx.tx->witness.vtxinwit[i].m_pegin_witness;
CAmount val;
VectorReader vr_val(SER_NETWORK, PROTOCOL_VERSION, pegin_witness.stack[0], 0);
vr_val >> val;
psbtx.inputs[i].value = val;
VectorReader vr_asset(SER_NETWORK, PROTOCOL_VERSION, pegin_witness.stack[1], 0);
vr_asset >> psbtx.inputs[i].asset;
VectorReader vr_genesis(SER_NETWORK, PROTOCOL_VERSION, pegin_witness.stack[2], 0);
vr_genesis >> psbtx.inputs[i].genesis_hash;
psbtx.inputs[i].claim_script.assign(pegin_witness.stack[3].begin(), pegin_witness.stack[3].end());

VectorReader vr_tx(SER_NETWORK, PROTOCOL_VERSION, pegin_witness.stack[4], 0);
VectorReader vr_proof(SER_NETWORK, PROTOCOL_VERSION, pegin_witness.stack[5], 0);
if (Params().GetConsensus().ParentChainHasPow()) {
Sidechain::Bitcoin::CTransactionRef tx_btc;
vr_tx >> tx_btc;
psbtx.inputs[i].peg_in_tx = tx_btc;
Sidechain::Bitcoin::CMerkleBlock tx_proof;
vr_proof >> tx_proof;
psbtx.inputs[i].txout_proof = tx_proof;
} else {
CTransactionRef tx_btc;
vr_tx >> tx_btc;
psbtx.inputs[i].peg_in_tx = tx_btc;
CMerkleBlock tx_proof;
vr_proof >> tx_proof;
psbtx.inputs[i].txout_proof = tx_proof;
}
pegin_witness.SetNull();
psbtx.tx->vin[i].m_is_pegin = false;
}
}

// Serialize the PSBT
CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
ssTx << psbtx;
Expand Down Expand Up @@ -6641,7 +6789,7 @@ UniValue getwalletpakinfo(const JSONRPCRequest& request);
static const CRPCCommand commands[] =
{ // category name actor (function) argNames
// --------------------- ------------------------ ----------------------- ----------
{ "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options","iswitness"} },
{ "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options","iswitness","solving_data"} },
{ "wallet", "abandontransaction", &abandontransaction, {"txid"} },
{ "wallet", "abortrescan", &abortrescan, {} },
{ "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label","address_type"} },
Expand Down Expand Up @@ -6692,7 +6840,7 @@ static const CRPCCommand commands[] =
{ "wallet", "signmessage", &signmessage, {"address","message"} },
{ "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} },
{ "wallet", "unloadwallet", &unloadwallet, {"wallet_name"} },
{ "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} },
{ "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs","solving_data"} },
{ "wallet", "walletlock", &walletlock, {} },
{ "wallet", "walletpassphrase", &walletpassphrase, {"passphrase","timeout"} },
{ "wallet", "walletpassphrasechange", &walletpassphrasechange, {"oldpassphrase","newpassphrase"} },
Expand Down
Loading

0 comments on commit c7bf5ba

Please sign in to comment.