Merge #18202: refactor: consolidate sendmany and sendtoaddress code

08fc6f6cfc3b06fd170452a766696d7b833113fa [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)

Pull request description:

  I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.

  Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.

  Salvaged from #18201.

ACKs for top commit:
  fjahr:
    Code review ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  jonatack:
    ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  meshcollider:
    Code review & functional test run ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa

Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
This commit is contained in:
Samuel Dobson 2020-07-12 14:31:46 +12:00 committed by Konstantin Akimov
parent fab41fd3c5
commit f86263b180
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
4 changed files with 70 additions and 87 deletions

View File

@ -0,0 +1,8 @@
Low-level RPC Changes
---------------------
- To make RPC `sendtoaddress` more consistent with `sendmany` the following error
`sendtoaddress` codes were changed from `-4` to `-6`:
- Insufficient funds
- Fee estimation failed
- Transaction has too long of a mempool chain

View File

@ -352,40 +352,55 @@ static RPCHelpMan setlabel()
};
}
void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector<CRecipient> &recipients) {
std::set<CTxDestination> destinations;
int i = 0;
for (const std::string& address: address_amounts.getKeys()) {
CTxDestination dest = DecodeDestination(address);
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + address);
}
static CTransactionRef SendMoney(CWallet* const pwallet, const CTxDestination& address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue)
if (destinations.count(dest)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + address);
}
destinations.insert(dest);
CScript script_pub_key = GetScriptForDestination(dest);
CAmount amount = AmountFromValue(address_amounts[i++]);
bool subtract_fee = false;
for (unsigned int idx = 0; idx < subtract_fee_outputs.size(); idx++) {
const UniValue& addr = subtract_fee_outputs[idx];
if (addr.get_str() == address) {
subtract_fee = true;
}
}
CRecipient recipient = {script_pub_key, amount, subtract_fee};
recipients.push_back(recipient);
}
}
UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector<CRecipient> &recipients, mapValue_t map_value)
{
CAmount curBalance = pwallet->GetBalance(0, coin_control.m_avoid_address_reuse).m_mine_trusted;
// Check amount
if (nValue <= 0)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid amount");
if (nValue > curBalance)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds");
EnsureWalletIsUnlocked(pwallet);
if (coin_control.IsUsingCoinJoin()) {
mapValue["DS"] = "1";
map_value["DS"] = "1";
}
// Parse Dash address
CScript scriptPubKey = GetScriptForDestination(address);
// Create and send the transaction
// Send
CAmount nFeeRequired = 0;
bilingual_str error;
std::vector<CRecipient> vecSend;
int nChangePosRet = -1;
CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount};
vecSend.push_back(recipient);
bilingual_str error;
CTransactionRef tx;
if (!pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, error, coin_control)) {
if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance)
error = strprintf(Untranslated("Error: This transaction requires a transaction fee of at least %s"), FormatMoney(nFeeRequired));
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
bool fCreated = pwallet->CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
if (!fCreated) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original);
}
pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */);
return tx;
pwallet->CommitTransaction(tx, std::move(map_value), {} /* orderForm */);
return tx->GetHash().GetHex();
}
static RPCHelpMan sendtoaddress()
@ -434,16 +449,6 @@ static RPCHelpMan sendtoaddress()
LOCK(pwallet->cs_wallet);
CTxDestination dest = DecodeDestination(request.params[0].get_str());
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
}
// Amount
CAmount nAmount = AmountFromValue(request.params[1]);
if (nAmount <= 0)
throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send");
// Wallet comments
mapValue_t mapValue;
if (!request.params[2].isNull() && !request.params[2].get_str().empty())
@ -471,8 +476,18 @@ static RPCHelpMan sendtoaddress()
EnsureWalletIsUnlocked(pwallet);
CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue));
return tx->GetHash().GetHex();
UniValue address_amounts(UniValue::VOBJ);
const std::string address = request.params[0].get_str();
address_amounts.pushKV(address, request.params[1]);
UniValue subtractFeeFromAmount(UniValue::VARR);
if (fSubtractFeeFromAmount) {
subtractFeeFromAmount.push_back(address);
}
std::vector<CRecipient> recipients;
ParseRecipients(address_amounts, subtractFeeFromAmount, recipients);
return SendMoney(pwallet, coin_control, recipients, mapValue);
},
};
}
@ -935,9 +950,9 @@ static RPCHelpMan sendmany()
if (!request.params[4].isNull() && !request.params[4].get_str().empty())
mapValue["comment"] = request.params[4].get_str();
UniValue subtractFeeFrom(UniValue::VARR);
UniValue subtractFeeFromAmount(UniValue::VARR);
if (!request.params[5].isNull())
subtractFeeFrom = request.params[5].get_array();
subtractFeeFromAmount = request.params[5].get_array();
// request.params[6] ("use_is") is deprecated and not used here
@ -949,53 +964,10 @@ static RPCHelpMan sendmany()
SetFeeEstimateMode(pwallet, coin_control, request.params[9], request.params[8]);
if (coin_control.IsUsingCoinJoin()) {
mapValue["DS"] = "1";
}
std::vector<CRecipient> recipients;
ParseRecipients(sendTo, subtractFeeFromAmount, recipients);
std::set<CTxDestination> destinations;
std::vector<CRecipient> vecSend;
std::vector<std::string> keys = sendTo.getKeys();
for (const std::string& name_ : keys) {
CTxDestination dest = DecodeDestination(name_);
if (!IsValidDestination(dest)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + name_);
}
if (destinations.count(dest)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + name_);
}
destinations.insert(dest);
CScript scriptPubKey = GetScriptForDestination(dest);
CAmount nAmount = AmountFromValue(sendTo[name_]);
if (nAmount <= 0)
throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send");
bool fSubtractFeeFromAmount = false;
for (unsigned int idx = 0; idx < subtractFeeFrom.size(); idx++) {
const UniValue& addr = subtractFeeFrom[idx];
if (addr.get_str() == name_)
fSubtractFeeFromAmount = true;
}
CRecipient recipient = {scriptPubKey, nAmount, fSubtractFeeFromAmount};
vecSend.push_back(recipient);
}
EnsureWalletIsUnlocked(pwallet);
// Send
CAmount nFeeRequired = 0;
int nChangePosRet = -1;
bilingual_str error;
CTransactionRef tx;
bool fCreated = pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, error, coin_control);
if (!fCreated)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original);
pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */);
return tx->GetHash().GetHex();
return SendMoney(pwallet, coin_control, recipients, std::move(mapValue));
},
};
}

