Merge pull request #259 from LedgerHQ/opreturn

Allow up to 5 data pushes in OP_RETURN outputs

CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 Dates are in `dd-mm-yyyy` format.
 
+## [2.2.3] - 06-05-2024
+
+### Added
+
+- Support for signing transactions with `OP_RETURN` outputs extended to up to 5 push opcodes, instead of a single one.
+
 ## [2.2.2] - 08-04-2024
 
 ### Added

Makefile

@@ -47,7 +47,7 @@ PATH_SLIP21_APP_LOAD_PARAMS = "LEDGER-Wallet policy"
 # Application version
 APPVERSION_M = 2
 APPVERSION_N = 2
-APPVERSION_P = 2
+APPVERSION_P = 3
 APPVERSION_SUFFIX = # if not empty, appended at the end. Do not add a dash.
 
 ifeq ($(APPVERSION_SUFFIX),)

src/common/script.c

@@ -123,73 +123,99 @@ int format_opscript_script(const uint8_t script[],
         return -1;
     }
 
+    if (script_len > 83) {
+        // a script that is more than 83 bytes violates the "max 80 bytes total data" rule
+        // (+ 3 bytes of opcodes) and is therefore not standard in Bitcoin Core.
+        return -1;
+    }
+
     strncpy(out, "OP_RETURN ", MAX_OPRETURN_OUTPUT_DESC_SIZE);
     int out_ctr = 10;
 
     // If the length of the script is 1 (just "OP_RETURN"), then it's not standard per bitcoin-core.
     // However, signing such outputs is part of BIP-0322, and there's no danger in allowing them.
 
     if (script_len == 1) {
-        --out_ctr;  // remove extra space
-    } else {
-        // We parse the rest as a single push opcode.
-        // This supports a subset of the scripts that bitcoin-core considers standard.
+        out[out_ctr - 1] = '\0';  // remove extra space
+        return out_ctr;
+    }
+
+    size_t offset = 1;  // start after OP_RETURN
+    int num_pushes = 0;
+    const char hex[] = "0123456789abcdef";
+
+    while (offset < script_len && num_pushes < 5) {
+        uint8_t opcode = script[offset++];
+        size_t hex_length = 0;  // Data length to process
 
-        uint8_t opcode = script[1];  // the push opcode
         if (opcode > OP_16 || opcode == OP_RESERVED || opcode == OP_PUSHDATA2 ||
             opcode == OP_PUSHDATA4) {
             return -1;  // unsupported
         }
 
-        int hex_offset = 1;
-        size_t hex_length = 0;  // if non-zero, `hex_length` bytes starting from script[hex_offset]
-                                // must be hex-encoded
-
         if (opcode == OP_0) {
-            if (script_len != 1 + 1) return -1;
             out[out_ctr++] = '0';
         } else if (opcode >= 1 && opcode <= 75) {
-            hex_offset += 1;
+            // opcodes between 1 and 75 indicate a data push of the corresponding length
             hex_length = opcode;
-
-            if (script_len != 1 + 1 + hex_length) return -1;
         } else if (opcode == OP_PUSHDATA1) {
-            // OP_RETURN OP_PUSHDATA1 <len:1-byte> <data:len bytes>
-            hex_offset += 2;
-            hex_length = script[2];
-
-            if (script_len != 1 + 1 + 1 + hex_length || hex_length > 80) return -1;
+            // the next byte is the length
+            if (offset >= script_len) {
+                return -1;  // out of bounds for length byte
+            }
+            hex_length = script[offset++];
+            if (hex_length <= 75) {
+                return -1;  // non-standard, should have used the minimal push opcode
+            }
         } else if (opcode == OP_1NEGATE) {
-            if (script_len != 1 + 1) return -1;
-
             out[out_ctr++] = '-';
             out[out_ctr++] = '1';
         } else if (opcode >= OP_1 && opcode <= OP_16) {
-            if (script_len != 1 + 1) return -1;
-
-            // encode OP_1 to OP_16 as a decimal number
             uint8_t num = opcode - 0x50;
+            // num is a number between 1 and 16 (included)
             if (num >= 10) {
-                out[out_ctr++] = '0' + (num / 10);
+                out[out_ctr++] = '1';
+                num -= 10;
             }
-            out[out_ctr++] = '0' + (num % 10);
+            out[out_ctr++] = '0' + num;
         } else {
-            return -1;  // can never happen
+            // any other opcode is invalid or unsupported
+            return -1;
         }
 
-        if (hex_length > 0) {
-            const char hex[] = "0123456789abcdef";
+        if (offset + hex_length > script_len) {
+            // overflow, not enough bytes to read in the script
+            return -1;
+        }
+
+        if (hex_length == 1) {
+            if (script[offset] == 0x81 || script[offset] <= 16) {
+                // non-standard, it should use OP_1NEGATE, or one of OP_0, ..., OP_16
+                return -1;
+            }
+        }
 
+        if (hex_length > 0) {
             out[out_ctr++] = '0';
             out[out_ctr++] = 'x';
             for (unsigned int i = 0; i < hex_length; i++) {
-                uint8_t data = script[hex_offset + i];
+                uint8_t data = script[offset + i];
                 out[out_ctr++] = hex[data / 16];
                 out[out_ctr++] = hex[data % 16];
             }
+            offset += hex_length;
         }
+
+        num_pushes++;
+        out[out_ctr++] = ' ';
+    }
+
+    if (offset < script_len) {
+        // if there are still more opcodes left, we do not support this script
+        // (for example: more than 5 push opcodes)
+        return -1;
     }
 
-    out[out_ctr++] = '\0';
+    out[out_ctr - 1] = '\0';
     return out_ctr;
-}
+}

