Skip to content

Commit

Permalink
Merge a993a7c into merged_master (Elements PR #960)
Browse files Browse the repository at this point in the history
Several conflicts in the C++ code related to the new `flags` parameter
to `CheckSignature` and the corresponding function being renamed upstream
to `CheckSignatureECDSA`.

Several conflicts in the test harness as Steven sorta pulled the new
upstream ECKey module into the Python code, and the actual upstream
code was slightly different. Also needed to update the feature_taproot
code to always use the non-RANGEPROOF sighash since dynafed is not
enabled in the Taproot test.

Also had to pull the `set_wif` method out of `ECKey` and inline it because
otherwise it triggers a "circular inclusion" error between script.py (which
would pull in `base58_to_bytes` from address.py) and address.py (which now
pulls in some taproot EC related stuff from script.py).

Noticed that #960 does not test the "sighash rangeproof flag set but no
witnesses" case.
  • Loading branch information
apoelstra committed Mar 25, 2021
2 parents 81e75ab + a993a7c commit 22cf380
Show file tree
Hide file tree
Showing 18 changed files with 410 additions and 65 deletions.
2 changes: 1 addition & 1 deletion src/bench/verify_script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static void VerifyScriptBench(benchmark::Bench& bench)
txSpend.witness.vtxinwit.resize(1);
CScriptWitness& witness = txSpend.witness.vtxinwit[0].scriptWitness;
witness.stack.emplace_back();
key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0), witness.stack.back());
key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SigVersion::WITNESS_V0, 0), witness.stack.back());
witness.stack.back().push_back(static_cast<unsigned char>(SIGHASH_ALL));
witness.stack.push_back(ToByteVector(pubkey));

Expand Down
2 changes: 1 addition & 1 deletion src/qt/qrimagewidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ bool QRImageWidget::setQR(const QString& data, const QString& text)

// Elements: Hack to get QR address to print right
const size_t MORE_WIDTH = 80;

const int qr_image_size = QR_IMAGE_SIZE + MORE_WIDTH + (text.isEmpty() ? 0 : 2 * QR_IMAGE_MARGIN);
QImage qrAddrImage(qr_image_size, qr_image_size, QImage::Format_RGB32);
qrAddrImage.fill(0xffffff);
Expand Down
4 changes: 2 additions & 2 deletions src/script/generic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class SimpleSignatureChecker : public BaseSignatureChecker
bool sighash_byte;