View File

@ -126,7 +126,7 @@ class WalletTest(BitcoinTestFramework):
assert_raises_rpc_error(-8, "Invalid parameter, expected locked output", self.nodes[2].lockunspent, True, [unspent_0])
self.nodes[2].lockunspent(False, [unspent_0])
assert_raises_rpc_error(-8, "Invalid parameter, output already locked", self.nodes[2].lockunspent, False, [unspent_0])
assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].sendtoaddress, self.nodes[2].getnewaddress(), 200)
assert_raises_rpc_error(-6, "Insufficient funds", self.nodes[2].sendtoaddress, self.nodes[2].getnewaddress(), 200)
assert_equal([unspent_0], self.nodes[2].listlockunspent())
self.nodes[2].lockunspent(True, [unspent_0])
assert_equal(len(self.nodes[2].listlockunspent()), 0)
@ -380,6 +380,9 @@ class WalletTest(BitcoinTestFramework):
assert_equal(tx_obj['amount'], Decimal('-0.0001'))
# General checks for errors from incorrect inputs
# This will raise an exception because the amount is negative
assert_raises_rpc_error(-3, "Amount out of range", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "-1")
# This will raise an exception because the amount type is wrong
assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "1f-4")
@ -608,7 +611,7 @@ class WalletTest(BitcoinTestFramework):
node0_balance = self.nodes[0].getbalance()
# With walletrejectlongchains we will not create the tx and store it in our wallet.
assert_raises_rpc_error(-4, "Transaction has too long of a mempool chain", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01'))
assert_raises_rpc_error(-6, "Transaction has too long of a mempool chain", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01'))
# Verify nothing new in wallet
assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999)))

View File

@ -24,7 +24,7 @@ class WalletRBFTest(BitcoinTestFramework):
# test sending a tx with disabled fallback fee (must fail)
self.restart_node(0, extra_args=["-fallbackfee=0"])
assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1))
assert_raises_rpc_error(-6, "Fee estimation failed", lambda: self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1))
assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].fundrawtransaction(self.nodes[0].createrawtransaction([], {self.nodes[0].getnewaddress(): 1})))
assert_raises_rpc_error(-6, "Fee estimation failed", lambda: self.nodes[0].sendmany("", {self.nodes[0].getnewaddress(): 1}))