sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-27 13:03:17 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #9640: Bumpfee: bugfixes for error handling and feerate calculation

Browse Source 
9522b53 rpc: bumpfee: handle errors more gracefully (Suhas Daftuar)
f626594 rpc: bumpfee: use correct maximum signed tx size for fee calculation (Suhas Daftuar)
d625b90 wallet: Refactor dummy signature signing for reusability (Suhas Daftuar)
This commit is contained in:
Wladimir J. van der Laan 2017-02-01 08:41:21 +01:00
commit 7bfb77045c
No known key found for this signature in database
GPG Key ID: 74810B012346C9A6
3 changed files with 77 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/wallet/rpcwallet.cpp
View File

@ -2664,6 +2664,33 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
return result;
}
// Calculate the size of the transaction assuming all signatures are max size
// Use DummySignatureCreator, which inserts 72 byte signatures everywhere.
// TODO: re-use this in CWallet::CreateTransaction (right now
// CreateTransaction uses the constructed dummy-signed tx to do a priority
// calculation, but we should be able to refactor after priority is removed).
// NOTE: this requires that all inputs must be in mapWallet (eg the tx should
// be IsAllFromMe).
int64_t CalculateMaximumSignedTxSize(const CTransaction &tx)
{
CMutableTransaction txNew(tx);
std::vector<pair<CWalletTx *, unsigned int>> vCoins;
// Look up the inputs. We should have already checked that this transaction
// IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our
// wallet, with a valid index into the vout array.
for (auto& input : tx.vin) {
const auto mi = pwalletMain->mapWallet.find(input.prevout.hash);
assert(mi != pwalletMain->mapWallet.end() && input.prevout.n < mi->second.tx->vout.size());
vCoins.emplace_back(make_pair(&(mi->second), input.prevout.n));
}
if (!pwalletMain->DummySignTx(txNew, vCoins)) {
// This should never happen, because IsAllFromMe(ISMINE_SPENDABLE)
// implies that we can sign for every input.
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction contains inputs that cannot be signed");
}
return GetVirtualTransactionSize(txNew);
}
UniValue bumpfee(const JSONRPCRequest& request)
{
if (!EnsureWalletIsAvailable(request.fHelp)) {
@ -2706,6 +2733,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
" \"txid\": \"value\", (string) The id of the new transaction\n"
" \"origfee\": n, (numeric) Fee of the replaced transaction\n"
" \"fee\": n, (numeric) Fee of the new transaction\n"
" \"errors\": [ str... ] (json array of strings) Errors encountered during processing (may be empty)\n"
"}\n"
"\nExamples:\n"
"\nBump the fee, get the new transaction\'s txid\n" +
@ -2769,9 +2797,9 @@ UniValue bumpfee(const JSONRPCRequest& request)
throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output");
}
// signature sizes can vary by a byte, so add 1 for each input when calculating the new fee
// Calculate the expected size of the new transaction.
int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
const int64_t maxNewTxSize = txSize + wtx.tx->vin.size();
const int64_t maxNewTxSize = CalculateMaximumSignedTxSize(*wtx.tx);
// optional parameters
bool specifiedConfirmTarget = false;
@ -2845,8 +2873,12 @@ UniValue bumpfee(const JSONRPCRequest& request)
nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize);
// New fee rate must be at least old rate + minimum incremental relay rate
if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + walletIncrementalRelayFee.GetFeePerK()) {
nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + walletIncrementalRelayFee.GetFeePerK());
// walletIncrementalRelayFee.GetFeePerK() should be exact, because it's initialized
// in that unit (fee per kb).
// However, nOldFeeRate is a calculated value from the tx fee/size, so
// add 1 satoshi to the result, because it may have been rounded down.
if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + 1 + walletIncrementalRelayFee.GetFeePerK()) {
nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + 1 + walletIncrementalRelayFee.GetFeePerK());
nNewFee = nNewFeeRate.GetFee(maxNewTxSize);
}
}
@ -2914,23 +2946,32 @@ UniValue bumpfee(const JSONRPCRequest& request)
CWalletTx wtxBumped(pwalletMain, MakeTransactionRef(std::move(tx)));
wtxBumped.mapValue["replaces_txid"] = hash.ToString();
CValidationState state;
if (!pwalletMain->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state) || !state.IsValid()) {
if (!pwalletMain->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) {
// NOTE: CommitTransaction never returns false, so this should never happen.
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason()));
}
UniValue vErrors(UniValue::VARR);
if (state.IsInvalid()) {
// This can happen if the mempool rejected the transaction. Report
// what happened in the "errors" response.
vErrors.push_back(strprintf("Error: The transaction was rejected: %s", FormatStateMessage(state)));
}
// mark the original tx as bumped
if (!pwalletMain->MarkReplaced(wtx.GetHash(), wtxBumped.GetHash())) {
// TODO: see if JSON-RPC has a standard way of returning a response
// along with an exception. It would be good to return information about
// wtxBumped to the caller even if marking the original transaction
// replaced does not succeed for some reason.
throw JSONRPCError(RPC_WALLET_ERROR, "Error: Created new bumpfee transaction but could not mark the original transaction as replaced.");
vErrors.push_back("Error: Created new bumpfee transaction but could not mark the original transaction as replaced.");
}
UniValue result(UniValue::VOBJ);
result.push_back(Pair("txid", wtxBumped.GetHash().GetHex()));
result.push_back(Pair("origfee", ValueFromAmount(nOldFee)));
result.push_back(Pair("fee", ValueFromAmount(nNewFee)));
result.push_back(Pair("errors", vErrors));
return result;
}

14
src/wallet/wallet.cpp
View File

@ -2583,21 +2583,9 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
std::numeric_limits<unsigned int>::max() - (fWalletRbf ? 2 : 1)));
// Fill in dummy signatures for fee calculation.
int nIn = 0;
for (const auto& coin : setCoins)
{
const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey;
SignatureData sigdata;
if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata))
{
if (!DummySignTx(txNew, setCoins)) {
strFailReason = _("Signing transaction failed");
return false;
} else {
UpdateTransaction(txNew, nIn, sigdata);
}
nIn++;
}
unsigned int nBytes = GetVirtualTransactionSize(txNew);

27
src/wallet/wallet.h
View File

@ -13,6 +13,7 @@
#include "utilstrencodings.h"
#include "validationinterface.h"
#include "script/ismine.h"
#include "script/sign.h"
#include "wallet/crypter.h"
#include "wallet/walletdb.h"
#include "wallet/rpcwallet.h"
@ -796,6 +797,8 @@ public:
void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries);
bool AddAccountingEntry(const CAccountingEntry&);
bool AddAccountingEntry(const CAccountingEntry&, CWalletDB *pwalletdb);
template <typename ContainerType>
bool DummySignTx(CMutableTransaction &txNew, const ContainerType &coins);
static CFeeRate minTxFee;
static CFeeRate fallbackFee;
@ -1028,4 +1031,28 @@ public:
}
};
// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes)
// ContainerType is meant to hold pair<CWalletTx *, int>, and be iterable
// so that each entry corresponds to each vIn, in order.
template <typename ContainerType>
bool CWallet::DummySignTx(CMutableTransaction &txNew, const ContainerType &coins)
{
// Fill in dummy signatures for fee calculation.
int nIn = 0;
for (const auto& coin : coins)
{
const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey;
SignatureData sigdata;
if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata))
{
return false;
} else {
UpdateTransaction(txNew, nIn, sigdata);
}
nIn++;
}
return true;
}
#endif // BITCOIN_WALLET_WALLET_H