Repository: bitcoinjs/bitcoinjs-lib
Pull request #867 (2017-08-24 00:55:10)

script: use asMinimalOP for ASM/decompile
Everything is now consistent in `compile/decompile`.

dcousens: Merging as we generally each agreed to this,  and this is from https://github.com/bitcoinjs/bitcoinjs-lib/pull/863 which had had, _that_ part reviewed.

Any ACKs still appreciated however.

Patch:
diff --git a/src/script.js b/src/script.js
index ecb4e854..5e7a1104 100644
--- a/src/script.js
+++ b/src/script.js
@@ -24,6 +24,13 @@ function isPushOnly (value) {
   return types.Array(value) && value.every(isPushOnlyChunk)
 }
 
+function asMinimalOP (buffer) {
+  if (buffer.length === 0) return OPS.OP_0
+  if (buffer.length !== 1) return
+  if (buffer[0] >= 1 && buffer[0] <= 16) return OP_INT_BASE + buffer[0]
+  if (buffer[0] === 0x81) return OPS.OP_1NEGATE
+}
+
 function compile (chunks) {
   // TODO: remove me
   if (Buffer.isBuffer(chunks)) return chunks
@@ -34,7 +41,7 @@ function compile (chunks) {
     // data chunk
     if (Buffer.isBuffer(chunk)) {
       // adhere to BIP62.3, minimal push policy
-      if (chunk.length === 1 && (chunk[0] === 0x81 || (chunk[0] >= 1 && chunk[0] <= 16))) {
+      if (chunk.length === 1 && asMinimalOP(chunk) !== undefined) {
         return accum + 1
       }
 
@@ -52,21 +59,14 @@ function compile (chunks) {
     // data chunk
     if (Buffer.isBuffer(chunk)) {
       // adhere to BIP62.3, minimal push policy
-      if (chunk.length === 1 && chunk[0] >= 1 && chunk[0] <= 16) {
-        var opcode = OP_INT_BASE + chunk[0]
+      var opcode = asMinimalOP(chunk)
+      if (opcode !== undefined) {
         buffer.writeUInt8(opcode, offset)
         offset += 1
         return
       }
 
-      if (chunk.length === 1 && chunk[0] === 0x81) {
-        buffer.writeUInt8(OPS.OP_1NEGATE, offset)
-        offset += 1
-        return
-      }
-
       offset += pushdata.encode(buffer, chunk.length, offset)
-
       chunk.copy(buffer, offset)
       offset += chunk.length
 
@@ -107,7 +107,13 @@ function decompile (buffer) {
       var data = buffer.slice(i, i + d.number)
       i += d.number
 
-      chunks.push(data)
+      // decompile minimally
+      var op = asMinimalOP(data)
+      if (op !== undefined) {
+        chunks.push(op)
+      } else {
+        chunks.push(data)
+      }
 
     // opcode
     } else {
@@ -127,7 +133,11 @@ function toASM (chunks) {
 
   return chunks.map(function (chunk) {
     // data?
-    if (Buffer.isBuffer(chunk)) return chunk.toString('hex')
+    if (Buffer.isBuffer(chunk)) {
+      var op = asMinimalOP(chunk)
+      if (op === undefined) return chunk.toString('hex')
+      chunk = op
+    }
 
     // opcode!
     return REVERSE_OPS[chunk]


Test patch:
diff --git a/test/fixtures/script.json b/test/fixtures/script.json
index b5286cfa..3ba8bc3c 100644
--- a/test/fixtures/script.json
+++ b/test/fixtures/script.json
@@ -155,7 +155,7 @@
         "06"
       ],
       "nonstandard": {
-        "scriptSig": "06",
+        "scriptSig": "OP_6",
         "scriptSigHex": "0106"
       }
     },
diff --git a/test/script.js b/test/script.js
index e443fae8..4adf2935 100644
--- a/test/script.js
+++ b/test/script.js
@@ -24,7 +24,6 @@ describe('script', function () {
     fixtures.valid.forEach(function (f) {
       it('encodes/decodes ' + f.asm, function () {
         var scriptSig = bscript.fromASM(f.asm)
-
         assert.strictEqual(bscript.toASM(scriptSig), f.asm)
       })
     })
@@ -52,17 +51,16 @@ describe('script', function () {
   describe('toStack', function () {
     fixtures.valid.forEach(function (f) {
       it('returns ' + !!f.stack + ' for ' + f.asm, function () {
+        if (!f.stack || !f.asm) return
+
         var script = bscript.fromASM(f.asm)
 
-        if (f.stack && f.asm) {
-          try {
-            var stack = bscript.toStack(script)
-            assert.deepEqual(stack.map(function (x) { return x.toString('hex') }), f.stack)
-            assert.equal(bscript.toASM(bscript.compile(stack)), f.asm, 'should rebuild same script from stack')
-          } catch (e) {
-            assert.strictEqual(f.stack, undefined)
-          }
-        }
+        var stack = bscript.toStack(script)
+        assert.deepEqual(stack.map(function (x) {
+          return x.toString('hex')
+        }), f.stack)
+
+        assert.equal(bscript.toASM(bscript.compile(stack)), f.asm, 'should rebuild same script from stack')
       })
     })
   })