SimpleSignatureChecker(const uint256& hashIn, bool sighash_byte_in) : hash(hashIn), sighash_byte(sighash_byte_in) {};
bool CheckECDSASignature(const std::vector<unsigned char>& vchSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override
bool CheckECDSASignature(const std::vector<unsigned char>& vchSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const override
{
std::vector<unsigned char> vchSigCopy(vchSig);
CPubKey pubkey(vchPubKey);
Expand Down Expand Up @@ -48,7 +48,7 @@ class SimpleSignatureCreator : public BaseSignatureCreator
public:
SimpleSignatureCreator(const uint256& hashIn, bool sighash_byte_in) : checker(hashIn, sighash_byte_in), sighash_byte(sighash_byte_in) {};
const BaseSignatureChecker& Checker() const override { return checker; }
bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override
bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const override
{
CKey key;
if (!provider.GetKey(keyid, key))
Expand Down
80 changes: 69 additions & 11 deletions src/script/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,17 @@ bool static IsLowDERSignature(const valtype &vchSig, ScriptError* serror) {
return true;
}

bool static IsDefinedHashtypeSignature(const valtype &vchSig) {
bool static IsDefinedHashtypeSignature(const valtype &vchSig, unsigned int flags) {
if (vchSig.size() == 0) {
return false;
}
unsigned char nHashType = vchSig[vchSig.size() - 1] & (~(SIGHASH_ANYONECANPAY));

// ELEMENTS: Only allow SIGHASH_RANGEPROOF if the flag is set (after dynafed activation).
if ((flags & SCRIPT_SIGHASH_RANGEPROOF) == SCRIPT_SIGHASH_RANGEPROOF) {
nHashType = nHashType & (~(SIGHASH_RANGEPROOF));
}

if (nHashType < SIGHASH_ALL || nHashType > SIGHASH_SINGLE)
return false;

Expand All @@ -216,7 +222,7 @@ bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned i
} else if ((flags & SCRIPT_VERIFY_LOW_S) != 0 && !IsLowDERSignature(vchSigCopy, serror)) {
// serror is set
return false;
} else if ((flags & SCRIPT_VERIFY_STRICTENC) != 0 && !IsDefinedHashtypeSignature(vchSigCopy)) {
} else if ((flags & SCRIPT_VERIFY_STRICTENC) != 0 && !IsDefinedHashtypeSignature(vchSigCopy, flags)) {
return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE);
}
return true;
Expand Down Expand Up @@ -368,7 +374,7 @@ static bool EvalChecksigPreTapscript(const valtype& vchSig, const valtype& vchPu
//serror is set
return false;
}
fSuccess = checker.CheckECDSASignature(vchSig, vchPubKey, scriptCode, sigversion);
fSuccess = checker.CheckECDSASignature(vchSig, vchPubKey, scriptCode, sigversion, flags);

if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size())
return set_error(serror, SCRIPT_ERR_SIG_NULLFAIL);
Expand Down Expand Up @@ -1466,7 +1472,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
}

// Check signature
bool fOk = checker.CheckECDSASignature(vchSig, vchPubKey, scriptCode, sigversion);
bool fOk = checker.CheckECDSASignature(vchSig, vchPubKey, scriptCode, sigversion, flags);

if (fOk) {
isig++;
Expand Down Expand Up @@ -1647,13 +1653,15 @@ class CTransactionSignatureSerializer
const CScript& scriptCode; //!< output script being consumed
const unsigned int nIn; //!< input index of txTo being signed
const bool fAnyoneCanPay; //!< whether the hashtype has the SIGHASH_ANYONECANPAY flag set
const bool fRangeproof; //!< whether the hashtype has the SIGHASH_RANGEPROOF flag set
const bool fHashSingle; //!< whether the hashtype is SIGHASH_SINGLE
const bool fHashNone; //!< whether the hashtype is SIGHASH_NONE

public:
CTransactionSignatureSerializer(const T& txToIn, const CScript& scriptCodeIn, unsigned int nInIn, int nHashTypeIn) :
CTransactionSignatureSerializer(const T& txToIn, const CScript& scriptCodeIn, unsigned int nInIn, int nHashTypeIn, unsigned int flags) :
txTo(txToIn), scriptCode(scriptCodeIn), nIn(nInIn),
fAnyoneCanPay(!!(nHashTypeIn & SIGHASH_ANYONECANPAY)),
fRangeproof(!!(flags & SCRIPT_SIGHASH_RANGEPROOF) && !!(nHashTypeIn & SIGHASH_RANGEPROOF)),
fHashSingle((nHashTypeIn & 0x1f) == SIGHASH_SINGLE),
fHashNone((nHashTypeIn & 0x1f) == SIGHASH_NONE) {}

Expand Down Expand Up @@ -1710,11 +1718,23 @@ class CTransactionSignatureSerializer
/** Serialize an output of txTo */
template<typename S>
void SerializeOutput(S &s, unsigned int nOutput) const {
if (fHashSingle && nOutput != nIn)
if (fHashSingle && nOutput != nIn) {
// Do not lock-in the txout payee at other indices as txin
::Serialize(s, CTxOut());
else
} else {
::Serialize(s, txTo.vout[nOutput]);

// Serialize rangeproof
if (fRangeproof) {
if (nOutput < txTo.witness.vtxoutwit.size()) {
::Serialize(s, txTo.witness.vtxoutwit[nOutput].vchRangeproof);
::Serialize(s, txTo.witness.vtxoutwit[nOutput].vchSurjectionproof);
} else {
::Serialize(s, (unsigned char) 0);
::Serialize(s, (unsigned char) 0);
}
}
}
}

/** Serialize txTo */
Expand Down Expand Up @@ -1803,6 +1823,20 @@ uint256 GetSpentScriptsSHA256(const std::vector<CTxOut>& outputs_spent)
return ss.GetSHA256();
}

template <class T>
uint256 GetRangeproofsHash(const T& txTo) {
CHashWriter ss(SER_GETHASH, 0);
for (size_t i = 0; i < txTo.vout.size(); i++) {
if (i < txTo.witness.vtxoutwit.size()) {
ss << txTo.witness.vtxoutwit[i].vchRangeproof;
ss << txTo.witness.vtxoutwit[i].vchSurjectionproof;
} else {
ss << (unsigned char) 0;
ss << (unsigned char) 0;
}
}
return ss.GetHash();
}

} // namespace

Expand Down Expand Up @@ -1850,6 +1884,7 @@ void PrecomputedTransactionData::Init(const T& txTo, std::vector<CTxOut>&& spent
hashSequence = SHA256Uint256(m_sequences_single_hash);
hashIssuance = SHA256Uint256(GetIssuanceSHA256(txTo));
hashOutputs = SHA256Uint256(m_outputs_single_hash);
hashRangeproofs = GetRangeproofsHash(txTo);
m_bip143_segwit_ready = true;
}
if (uses_bip341_taproot) {
Expand Down Expand Up @@ -1962,7 +1997,7 @@ bool SignatureHashSchnorr(uint256& hash_out, const ScriptExecutionData& execdata
}

template <class T>
uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CConfidentialValue& amount, SigVersion sigversion, const PrecomputedTransactionData* cache)
uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CConfidentialValue& amount, SigVersion sigversion, unsigned int flags, const PrecomputedTransactionData* cache)
{
assert(nIn < txTo.vin.size());

Expand All @@ -1971,7 +2006,9 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn
uint256 hashSequence;
uint256 hashIssuance;
uint256 hashOutputs;
uint256 hashRangeproofs;
const bool cacheready = cache && cache->m_bip143_segwit_ready;
bool fRangeproof = !!(flags & SCRIPT_SIGHASH_RANGEPROOF) && !!(nHashType & SIGHASH_RANGEPROOF);

if (!(nHashType & SIGHASH_ANYONECANPAY)) {
hashPrevouts = cacheready ? cache->hashPrevouts : SHA256Uint256(GetPrevoutsSHA256(txTo));
Expand All @@ -1987,10 +2024,26 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn

if ((nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) {
hashOutputs = cacheready ? cache->hashOutputs : SHA256Uint256(GetOutputsSHA256(txTo));

if (fRangeproof) {
hashRangeproofs = cacheready ? cache->hashRangeproofs : SHA256Uint256(GetRangeproofsHash(txTo));
}
} else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) {
CHashWriter ss(SER_GETHASH, 0);
ss << txTo.vout[nIn];
hashOutputs = ss.GetHash();

if (fRangeproof) {
CHashWriter ss(SER_GETHASH, 0);
if (nIn < txTo.witness.vtxoutwit.size()) {
ss << txTo.witness.vtxoutwit[nIn].vchRangeproof;
ss << txTo.witness.vtxoutwit[nIn].vchSurjectionproof;
} else {
ss << (unsigned char) 0;
ss << (unsigned char) 0;
}
hashRangeproofs = ss.GetHash();
}
}

CHashWriter ss(SER_GETHASH, 0);
Expand Down Expand Up @@ -2019,6 +2072,11 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn
}
// Outputs (none/one/all, depending on flags)
ss << hashOutputs;
if (fRangeproof) {
// This addition must be conditional because it was added after
// the segwit sighash was specified.
ss << hashRangeproofs;
}
// Locktime
ss << txTo.nLockTime;
// Sighash type
Expand All @@ -2036,7 +2094,7 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn
}

