Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
},
"dependencies": {
"bigi": "^1.4.0",
"bip66": "^1.1.0",
"bs58check": "^1.0.5",
"create-hash": "^1.1.0",
"create-hmac": "^1.1.3",
Expand Down
52 changes: 7 additions & 45 deletions src/ecsignature.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var bip66 = require('bip66')
var typeforce = require('typeforce')
var types = require('./types')

Expand Down Expand Up @@ -29,36 +30,10 @@ ECSignature.parseCompact = function (buffer) {
}
}

// Strict DER - https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki
// NOTE: SIGHASH byte ignored
ECSignature.fromDER = function (buffer) {
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
if (buffer.length < 8) throw new Error('DER sequence too short')
if (buffer.length > 72) throw new Error('DER sequence too long')
if (buffer[0] !== 0x30) throw new Error('Not a DER sequence')
if (buffer[1] !== buffer.length - 2) throw new Error('Invalid sequence length')
if (buffer[2] !== 0x02) throw new Error('Expected a DER integer')

var lenR = buffer[3]
if (lenR === 0) throw new Error('R length is zero')
if (5 + lenR >= buffer.length) throw new Error('Invalid DER encoding')
if (buffer[4 + lenR] !== 0x02) throw new Error('Expected a DER integer (2)')

var lenS = buffer[5 + lenR]
if (lenS === 0) throw new Error('S length is zero')
if ((lenR + lenS + 6) !== buffer.length) throw new Error('Invalid DER encoding (2)')

if (buffer[4] & 0x80) throw new Error('R value is negative')
if (lenR > 1 && (buffer[4] === 0x00) && !(buffer[5] & 0x80)) throw new Error('R value excessively padded')

if (buffer[lenR + 6] & 0x80) throw new Error('S value is negative')
if (lenS > 1 && (buffer[lenR + 6] === 0x00) && !(buffer[lenR + 7] & 0x80)) throw new Error('S value excessively padded')

// non-BIP66 - extract R, S values
var rB = buffer.slice(4, 4 + lenR)
var sB = buffer.slice(lenR + 6)
var r = BigInteger.fromDERInteger(rB)
var s = BigInteger.fromDERInteger(sB)
var decode = bip66.decode(buffer)
var r = BigInteger.fromDERInteger(decode.r)
var s = BigInteger.fromDERInteger(decode.s)

return new ECSignature(r, s)
}
Expand Down Expand Up @@ -93,23 +68,10 @@ ECSignature.prototype.toCompact = function (i, compressed) {
}

ECSignature.prototype.toDER = function () {
var rBa = this.r.toDERInteger()
var sBa = this.s.toDERInteger()
var r = new Buffer(this.r.toDERInteger())
var s = new Buffer(this.s.toDERInteger())

var sequence = []

// INTEGER
sequence.push(0x02, rBa.length)
sequence = sequence.concat(rBa)

// INTEGER
sequence.push(0x02, sBa.length)
sequence = sequence.concat(sBa)

// SEQUENCE
sequence.unshift(0x30, sequence.length)

return new Buffer(sequence)
return bip66.encode(r, s)
}

