Merge #16128: Delete error-prone CScript constructor only used with FindAndDelete

e1a55690e6 Delete error-prone CScript constructor (Gregory Sanders)

Pull request description:

  The behavior of this constructor is not the expected behavior compared to the other constructors which directly interpret the vector as a CScript, rather than serialize it into a new CScript. It has only four uses in the entire codebase. Delete this constructor and replace its four uses with the more clear serialization construction.

ACKs for top commit:
  Empact:
    ACK e1a55690e6
  sipa:
    Concept and code review ACK e1a55690e6, but I'd like to make sure we have tests covering the FindAndDelete usage.

Tree-SHA512: b6721e343c867ca401a80ec87c25939d7f1fc798f3bf7e5feb0ea6f8280eecb6bd65afc8286912c76ff8119ccea50ad7726b1a4137cae70c9d4fed7d960e10d3
This commit is contained in:
Wladimir J. van der Laan 2019-07-08 20:29:19 +02:00
commit c799976c86
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
3 changed files with 9 additions and 5 deletions

View file

@ -418,7 +418,8 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
if (amount > 0) if (amount > 0)
{ {
CTxOut txout(amount, static_cast<CScript>(std::vector<unsigned char>(24, 0))); // Assumes a p2pkh script size
CTxOut txout(amount, CScript() << std::vector<unsigned char>(24, 0));
txDummy.vout.push_back(txout); txDummy.vout.push_back(txout);
fDust |= IsDust(txout, model->node().getDustRelayFee()); fDust |= IsDust(txout, model->node().getDustRelayFee());
} }
@ -509,7 +510,8 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
// Never create dust outputs; if we would, just add the dust to the fee. // Never create dust outputs; if we would, just add the dust to the fee.
if (nChange > 0 && nChange < MIN_CHANGE) if (nChange > 0 && nChange < MIN_CHANGE)
{ {
CTxOut txout(nChange, static_cast<CScript>(std::vector<unsigned char>(24, 0))); // Assumes a p2pkh script size
CTxOut txout(nChange, CScript() << std::vector<unsigned char>(24, 0));
if (IsDust(txout, model->node().getDustRelayFee())) if (IsDust(txout, model->node().getDustRelayFee()))
{ {
nPayFee += nChange; nPayFee += nChange;

View file

@ -926,7 +926,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
// Drop the signature in pre-segwit scripts but not segwit scripts // Drop the signature in pre-segwit scripts but not segwit scripts
if (sigversion == SigVersion::BASE) { if (sigversion == SigVersion::BASE) {
int found = FindAndDelete(scriptCode, CScript(vchSig)); int found = FindAndDelete(scriptCode, CScript() << vchSig);
if (found > 0 && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE)) if (found > 0 && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE))
return set_error(serror, SCRIPT_ERR_SIG_FINDANDDELETE); return set_error(serror, SCRIPT_ERR_SIG_FINDANDDELETE);
} }
@ -992,7 +992,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
{ {
valtype& vchSig = stacktop(-isig-k); valtype& vchSig = stacktop(-isig-k);
if (sigversion == SigVersion::BASE) { if (sigversion == SigVersion::BASE) {
int found = FindAndDelete(scriptCode, CScript(vchSig)); int found = FindAndDelete(scriptCode, CScript() << vchSig);
if (found > 0 && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE)) if (found > 0 && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE))
return set_error(serror, SCRIPT_ERR_SIG_FINDANDDELETE); return set_error(serror, SCRIPT_ERR_SIG_FINDANDDELETE);
} }

View file

@ -437,7 +437,9 @@ public:
explicit CScript(opcodetype b) { operator<<(b); } explicit CScript(opcodetype b) { operator<<(b); }
explicit CScript(const CScriptNum& b) { operator<<(b); } explicit CScript(const CScriptNum& b) { operator<<(b); }
explicit CScript(const std::vector<unsigned char>& b) { operator<<(b); } // delete non-existent constructor to defend against future introduction
// e.g. via prevector
explicit CScript(const std::vector<unsigned char>& b) = delete;
CScript& operator<<(int64_t b) { return push_int64(b); } CScript& operator<<(int64_t b) { return push_int64(b); }