Skip to content

Commit

Permalink
Tightened standardness checks, added more unit tests; added more comm…
Browse files Browse the repository at this point in the history 
…ents
  • Loading branch information
@bigspider
bigspider committed May 13, 2024
1 parent a8759a6 commit 5fe7f05
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 18 deletions.
44 changes: 39 additions & 5 deletions src/common/script.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ 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;

Expand Down Expand Up @@ -150,21 +156,43 @@ int format_opscript_script(const uint8_t script[],
if (opcode == OP_0) {
out[out_ctr++] = '0';
} else if (opcode >= 1 && opcode <= 75) {
// opcodes between 1 and 75 indicate a data push of the corresponding length
hex_length = opcode;
if (offset + hex_length > script_len) return -1; // out of bounds
} else if (opcode == OP_PUSHDATA1) {
if (offset >= script_len) return -1; // out of bounds for length byte
// 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 > 80 || offset + hex_length > script_len) return -1;
if (hex_length <= 75) {
return -1; // non-standard, should have used the minimal push opcode
}
} else if (opcode == OP_1NEGATE) {
out[out_ctr++] = '-';
out[out_ctr++] = '1';
} else if (opcode >= OP_1 && opcode <= OP_16) {
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;
} else {
// any other opcode is invalid or unsupported
return -1;
}

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;
}
out[out_ctr++] = '0' + (num % 10);
}

if (hex_length > 0) {
Expand All @@ -182,6 +210,12 @@ int format_opscript_script(const uint8_t script[],
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 - 1] = '\0';
return out_ctr;
}
18 changes: 12 additions & 6 deletions src/common/script.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,21 +208,27 @@ int get_script_address(const uint8_t script[], size_t script_len, char *out, siz

/**
* 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,
Expand Down
47 changes: 40 additions & 7 deletions unit-tests/test_script.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -239,10 +236,9 @@ static void test_format_opscript_script_valid(void **state) {
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, OP_PUSHDATA1, 15, 1, 2, 3, 4, 5, 6,
7, 8, 9, 10, 11, 12, 13, 14, 15, OP_0, OP_PUSHDATA1,
7, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77};
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");

Expand Down Expand Up @@ -275,6 +271,43 @@ static void test_format_opscript_script_invalid(void **state) {

uint8_t input_extra_push[] = {OP_RETURN, 4, 1, 2, 3, 4, 42};
CHECK_INVALID_TESTCASE(input_extra_push);

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() {
Expand Down

0 comments on commit 5fe7f05

Please sign in to comment.