feat(common-scripts): whitelist to disallow p2sh by default

.changeset/green-pumpkins-build.md

@@ -2,4 +2,4 @@
 "@ckb-lumos/common-scripts": minor
 ---
 
-**BREAKING CHANGE**: `allowP2SH` is required in `createOmnilockScript` to convert p2sh addresses
+**BREAKING CHANGE**: `createOmnilockScript` uses the `allow` option to restrict allowed btc addresses

.eslintrc.next.js

@@ -23,6 +23,7 @@ module.exports = {
               -1, // index -1 is not found
               0, // first element of an array
               1, // common for i + 1 in a loop
+              2, // slice(2) for string that starts with "0x" is common to work with 3rd-party libs
               16, // toString(16)
               1000, // second to millisecond
             ],

packages/common-scripts/src/omnilock-bitcoin.ts

@@ -2,42 +2,44 @@ import { bytes, BytesLike } from "@ckb-lumos/codec";
 import { bech32 } from "bech32";
 import bs58 from "bs58";
 
+export type SupportedBtcAddressType = "P2SH-P2WPKH" | "P2WPKH" | "P2PKH";
+
 // https://github.com/cryptape/omnilock/blob/9419b7795641da0ade25a04127e25d8a0b709077/c/ckb_identity.h#L28
 const BTC_PREFIX = "CKB (Bitcoin Layer) transaction: 0x";
 
 /**
  * Decode bitcoin address to public key hash in bytes
  * @see https://en.bitcoin.it/wiki/List_of_address_prefixes
  * @param address
- * @param allowP2SH defaults to false
+ * @param allows
  */
 export function decodeAddress(
   address: string,
-  allowP2SH?: boolean
+  allows: SupportedBtcAddressType[]
 ): ArrayLike<number> {
   const btcAddressFlagSize = 1;
   const hashSize = 20;
 
-  if (isP2wpkhAddress(address)) {
+  if (isP2wpkhAddress(address) && allows.includes("P2WPKH")) {
     return bech32.fromWords(bech32.decode(address).words.slice(1));
   }
 
-  if (isP2pkhAddress(address)) {
+  if (isP2pkhAddress(address) && allows.includes("P2PKH")) {
     return bs58
       .decode(address)
       .slice(btcAddressFlagSize, btcAddressFlagSize + hashSize);
   }
 
   if (isP2shAddress(address)) {
-    if (!allowP2SH) {
-      throw new Error(
-        "'allowP2SH' must be true to enable decoding the P2SH address"
-      );
+    if (allows.includes("P2SH-P2WPKH")) {
+      return bs58
+        .decode(address)
+        .slice(btcAddressFlagSize, btcAddressFlagSize + hashSize);
     }
 
-    return bs58
-      .decode(address)
-      .slice(btcAddressFlagSize, btcAddressFlagSize + hashSize);
+    throw new Error(
+      "'P2SH-P2WPKH' must be included in the 'allows' for the P2SH address"
+    );
   }
 
   // https://bitcoin.design/guide/glossary/address/#taproot-address---p2tr

packages/common-scripts/src/omnilock.ts

@@ -38,6 +38,7 @@ import * as bitcoin from "./omnilock-bitcoin";
 import * as solana from "./omnilock-solana";
 import { decode as bs58Decode } from "bs58";
 import { ckbHash } from "@ckb-lumos/base/lib/utils";
+import { SupportedBtcAddressType } from "./omnilock-bitcoin";
 
 const { ScriptValue } = values;
 
@@ -66,6 +67,7 @@ export type IdentityEthereum = {
    */
   content: BytesLike;
 };
+
 export type IdentityBitcoin = {
   flag: "BITCOIN";
   /**
@@ -77,12 +79,11 @@ export type IdentityBitcoin = {
   content: string;
 
   /**
-   * defaults to false.
-   * when it is true, the P2SH address will be allowed for an Omnilock.
-   * make sure that the P2SH address is a P2SH-P2WPKH address,
-   * or the script CANNOT be unlocked
+   * Allows the P2PKH and P2WPKH by default.
+   * To allow the P2SH-P2WPKH address,
+   * change this option to `["P2PKH", "P2WPKH", "P2SH-P2WPKH"]`
    */
-  allowP2SH?: boolean;
+  allows?: SupportedBtcAddressType[];
 };
 
 export type IdentitySolana = {
@@ -186,7 +187,7 @@ export function createOmnilockScript(
             [IdentityFlagsType.IdentityFlagsBitcoin],
             bitcoin.decodeAddress(
               omnilockInfo.auth.content,
-              omnilockInfo.auth.allowP2SH
+              omnilockInfo.auth.allows || ["P2WPKH", "P2PKH"]
             ),
             omnilockArgs
           )

packages/common-scripts/tests/omnilock-bitcoin.test.ts

@@ -12,7 +12,11 @@ import { mockOutPoint } from "@ckb-lumos/debugger/lib/context";
 import { createOmnilockScript, OmnilockWitnessLock } from "../src/omnilock";
 import { address, AddressType, core, keyring } from "@unisat/wallet-sdk";
 import { NetworkType } from "@unisat/wallet-sdk/lib/network";
-import { Provider, signMessage } from "../src/omnilock-bitcoin";
+import {
+  Provider,
+  signMessage,
+  SupportedBtcAddressType,
+} from "../src/omnilock-bitcoin";
 import { SimpleKeyring } from "@unisat/wallet-sdk/lib/keyring";
 
 test.before(async () => {
@@ -41,15 +45,15 @@ test.serial("Omnilock#Bitcoin P2WPKH", async (t) => {
 
 test.serial("Omnilock#Bitcoin P2SH_P2WPKH", async (t) => {
   const { provider } = makeProvider(AddressType.P2SH_P2WPKH);
-  const result = await execute(provider);
+  const result = await execute(provider, ["P2SH-P2WPKH"]);
 
   t.is(result.code, 0, result.message);
 });
 
-async function execute(provider: Provider) {
+async function execute(provider: Provider, allows?: SupportedBtcAddressType[]) {
   const addr = (await provider.requestAccounts())[0];
 
-  const { txSkeleton, lock } = await setupTxSkeleton(addr);
+  const { txSkeleton, lock } = await setupTxSkeleton(addr, allows);
 
   const message = txSkeleton.get("signingEntries").get(0)!.message;
   const signature = await signMessage(message, "ecdsa", provider);
@@ -95,11 +99,14 @@ function makeProvider(addressType: AddressType): {
   };
 }
 
-async function setupTxSkeleton(addr: string) {
+async function setupTxSkeleton(
+  addr: string,
+  allows?: SupportedBtcAddressType[]
+) {
   const txSkeleton = TransactionSkeleton().asMutable();
 
   const lock = createOmnilockScript(
-    { auth: { flag: "BITCOIN", content: addr } },
+    { auth: { flag: "BITCOIN", content: addr, allows } },
     { config: managerConfig }
   );
 
@@ -135,7 +142,7 @@ test("Omnilock#Bitcoin P2SH", (t) => {
 
   t.notThrows(() =>
     createOmnilockScript({
-      auth: { flag: "BITCOIN", content: p2shAddr, allowP2SH: true },
+      auth: { flag: "BITCOIN", content: p2shAddr, allows: ["P2SH-P2WPKH"] },
     })
   );
 });

website/docs/migrations/migrate-to-v0.24.md

@@ -0,0 +1,19 @@
+# Migrate to Lumos v0.24
+
+## Disallow the Omnilock P2SH Address by Default
+
+The default options of `createOmnilockScript` disallows the use of P2SH addresses for security reasons.
+Not all P2SH addresses are P2SH-P2WPKH addresses.
+This means that developers may unintentionally use a non-P2SH-P2WPKH address to convert to an Omnilock script,
+which can lead to the script not being lockable.
+If you still need to use a P2SH address, use the following code
+
+```diff
+createOmnilockScript({
+  auth: {
+    flag: "BITCOIN",
+    content: addr,
++   allows: ["P2WPKH", "P2PKH", "P2SH-P2WPKH"]
+  }
+})
+```