Wallet: Do not perform ECDSA in the fee calculation inner loop.

Performing signing in the inner loop has terrible performance
 when many passes through are needed to complete the selection.

Signing before the algorithm is complete also gets in the way
 of correctly setting the fee (e.g. preventing over-payment when
 the fee required goes down on the final selection.)

Use of the dummy might overpay on the signatures by a couple bytes
 in uncommon cases where the signatures' DER encoding is smaller
 than the dummy: Who cares?
This commit is contained in:
Gregory Maxwell 2017-01-04 08:51:14 +00:00
parent 649cf5fe89
commit b3d7b1cbe7

View File

@ -2245,7 +2245,7 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
CAmount nValue = 0;
int nChangePosRequest = nChangePosInOut;
unsigned int nSubtractFeeFromAmount = 0;
BOOST_FOREACH (const CRecipient& recipient, vecSend)
for (const auto& recipient : vecSend)
{
if (nValue < 0 || recipient.nAmount < 0)
{
@ -2300,6 +2300,7 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
assert(txNew.nLockTime < LOCKTIME_THRESHOLD);
{
set<pair<const CWalletTx*,unsigned int> > setCoins;
LOCK2(cs_main, cs_wallet);
{
std::vector<COutput> vAvailableCoins;
@ -2320,7 +2321,7 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
nValueToSelect += nFeeRet;
double dPriority = 0;
// vouts to the payees
BOOST_FOREACH (const CRecipient& recipient, vecSend)
for (const auto& recipient : vecSend)
{
CTxOut txout(recipient.nAmount, recipient.scriptPubKey);
@ -2352,14 +2353,14 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
}
// Choose coins to use
set<pair<const CWalletTx*,unsigned int> > setCoins;
CAmount nValueIn = 0;
setCoins.clear();
if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coinControl))
{
strFailReason = _("Insufficient funds");
return false;
}
BOOST_FOREACH(PAIRTYPE(const CWalletTx*, unsigned int) pcoin, setCoins)
for (const auto& pcoin : setCoins)
{
CAmount nCredit = pcoin.first->tx->vout[pcoin.second].nValue;
//The coin age after the next block (depth+1) is used instead of the current,
@ -2470,24 +2471,18 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
// to avoid conflicting with other possible uses of nSequence,
// and in the spirit of "smallest posible change from prior
// behavior."
BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins)
for (const auto& coin : setCoins)
txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(),
std::numeric_limits<unsigned int>::max() - (fWalletRbf ? 2 : 1)));
// Sign
// Fill in dummy signatures for fee calculation.
int nIn = 0;
CTransaction txNewConst(txNew);
BOOST_FOREACH(const PAIRTYPE(const CWalletTx*,unsigned int)& coin, setCoins)
for (const auto& coin : setCoins)
{
bool signSuccess;
const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey;
SignatureData sigdata;
if (sign)
signSuccess = ProduceSignature(TransactionSignatureCreator(this, &txNewConst, nIn, coin.first->tx->vout[coin.second].nValue, SIGHASH_ALL), scriptPubKey, sigdata);
else
signSuccess = ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata);
if (!signSuccess)
if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata))
{
strFailReason = _("Signing transaction failed");
return false;
@ -2500,26 +2495,15 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
unsigned int nBytes = GetVirtualTransactionSize(txNew);
// Remove scriptSigs if we used dummy signatures for fee calculation
if (!sign) {
BOOST_FOREACH (CTxIn& vin, txNew.vin) {
vin.scriptSig = CScript();
vin.scriptWitness.SetNull();
}
CTransaction txNewConst(txNew);
dPriority = txNewConst.ComputePriority(dPriority, nBytes);
// Remove scriptSigs to eliminate the fee calculation dummy signatures
for (auto& vin : txNew.vin) {
vin.scriptSig = CScript();
vin.scriptWitness.SetNull();
}
// Embed the constructed transaction data in wtxNew.
wtxNew.SetTx(MakeTransactionRef(std::move(txNew)));
// Limit size
if (GetTransactionWeight(wtxNew) >= MAX_STANDARD_TX_WEIGHT)
{
strFailReason = _("Transaction too large");
return false;
}
dPriority = wtxNew.tx->ComputePriority(dPriority, nBytes);
// Allow to override the default confirmation target over the CoinControl instance
int currentConfirmationTarget = nTxConfirmTarget;
if (coinControl && coinControl->nConfirmTarget > 0)
@ -2558,6 +2542,37 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
continue;
}
}
if (sign)
{
CTransaction txNewConst(txNew);
int nIn = 0;
for (const auto& coin : setCoins)
{
const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey;
SignatureData sigdata;
if (!ProduceSignature(TransactionSignatureCreator(this, &txNewConst, nIn, coin.first->tx->vout[coin.second].nValue, SIGHASH_ALL), scriptPubKey, sigdata))
{
strFailReason = _("Signing transaction failed");
return false;
} else {
UpdateTransaction(txNew, nIn, sigdata);
}
nIn++;
}
}
// Embed the constructed transaction data in wtxNew.
wtxNew.SetTx(MakeTransactionRef(std::move(txNew)));
// Limit size
if (GetTransactionWeight(wtxNew) >= MAX_STANDARD_TX_WEIGHT)
{
strFailReason = _("Transaction too large");
return false;
}
}
if (GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {