Page MenuHomePhabricator
Differential D2477 Diff 7093 src/script/sigencoding.cpp

Changeset View

Standalone View

View Options

src/script/sigencoding.cpp

Show All 18 Lines
* not set), and not excessively padded (do not start with a 0 byte, unless an * not set), and not excessively padded (do not start with a 0 byte, unless an
* otherwise negative number follows, in which case a single 0 byte is * otherwise negative number follows, in which case a single 0 byte is
* necessary and even required). * necessary and even required).
* *
* See https://bitcointalk.org/index.php?topic=8392.msg127623#msg127623 * See https://bitcointalk.org/index.php?topic=8392.msg127623#msg127623
* *
* This function is consensus-critical since BIP66. * This function is consensus-critical since BIP66.
*/ */
static bool IsValidSignatureEncoding(const slicedvaltype &sig) { static bool IsValidECDSASignatureEncoding(const slicedvaltype &sig) {
deadalnixUnsubmitted
Done
Inline Actions

It has nothing to do with ECDSA, it is DER encoding. Please adjust the name accordingly. CheckRawECDSASignatureEncoding is the only part doing anything ECDSA specific.

deadalnix: It has nothing to do with ECDSA, it is DER encoding. Please adjust the name accordingly.
MengerianAuthorUnsubmitted
Done
Inline Actions

OK, renamed to IsValidDERSignatureEncoding

Mengerian: OK, renamed to `IsValidDERSignatureEncoding`
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S] // Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
// * total-length: 1-byte length descriptor of everything that follows, // * total-length: 1-byte length descriptor of everything that follows,
// excluding the sighash byte. // excluding the sighash byte.
// * R-length: 1-byte length descriptor of the R value that follows. // * R-length: 1-byte length descriptor of the R value that follows.
// * R: arbitrary-length big-endian encoded R value. It must use the // * R: arbitrary-length big-endian encoded R value. It must use the
// shortest possible encoding for a positive integers (which means no null // shortest possible encoding for a positive integers (which means no null
// bytes at the start, except a single one when the next byte has its // bytes at the start, except a single one when the next byte has its
// highest bit set). // highest bit set).
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Linesstatic bool IsValidECDSASignatureEncoding(const slicedvaltype &sig) {
// out of bound elements. // out of bound elements.
if (lenS > 1 && (sig[startS + 2] == 0x00) && !(sig[startS + 3] & 0x80)) { if (lenS > 1 && (sig[startS + 2] == 0x00) && !(sig[startS + 3] & 0x80)) {
return false; return false;
} }
return true; return true;
} }
static bool CheckRawSignatureEncoding(const slicedvaltype &sig, uint32_t flags, static bool CheckRawECDSASignatureEncoding(const slicedvaltype &sig,
uint32_t flags,
ScriptError *serror) { ScriptError *serror) {
if ((flags & (SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | if ((flags & (SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S |
SCRIPT_VERIFY_STRICTENC)) && SCRIPT_VERIFY_STRICTENC)) &&
!IsValidSignatureEncoding(sig)) { !IsValidECDSASignatureEncoding(sig)) {
return set_error(serror, SCRIPT_ERR_SIG_DER); return set_error(serror, SCRIPT_ERR_SIG_DER);
} }
if ((flags & SCRIPT_VERIFY_LOW_S) && !CPubKey::CheckLowS(sig)) { if ((flags & SCRIPT_VERIFY_LOW_S) && !CPubKey::CheckLowS(sig)) {
return set_error(serror, SCRIPT_ERR_SIG_HIGH_S); return set_error(serror, SCRIPT_ERR_SIG_HIGH_S);
} }
return true; return true;
} }
static bool CheckRawSignatureEncoding(const slicedvaltype &sig, uint32_t flags,
ScriptError *serror) {
return CheckRawECDSASignatureEncoding(sig, flags, serror);
markblundebergUnsubmitted
Not Done
Inline Actions

my diffs will add just before this line
if(size==64 & schnorr) return true;

markblundeberg: my diffs will add just before this line if(size==64 & schnorr) return true;
MengerianAuthorUnsubmitted
Done
Inline Actions

Yes, agreed.

Mengerian: Yes, agreed.
}
bool CheckDataSignatureEncoding(const valtype &vchSig, uint32_t flags, bool CheckDataSignatureEncoding(const valtype &vchSig, uint32_t flags,
ScriptError *serror) { ScriptError *serror) {
// Empty signature. Not strictly DER encoded, but allowed to provide a // Empty signature. Not strictly DER encoded, but allowed to provide a
// compact way to provide an invalid signature for use with CHECK(MULTI)SIG // compact way to provide an invalid signature for use with CHECK(MULTI)SIG
if (vchSig.size() == 0) { if (vchSig.size() == 0) {
return true; return true;
} }
return CheckRawSignatureEncoding( return CheckRawSignatureEncoding(
vchSig | boost::adaptors::sliced(0, vchSig.size()), flags, serror); vchSig | boost::adaptors::sliced(0, vchSig.size()), flags, serror);
} }
bool CheckTransactionSignatureEncoding(const valtype &vchSig, uint32_t flags, static bool CheckSighashEncoding(const valtype &vchSig, uint32_t flags,
ScriptError *serror) { ScriptError *serror) {
// Empty signature. Not strictly DER encoded, but allowed to provide a
// compact way to provide an invalid signature for use with CHECK(MULTI)SIG
if (vchSig.size() == 0) {
return true;
}
if (!CheckRawSignatureEncoding(
vchSig | boost::adaptors::sliced(0, vchSig.size() - 1), flags,
serror)) {
// serror is set
return false;
}
if (flags & SCRIPT_VERIFY_STRICTENC) { if (flags & SCRIPT_VERIFY_STRICTENC) {
if (!GetHashType(vchSig).isDefined()) { if (!GetHashType(vchSig).isDefined()) {
return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE); return set_error(serror, SCRIPT_ERR_SIG_HASHTYPE);
} }
bool usesForkId = GetHashType(vchSig).hasForkId(); bool usesForkId = GetHashType(vchSig).hasForkId();
bool forkIdEnabled = flags & SCRIPT_ENABLE_SIGHASH_FORKID; bool forkIdEnabled = flags & SCRIPT_ENABLE_SIGHASH_FORKID;
if (!forkIdEnabled && usesForkId) { if (!forkIdEnabled && usesForkId) {
return set_error(serror, SCRIPT_ERR_ILLEGAL_FORKID); return set_error(serror, SCRIPT_ERR_ILLEGAL_FORKID);
} }
if (forkIdEnabled && !usesForkId) { if (forkIdEnabled && !usesForkId) {
return set_error(serror, SCRIPT_ERR_MUST_USE_FORKID); return set_error(serror, SCRIPT_ERR_MUST_USE_FORKID);
} }
} }
return true; return true;
} }
bool CheckTransactionECDSASignatureEncoding(const valtype &vchSig, bool CheckTransactionECDSASignatureEncoding(const valtype &vchSig,
uint32_t flags, uint32_t flags,
ScriptError *serror) { ScriptError *serror) {
return CheckTransactionSignatureEncoding(vchSig, flags, serror); // Empty signature. Not strictly DER encoded, but allowed to provide a
// compact way to provide an invalid signature for use with CHECK(MULTI)SIG
if (vchSig.size() == 0) {
return true;
}
markblundebergUnsubmitted
Not Done
Inline Actions

my diffs will add here:
if(size==64 & schnorr) set error & return false;

markblundeberg: my diffs will add here: if(size==64 & schnorr) set error & return false;
markblundebergUnsubmitted
Not Done
Inline Actions

correction: size == 65

markblundeberg: correction: size == 65
MengerianAuthorUnsubmitted
Done
Inline Actions

I was actually imagining the 64-byte ban being inside CheckRawECDSASignatureEncoding

But it works in either place.

Mengerian: I was actually imagining the 64-byte ban being inside CheckRawECDSASignatureEncoding But it…
markblundebergUnsubmitted
Not Done
Inline Actions

Oh! Good point, yes that works too, even better...

markblundeberg: Oh! Good point, yes that works too, even better...
if (!CheckRawECDSASignatureEncoding(
vchSig | boost::adaptors::sliced(0, vchSig.size() - 1), flags,
serror)) {
// serror is set
return false;
}
return CheckSighashEncoding(vchSig, flags, serror);
}
bool CheckTransactionSignatureEncoding(const valtype &vchSig, uint32_t flags,
ScriptError *serror) {
// Empty signature. Not strictly DER encoded, but allowed to provide a
// compact way to provide an invalid signature for use with CHECK(MULTI)SIG
if (vchSig.size() == 0) {
return true;
}
if (!CheckRawSignatureEncoding(
vchSig | boost::adaptors::sliced(0, vchSig.size() - 1), flags,
serror)) {
// serror is set
return false;
}
return CheckSighashEncoding(vchSig, flags, serror);
deadalnixUnsubmitted
Done
Inline Actions

Because both have the exact same structure, as the exception of the CheckRawSignatureEncoding call, it's probaby a good idea to build a template.

template<typename F> static bool CheckTransactionSignatureEncodingImpl(const valtype &vchSig, uint32_t flags, ScriptError *serror, F fun) {
  // Empty signature. Not strictly DER encoded, but allowed to provide a
  // compact way to provide an invalid signature for use with CHECK(MULTI)SIG
  if (vchSig.size() == 0) {
      return true;
  }

  if (!fun(vchSig | boost::adaptors::sliced(0, vchSig.size() - 1), flags, serror)) {
      // serror is set
      return false;
  }

  return CheckSighashEncoding(vchSig, flags, serror);
}

bool CheckTransactionSignatureEncoding(const valtype &vchSig, uint32_t flags,
                                     ScriptError *serror) {
  return CheckTransactionSignatureEncodingImpl(vchSig, flags, serror,
      [](const slicedvaltype &sig, uint32_t flags, ScriptError *serror) {
        return CheckRawSignatureEncoding(sig, flags, serror);
      });
}

And so on.

deadalnix: Because both have the exact same structure, as the exception of the `CheckRawSignatureEncoding`…
MengerianAuthorUnsubmitted
Done
Inline Actions

OK, I got the template working.

The compiler didn't like the parameter names (vchSig, flags, serror) repeated, so I used the names "templateSig", "templateFlags", and "templateSerror" because I couldn't think of anything better. Let me know if you would prefer other variable names.

Mengerian: OK, I got the template working. The compiler didn't like the parameter names (vchSig, flags…
} }
static bool IsCompressedOrUncompressedPubKey(const valtype &vchPubKey) { static bool IsCompressedOrUncompressedPubKey(const valtype &vchPubKey) {
switch (vchPubKey.size()) { switch (vchPubKey.size()) {
case 33: case 33:
// Compressed public key: must start with 0x02 or 0x03. // Compressed public key: must start with 0x02 or 0x03.
return vchPubKey[0] == 0x02 || vchPubKey[0] == 0x03; return vchPubKey[0] == 0x02 || vchPubKey[0] == 0x03;
Show All 36 Lines