fix(script): allow multisig scripts generation upto 20 keys and add i…

…sSorted and isWitness flag
bcoin-org · Aug 4, 2023 · f3184af · f3184af
1 parent ca83bdd
commit f3184af
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 12 deletions.
diff --git a/lib/node/rpc.js b/lib/node/rpc.js
@@ -2076,7 +2076,7 @@ class RPC extends RPCBase {
       keys[i] = key;
     }
 
-    const script = Script.fromMultisig(m, n, keys);
+    const script = Script.fromMultisig(m, n, keys, true, false);
 
     if (script.getSize() > consensus.MAX_SCRIPT_PUSH) {
       throw new RPCError(errs.VERIFY_ERROR,

diff --git a/lib/script/script.js b/lib/script/script.js
@@ -28,6 +28,7 @@ const opcodes = common.opcodes;
 const scriptTypes = common.types;
 const {encoding} = bio;
 const {inspectSymbol} = require('../utils');
+const {MAX_MULTISIG_PUBKEYS, MAX_SCRIPT_PUSH} = consensus;
 
 /*
  * Constants
@@ -1480,38 +1481,53 @@ class Script {
    * @param {Number} m
    * @param {Number} n
    * @param {Buffer[]} keys
+   * @param {Boolean?} isSorted
+   * @param {Boolean?} isWitness
+   * @returns {Script}
    */
 
-  fromMultisig(m, n, keys) {
+  fromMultisig(m, n, keys, isSorted = true, isWitness = true) {
     assert((m & 0xff) === m && (n & 0xff) === n);
     assert(Array.isArray(keys));
     assert(keys.length === n, '`n` keys are required for multisig.');
     assert(m >= 1 && m <= n);
-    assert(n >= 1 && n <= 15);
+
+    assert(
+      n >= 1 && n <= MAX_MULTISIG_PUBKEYS,
+      `${n} keys not allowed in script. Max allowed: ${MAX_MULTISIG_PUBKEYS}`
+    );
 
     this.clear();
 
-    this.pushSmall(m);
+    this.pushInt(m);
 
-    for (const key of sortKeys(keys))
+    for (const key of isSorted ? sortKeys(keys) : keys)
       this.pushData(key);
 
-    this.pushSmall(n);
+    this.pushInt(n);
     this.pushOp(opcodes.OP_CHECKMULTISIG);
 
-    return this.compile();
+    const script = this.compile();
+
+    if (!isWitness) {
+      assert(script.getSize() <= MAX_SCRIPT_PUSH, 'Script size is too large');
+    }
+
+    return script;
   }
 
   /**
    * Create a pay-to-multisig script.
    * @param {Number} m
    * @param {Number} n
    * @param {Buffer[]} keys
+   * @param {Boolean?} isSorted
+   * @param {Boolean?} isWitness
    * @returns {Script}
    */
 
-  static fromMultisig(m, n, keys) {
-    return new this().fromMultisig(m, n, keys);
+  static fromMultisig(m, n, keys, isSorted = true, isWitness = true) {
+    return new this().fromMultisig(m, n, keys, isSorted, isWitness);
   }
 
   /**

diff --git a/lib/wallet/account.js b/lib/wallet/account.js
@@ -15,6 +15,7 @@ const Script = require('../script/script');
 const WalletKey = require('./walletkey');
 const {HDPublicKey} = require('../hd/hd');
 const {inspectSymbol} = require('../utils');
+const consensus = require('../protocol/consensus');
 
 /**
  * Account
@@ -148,8 +149,14 @@ class Account {
 
     this.accountKey = options.accountKey;
 
-    if (this.n > 1)
+    if (this.n > 1) {
       this.type = Account.types.MULTISIG;
+      const maxAllowedKeys = this.witness ? consensus.MAX_MULTISIG_PUBKEYS : 15;
+      assert(
+        this.n <= maxAllowedKeys,
+        `n ranges between 1 and ${maxAllowedKeys}`
+      );
+    }
 
     if (!this.name)
       this.name = this.accountIndex.toString(10);
@@ -496,7 +503,9 @@ class Account {
           keys.push(key.publicKey);
         }
 
-        ring.script = Script.fromMultisig(this.m, this.n, keys);
+        ring.script = Script.fromMultisig(
+          this.m, this.n, keys, true, this.witness
+        );
 
         break;
     }

diff --git a/test/script-test.js b/test/script-test.js
@@ -10,8 +10,9 @@ const Stack = require('../lib/script/stack');
 const Opcode = require('../lib/script/opcode');
 const TX = require('../lib/primitives/tx');
 const consensus = require('../lib/protocol/consensus');
-const {fromFloat} = require('../lib/utils/fixed');
+const {HDPrivateKey} = require('../lib/hd');
 
+const {fromFloat} = require('../lib/utils/fixed');
 // test files: https://github.com/bitcoin/bitcoin/tree/master/src/test/data
 const scripts = require('./data/core-data/script-tests.json');
 
@@ -345,4 +346,45 @@ describe('Script', function() {
       });
     }
   }
+
+  for (const isSorted of [true, false]) {
+    for (const isWitness of [true, false]) {
+      for (let n = 1; n <= 22; n++) {
+        for (let m = 1; m <= n; m++) {
+          it(`should handle script generation for ${n} keys. (${m} of ${n}) (isSorted = ${isSorted}) (isWitness = ${isWitness})`, () => {
+            const keys = [];
+
+            for (let i = 0; i < n; i++) {
+              keys.push(HDPrivateKey.generate().publicKey);
+            }
+
+            let script;
+            let error;
+
+            try {
+              script = Script.fromMultisig(m, n, keys, isSorted, isWitness).toRaw().toString('hex');
+            } catch (e) {
+              error = e;
+            }
+
+            if (n > 20 || (n > 15 && !isWitness)) {
+              assert(error);
+              assert(error.message === (n > 20 ? `${n} keys not allowed in script. Max allowed: 20` : 'Script size is too large'));
+            } else {
+              if (isSorted) {
+                keys.sort((a, b) => a.compare(b));
+              }
+
+              const expectedScript = Opcode.fromInt(m).toRaw().toString('hex') // threshold
+                + keys.map(key => '21' + key.toString('hex')).join('') // keys
+                + Opcode.fromInt(n).toRaw().toString('hex') // number of keys
+                + 'ae'; // OP_CHECKMULTISIG
+
+              assert(script === expectedScript);
+            }
+          });
+        }
+      }
+    }
+  }
 });
diff --git a/test/wallet-test.js b/test/wallet-test.js
@@ -1501,6 +1501,46 @@ describe('Wallet', function() {
     });
   }
 
+  for (const witness of [true, false]) {
+    it(`should handle multisig account for keys greater than 15 when witness is ${witness} `, async () => {
+      let wallet;
+      let error;
+
+      try {
+        wallet = await wdb.create({
+          type: 'multisig',
+          m: 15,
+          n: 18,
+          witness
+        });
+      } catch (e) {
+        error = e;
+      }
+
+      if (!witness) {
+        assert(error);
+        assert(error.message === 'n ranges between 1 and 15');
+      } else {
+        const keys = [];
+
+        for (let i = 1; i <= 17; i++) {
+          const xpriv = HD.PrivateKey.generate();
+          let key = xpriv.deriveAccount(44, 0, 0).toPublic();
+          await wallet.addSharedKey(0, key);
+          key = key.derivePath('m/0/0').publicKey;
+          keys.push(key);
+        }
+
+        keys.push((await wallet.receiveKey()).publicKey);
+        const walletReceiveAddress = await wallet.receiveAddress();
+        const script = Script.fromMultisig(15, 18, keys, true, true);
+        const expectedAddress = Script.fromProgram(0, script.sha256()).getAddress();
+
+        assert.deepStrictEqual(expectedAddress, walletReceiveAddress);
+      }
+    });
+  }
+
   it('should get range of txs', async () => {
     const wallet = currentWallet;
     const txs = await wallet.getRange(null, {