src/common/script.h

@@ -199,26 +199,36 @@ int get_script_address(const uint8_t script[], size_t script_len, char *out, siz
 
 #endif
 
-// the longest OP_RETURN description "OP_RETURN 0x" followed by 160 hexadecimal characters
-#define MAX_OPRETURN_OUTPUT_DESC_SIZE (12 + 80 * 2 + 1)
+// the longest OP_RETURN description is upper bounded by:
+// - 9 bytes for "OP_RETURN"
+// - 5 times 3 for the " 0x"
+// - up to 2 * 80 = 160 hexadecimal bytes
+// - the termination null character
+#define MAX_OPRETURN_OUTPUT_DESC_SIZE (9 + 5 * 3 + 2 * 80 + 1)
 
 /**
  * Formats a valid OP_RETURN script for user verification. The resulting string is "OP_RETURN
- * <data>", where <data> is written according to the rules below. Only scripts with a single push
- * opcode are supported, and OP_PUSHDATA2 and OP_PUSHDATA4 are not supported. OP_1NEGATE is
+ * <data>", where <data> is written according to the rules below. Only scripts with up to 5 push
+ * opcodes are supported, and OP_PUSHDATA2 and OP_PUSHDATA4 are not supported. OP_1NEGATE is
  * represented as "-1", and OP_0, OP_1, ..., OP_16 are represented in decimal ("0", "1", ..., "16").
  * For other push opcodes, the data is represented in hexadecimal, two characters per byte, with the
  * "0x" prefix.
  *
+ * As a best-effort measure, this function returns an error if the transaction is non-standard
+ * according to the default rules of Bitcoin Core (maximum 80 bytes of data, only push
+ * instructions). Such transactions, even if valid, would not easily be relayed in the default
+ * mempool.
+ * An exception is an output script with a single OP_RETURN is accepted despite being non-standard,
+ * as such an output is used in BIP-0322.
+ *
  * The string is written onto `out` and is 0-terminated. Its length is returned.
  *
  * @param script the script to parse and format.
  * @param script_len the length of the script.
- * @param out the output array, that must be at least MAX_OPRETURN_OUTPUT_DESC_SIZE bytes long. The
- * longest possible string is "OP_RETURN 0x" followed by 160 hexadecimal characters, plus the
- * terminating null character, for a total of 173 characters.
+ * @param out the output array, that must be at least MAX_OPRETURN_OUTPUT_DESC_SIZE bytes long.
  * @return The length of the string written into `out` (including the terminating 0) on success; -1
- * on error.
+ * on error, or if such an output would make the transaction be non-standard per the default relay
+ * rules of Bitcoin Core.
  */
 int format_opscript_script(const uint8_t script[],
                            size_t script_len,

unit-tests/test_script.c

@@ -218,9 +218,6 @@ static void test_format_opscript_script_valid(void **state) {
         "0x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425"
         "262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a");
 
-    uint8_t input20[] = {OP_RETURN, OP_PUSHDATA1, 7, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
-    CHECK_VALID_TESTCASE(input20, "OP_RETURN 0x01020304050607");
-
     uint8_t input21[] = {OP_RETURN, OP_PUSHDATA1, 80, 0,  1,  2,  3,  4,  5,  6,  7,  8,  9,  10,
                          11,        12,           13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24,
                          25,        26,           27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38,
@@ -236,8 +233,17 @@ static void test_format_opscript_script_valid(void **state) {
     uint8_t input22[] = {OP_RETURN, OP_1NEGATE};
     CHECK_VALID_TESTCASE(input22, "OP_RETURN -1");
 
-    uint8_t input_23[] = {OP_RETURN};
-    CHECK_VALID_TESTCASE(input_23, "OP_RETURN");
+    uint8_t input23[] = {OP_RETURN, OP_0, OP_1, OP_5, OP_7, OP_16};
+    CHECK_VALID_TESTCASE(input23, "OP_RETURN 0 1 5 7 16");
+
+    uint8_t input24[] = {OP_RETURN, OP_8, OP_1NEGATE, 15,   1,    2,    3,    4,   5,  6,
+                         7,         8,    9,          10,   11,   12,   13,   14,  15, OP_0,
+                         7,         0x11, 0x22,       0x33, 0x44, 0x55, 0x66, 0x77};
+    CHECK_VALID_TESTCASE(input24,
+                         "OP_RETURN 8 -1 0x0102030405060708090a0b0c0d0e0f 0 0x11223344556677");
+
+    uint8_t input_25[] = {OP_RETURN};
+    CHECK_VALID_TESTCASE(input_25, "OP_RETURN");
 }
 
 static void test_format_opscript_script_invalid(void **state) {
@@ -263,11 +269,45 @@ static void test_format_opscript_script_invalid(void **state) {
         {OP_RETURN, OP_PUSHDATA4, 0x06, 0x00, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06};
     CHECK_INVALID_TESTCASE(input_pushdata4);
 
-    uint8_t input_extra_push[] = {OP_RETURN, OP_0, OP_0};
+    uint8_t input_extra_push[] = {OP_RETURN, 4, 1, 2, 3, 4, 42};
     CHECK_INVALID_TESTCASE(input_extra_push);
 
-    uint8_t input_extra_push2[] = {OP_RETURN, 4, 1, 2, 3, 4, 42};
-    CHECK_INVALID_TESTCASE(input_extra_push2);
+    uint8_t input_6pushes[] = {OP_RETURN, OP_1, OP_2, OP_3, OP_4, OP_5, OP_6};
+    CHECK_INVALID_TESTCASE(input_6pushes);
+
+    // clang-format off
+    uint8_t input_too_long[] = {
+        OP_RETURN,
+        OP_PUSHDATA1, 81,
+        0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+        0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+        0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+        0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+        0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+        81
+    };
+    // clang-format on
+    CHECK_INVALID_TESTCASE(input_too_long);
+
+    // not minimal push encodings
+    uint8_t input_pushdata_nonstandard[] =
+        {OP_RETURN, OP_PUSHDATA1, 7, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
+    CHECK_INVALID_TESTCASE(input_pushdata_nonstandard);
+
+    uint8_t input_negative1_notminimal_1[] = {OP_RETURN, 1, 0x81};
+    CHECK_INVALID_TESTCASE(input_negative1_notminimal_1);
+    uint8_t input_negative1_notminimal_2[] = {OP_RETURN, OP_PUSHDATA1, 1, 0x81};
+    CHECK_INVALID_TESTCASE(input_negative1_notminimal_2);
+    for (uint8_t i = 0; i <= 16; i++) {
+        uint8_t input_negative1_notminimal_push_1[] = {OP_RETURN, 1, i};
+        CHECK_INVALID_TESTCASE(input_negative1_notminimal_push_1);
+        uint8_t input_negative1_notminimal_push_2[] = {OP_RETURN, OP_PUSHDATA1, 1, i};
+        CHECK_INVALID_TESTCASE(input_negative1_notminimal_push_2);
+    }
+
+    // transaction containing non-push opcodes are not standard
+    uint8_t input_non_push_opcode[] = {OP_RETURN, OP_3, OP_2, OP_ADD, OP_0};
+    CHECK_INVALID_TESTCASE(input_non_push_opcode);
 }
 
 int main() {