// Wrapper to serialize only the necessary parts of the transaction being signed
CTransactionSignatureSerializer<T> txTmp(txTo, scriptCode, nIn, nHashType);
CTransactionSignatureSerializer<T> txTmp(txTo, scriptCode, nIn, nHashType, flags);

// Serialize and hash
CHashWriter ss(SER_GETHASH, 0);
Expand All @@ -2057,7 +2115,7 @@ bool GenericTransactionSignatureChecker<T>::VerifySchnorrSignature(Span<const un
}

template <class T>
bool GenericTransactionSignatureChecker<T>::CheckECDSASignature(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
bool GenericTransactionSignatureChecker<T>::CheckECDSASignature(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const
{
CPubKey pubkey(vchPubKey);
if (!pubkey.IsValid())
Expand All @@ -2070,7 +2128,7 @@ bool GenericTransactionSignatureChecker<T>::CheckECDSASignature(const std::vecto
int nHashType = vchSig.back();
vchSig.pop_back();

uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata);
uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, flags, this->txdata);

if (!VerifyECDSASignature(vchSig, pubkey, sighash))
return false;
Expand Down
18 changes: 14 additions & 4 deletions src/script/interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ enum
SIGHASH_DEFAULT = 0, //!< Taproot only; implied when sighash byte is missing, and equivalent to SIGHASH_ALL
SIGHASH_OUTPUT_MASK = 3,
SIGHASH_INPUT_MASK = 0x80,

// ELEMENTS:
// A flag that means the rangeproofs should be included in the sighash.
SIGHASH_RANGEPROOF = 0x40,
};

/** Script verification flags.
Expand Down Expand Up @@ -140,9 +144,15 @@ enum
// Making unknown public key versions (in BIP 342 scripts) non-standard
SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_PUBKEYTYPE = (1U << 20),

// ELEMENTS:

// Signature checking assumes no sighash byte after the DER signature
//
SCRIPT_NO_SIGHASH_BYTE = (1U << 21),

// Support/allow SIGHASH_RANGEPROOF.
//
SCRIPT_SIGHASH_RANGEPROOF = (1U << 22),
};

bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, unsigned int flags, ScriptError* serror);
Expand All @@ -160,7 +170,7 @@ struct PrecomputedTransactionData
bool m_bip341_taproot_ready = false;

// BIP143 precomputed data (double-SHA256).
uint256 hashPrevouts, hashSequence, hashOutputs, hashIssuance;
uint256 hashPrevouts, hashSequence, hashOutputs, hashIssuance, hashRangeproofs;
//! Whether the 3 fields above are initialized.
bool m_bip143_segwit_ready = false;

Expand Down Expand Up @@ -223,12 +233,12 @@ static constexpr size_t TAPROOT_CONTROL_MAX_NODE_COUNT = 128;
static constexpr size_t TAPROOT_CONTROL_MAX_SIZE = TAPROOT_CONTROL_BASE_SIZE + TAPROOT_CONTROL_NODE_SIZE * TAPROOT_CONTROL_MAX_NODE_COUNT;

template <class T>
uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CConfidentialValue& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr);
uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CConfidentialValue& amount, SigVersion sigversion, unsigned int flags, const PrecomputedTransactionData* cache = nullptr);

class BaseSignatureChecker
{
public:
virtual bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
virtual bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const
{
return false;
}
Expand Down Expand Up @@ -267,7 +277,7 @@ class GenericTransactionSignatureChecker : public BaseSignatureChecker
public:
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CConfidentialValue& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CConfidentialValue& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const override;
bool CheckSchnorrSignature(Span<const unsigned char> sig, Span<const unsigned char> pubkey, SigVersion sigversion, const ScriptExecutionData& execdata, ScriptError* serror = nullptr) const override;
bool CheckLockTime(const CScriptNum& nLockTime) const override;
bool CheckSequence(const CScriptNum& nSequence) const override;
Expand Down
Loading

0 comments on commit 22cf380

Please sign in to comment.