ECSignature.prototype.toScriptSignature = function (hashType) {
Expand Down
41 changes: 18 additions & 23 deletions src/script.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
var bip66 = require('bip66')
var bufferutils = require('./bufferutils')
var typeforce = require('typeforce')
var types = require('./types')

var ECSignature = require('./ecsignature')
var ecurve = require('ecurve')
var curve = ecurve.getCurveByName('secp256k1')

var OPS = require('./opcodes')
var REVERSE_OPS = []
for (var op in OPS) {
Expand Down Expand Up @@ -118,34 +115,31 @@ function decompile (buffer) {

function isCanonicalPubKey (buffer) {
if (!Buffer.isBuffer(buffer)) return false

try {
ecurve.Point.decodeFrom(curve, buffer)
} catch (e) {
if (!(e.message.match(/Invalid sequence (length|tag)/))) {
throw e
}

return false
if (buffer.length < 33) return false

switch (buffer[0]) {
case 0x02:
case 0x03:
return buffer.length === 33
case 0x04:
return buffer.length === 65
}

return true
return false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, huge performance improvement here. Added tests to cover it, overall script tests running in half the time, isolated benchmark gives >200% improvement.

In terms of validity, this is now on par with bitcoin-core: https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L67-L87

}

function isCanonicalSignature (buffer) {
if (!Buffer.isBuffer(buffer)) return false
if (!isDefinedHashType(buffer[buffer.length - 1])) return false

try {
ECSignature.parseScriptSignature(buffer)
} catch (e) {
if (!(e.message.match(/Not a DER sequence|Invalid sequence length|Expected a DER integer|R length is zero|S length is zero|R value excessively padded|S value excessively padded|R value is negative|S value is negative|Invalid hashType/))) {
throw e
}
return bip66.check(buffer.slice(0, -1))
}

return false
}
function isDefinedHashType (hashType) {
var hashTypeMod = hashType & ~0x80

return true
// return hashTypeMod > SIGHASH_ALL && hashTypeMod < SIGHASH_SINGLE
return hashTypeMod > 0x00 && hashTypeMod < 0x04
}

function isPubKeyHashInput (script) {
Expand Down Expand Up @@ -379,6 +373,7 @@ module.exports = {

isCanonicalPubKey: isCanonicalPubKey,
isCanonicalSignature: isCanonicalSignature,
isDefinedHashType: isDefinedHashType,
isPubKeyHashInput: isPubKeyHashInput,
isPubKeyHashOutput: isPubKeyHashOutput,
isPubKeyInput: isPubKeyInput,
Expand Down
4 changes: 1 addition & 3 deletions test/bitcoin.core.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,12 @@ describe('Bitcoin-core', function () {
if (i % 2 !== 0) return

var description = sig_noncanonical[i - 1].slice(0, -1)
if (description === 'too long') return // we support non secp256k1 signatures

var buffer = new Buffer(hex, 'hex')

it('throws on ' + description, function () {
assert.throws(function () {
bitcoin.ECSignature.parseScriptSignature(buffer)
})
}, /Expected DER (integer|sequence)|(R|S) value (excessively padded|is negative)|(R|S|DER sequence) length is (zero|too short|too long|invalid)|Invalid hashType/)
})
})
})
Expand Down
36 changes: 20 additions & 16 deletions test/fixtures/ecsignature.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,37 +130,33 @@
],
"DER": [
{
"exception": "DER sequence too short",
"exception": "DER sequence length is too short",
"hex": "ffffffffffffff"
},
{
"exception": "DER sequence too long",
"exception": "DER sequence length is too long",
"hex": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
},
{
"exception": "Invalid sequence length",
"exception": "Expected DER sequence",
"hex": "00ffff0400ffffff020400ffffff"
},
{
"exception": "DER sequence length is invalid",
"hex": "30ff020400ffffff020400ffffff"
},
{
"exception": "Invalid sequence length",
"exception": "DER sequence length is invalid",
"hex": "300c030400ffffff030400ffffff0000"
},
{
"exception": "Expected a DER integer",
"exception": "Expected DER integer",
"hex": "300cff0400ffffff020400ffffff"
},
{
"exception": "Expected a DER integer \\(2\\)",
"exception": "Expected DER integer \\(2\\)",
"hex": "300c020200ffffff020400ffffff"
},
{
"exception": "Invalid DER encoding",
"hex": "300c0204ddffffff020200ffffff"
},
{
"exception": "Invalid DER encoding \\(2\\)",
"hex": "300c020400ffffff02dd00ffffff"
},
{
"exception": "R length is zero",
"hex": "30080200020400ffffff"
Expand All @@ -169,13 +165,21 @@
"exception": "S length is zero",
"hex": "3008020400ffffff0200"
},
{
"exception": "R length is too long",
"hex": "300c02dd00ffffff020400ffffff"
},
{
"exception": "S length is invalid",
"hex": "300c020400ffffff02dd00ffffff"
},
{
"exception": "R value is negative",
"hex": "300c0204ffffffff020400ffffff"
"hex": "300c020480000000020400ffffff"
},
{
"exception": "S value is negative",
"hex": "300c020400ffffff0204ffffffff"
"hex": "300c020400ffffff020480000000"
},
{
"exception": "R value excessively padded",
Expand Down
22 changes: 20 additions & 2 deletions test/fixtures/script.json
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,26 @@
],
"isPubKeyInput": [
{
"description": "non-canonical signature",
"scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf7593ffffffffffffffff"
"description": "non-canonical signature (too short)",
"scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf7593"
},
{
"description": "non-canonical signature (too long)",
"scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf75939446f6ca28ffffffff01"
},
{
"description": "non-canonical signature (invalid hashType)",
"scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf75939446f6ca28ff"
}
],
"isPubKeyOutput": [
{
"description": "non-canonical pubkey (too short)",
"scriptPubKey": "02359c6e3f04cefbf089cf1d6670dc47c3fb4df68e2bad1fa5a369f9ce OP_CHECKSIG"
},
{
"description": "non-canonical pubkey (too long)",
"scriptPubKey": "02359c6e3f04cefbf089cf1d6670dc47c3fb4df68e2bad1fa5a369f9ce4b42bbd1ffffff OP_CHECKSIG"
}
],
"isMultisigOutput": [
Expand Down