script: add description for the functionality of each opcode

ChrisCho-H commented at 12:48 pm on February 16, 2023: none

While miniscript has a very kind explanation for it, original script does not. I add a simple description for the functionality of each opcode. It will help a lot for developers who want to do script programming.

DrahtBot commented at 12:49 pm on February 16, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	darosior
Concept ACK	jamesob, jonatack, kristapsk, theStack, brunoerg, RandyMcMillan
Stale ACK	Sjors

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

ChrisCho-H renamed this:
~~Script explained~~
script: add explanation for the functionality of each opcode
on Feb 16, 2023

DrahtBot added the label Consensus on Feb 16, 2023

jamesob commented at 3:26 pm on February 16, 2023: contributor

Concept ACK

Probably worth just squashing this down to 1 commit.

in src/script/script.h:72 in ea38a2d75a outdated

91-    OP_12 = 0x5c,
92-    OP_13 = 0x5d,
93-    OP_14 = 0x5e,
94-    OP_15 = 0x5f,
95-    OP_16 = 0x60,
96+    OP_0 = 0x00, // empty array of bytes

sipa commented at 3:31 pm on February 16, 2023:

Maybe for OP_0, OP_1..OP_16, OP_1NEGATE, elaborate a bit by stating they they push an element onto the stack, and perhaps give the exact byte encoding rather than just the number.

E.g. OP_3 = 0x53, // Push "\x03" onto the stack (which is interpreted as 3 by numerical opcodes)

sipa commented at 3:32 pm on February 16, 2023:

Also, perhaps use /**< ... */ or //!< .. so that doxygen will pick the descriptions up.

in src/script/script.h:104 in ea38a2d75a outdated

133+    OP_VER = 0x62, // mark tx invalid unless occuring in an unexecuted OP_IF branch
134+    OP_IF = 0x63, // if top stack value true, execute the statement
135+    OP_NOTIF = 0x64, // if top stack value false, execute the statement
136+    OP_VERIF = 0x65, // mark tx invalid even when occuring in an unexecuted OP_IF branch
137+    OP_VERNOTIF = 0x66, // mark tx invalid even when occuring in an unexecuted OP_IF branch
138+    OP_ELSE = 0x67, // if neither OP_IF, OP_NOTIF, OP_ELSE not executed, execute the statement

sipa commented at 3:33 pm on February 16, 2023:

I can’t really parse what you’re trying to say here.

in src/script/script.h:107 in ea38a2d75a outdated

136+    OP_VERIF = 0x65, // mark tx invalid even when occuring in an unexecuted OP_IF branch
137+    OP_VERNOTIF = 0x66, // mark tx invalid even when occuring in an unexecuted OP_IF branch
138+    OP_ELSE = 0x67, // if neither OP_IF, OP_NOTIF, OP_ELSE not executed, execute the statement
139+    OP_ENDIF = 0x68, // end if/else block(must include, otherwise tx become invalid)
140+    OP_VERIFY = 0x69, // mark tx invalid if top stack value false
141+    OP_RETURN = 0x6a, // mark tx invalid

sipa commented at 3:33 pm on February 16, 2023:

Maybe add whether or not it also applies in unexecuted branches?

in src/script/script.h:112 in ea38a2d75a outdated

160-    OP_ROT = 0x7b,
161-    OP_SWAP = 0x7c,
162-    OP_TUCK = 0x7d,
163+    OP_TOALTSTACK = 0x6b, // pop an item from the main stack onto the alt stack
164+    OP_FROMALTSTACK = 0x6c, // pop an item from the alt stack onto the main stack
165+    OP_2DROP = 0x6d, // remove top and second-top stack item

sipa commented at 3:34 pm on February 16, 2023:

“second-top” sounds a bit strange to me; perhaps “second from the top”?

Alternatively: “remove the two top stack items” ?

in src/script/script.h:131 in ea38a2d75a outdated

184-    OP_CAT = 0x7e,
185-    OP_SUBSTR = 0x7f,
186-    OP_LEFT = 0x80,
187-    OP_RIGHT = 0x81,
188-    OP_SIZE = 0x82,
189+    OP_CAT = 0x7e, // disabled, fail the script unconditionally

sipa commented at 3:35 pm on February 16, 2023:

Perhaps add whether this also applies in unexecuted branches? What does the unconditionally refer to?

in src/script/script.h:135 in ea38a2d75a outdated

188-    OP_SIZE = 0x82,
189+    OP_CAT = 0x7e, // disabled, fail the script unconditionally
190+    OP_SUBSTR = 0x7f, // disabled, fail the script unconditionally
191+    OP_LEFT = 0x80, // disabled, fail the script unconditionally
192+    OP_RIGHT = 0x81, // disabled, fail the script unconditionally
193+    OP_SIZE = 0x82, // push the length of top stack item

sipa commented at 3:36 pm on February 16, 2023:

Perhaps clarify that this does not pop the top element whose size is inspected.

ChrisCho-H commented at 4:56 pm on February 16, 2023: none

Grateful for kind feedbacks, @sipa I made commit to reflect feedbacks, and squashed this PR down to 1 commit!

ChrisCho-H renamed this:
~~script: add explanation for the functionality of each opcode~~
script: add description for the functionality of each opcode
on Feb 16, 2023

ChrisCho-H force-pushed on Feb 16, 2023

in src/script/script.h:78 in 4e8f9cb8db outdated

 97+    OP_FALSE = OP_0, // push "\x" onto the stack (which is an empty array of bytes)
 98+    OP_PUSHDATA1 = 0x4c, // read the next byte as N and push the next N bytes as an array onto the stack
 99+    OP_PUSHDATA2 = 0x4d, // read the next 2 bytes as N and push the next N bytes as an array onto the stack
100+    OP_PUSHDATA4 = 0x4e, // // read the next 4 bytes as N and push the next N bytes as an array onto the stack
101+    OP_1NEGATE = 0x4f, // push "\x-1" onto the stack (which is interpreted as -1 by numerical opcodes)
102+    OP_RESERVED = 0x50, // mark tx invalid unless occuring in an unexecuted OP_IF branch

jonatack commented at 5:44 pm on February 16, 2023:

Here and elsewhere in your changes.

0    OP_RESERVED = 0x50, // mark tx invalid unless occurring in an unexecuted OP_IF branch

(You can run a spelling check by running test/lint/lint-spelling.py from the repository root.)

in src/script/script.h:105 in 4e8f9cb8db outdated

134+    OP_IF = 0x63, // if top stack value true, execute the statement
135+    OP_NOTIF = 0x64, // if top stack value false, execute the statement
136+    OP_VERIF = 0x65, // mark tx invalid even when occuring in an unexecuted OP_IF branch
137+    OP_VERNOTIF = 0x66, // mark tx invalid even when occuring in an unexecuted OP_IF branch
138+    OP_ELSE = 0x67, // if the preceding OP_IF, OP_NOTIF or OP_ELSE not executed, execute the statement
139+    OP_ENDIF = 0x68, // end if/else block(must include, otherwise tx become invalid)

jonatack commented at 5:46 pm on February 16, 2023:

0    OP_ENDIF = 0x68, // end if/else block (must include, otherwise tx becomes invalid)

in src/script/script.h:114 in 4e8f9cb8db outdated

162-    OP_TUCK = 0x7d,
163+    OP_TOALTSTACK = 0x6b, // pop an item from the main stack onto the alt stack
164+    OP_FROMALTSTACK = 0x6c, // pop an item from the alt stack onto the main stack
165+    OP_2DROP = 0x6d, // remove the two top stack items
166+    OP_2DUP = 0x6e, // duplicate top and second from top stack item
167+    OP_3DUP = 0x6f, // duplicate top, second from top and third from top stack item

jonatack commented at 5:47 pm on February 16, 2023:

0    OP_3DUP = 0x6f, // duplicate top, second from top and third from top stack items

in src/script/script.h:113 in 4e8f9cb8db outdated

161-    OP_SWAP = 0x7c,
162-    OP_TUCK = 0x7d,
163+    OP_TOALTSTACK = 0x6b, // pop an item from the main stack onto the alt stack
164+    OP_FROMALTSTACK = 0x6c, // pop an item from the alt stack onto the main stack
165+    OP_2DROP = 0x6d, // remove the two top stack items
166+    OP_2DUP = 0x6e, // duplicate top and second from top stack item

jonatack commented at 5:47 pm on February 16, 2023:

0    OP_2DUP = 0x6e, // duplicate top and second from top stack items

in src/script/script.h:131 in 4e8f9cb8db outdated

184-    OP_CAT = 0x7e,
185-    OP_SUBSTR = 0x7f,
186-    OP_LEFT = 0x80,
187-    OP_RIGHT = 0x81,
188-    OP_SIZE = 0x82,
189+    OP_CAT = 0x7e, // disabled, fail the script unconditionally(apply in an unexecuted conditional branch)

jonatack commented at 5:48 pm on February 16, 2023:

Here and in the next 8 lines and also in additional lines after that.

0    OP_CAT = 0x7e, // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)

in src/script/script.h:152 in 4e8f9cb8db outdated

243-    OP_WITHIN = 0xa5,
244+    OP_1ADD = 0x8b, // 1 added to the top stack item
245+    OP_1SUB = 0x8c, // 1 subtracted from the top stack item
246+    OP_2MUL = 0x8d, // disabled, fail the script unconditionally(apply in an unexecuted conditional branch)
247+    OP_2DIV = 0x8e, // disabled, fail the script unconditionally(apply in an unexecuted conditional branch)
248+    OP_NEGATE = 0x8f, // multiply the tops stack item by -1

jonatack commented at 5:50 pm on February 16, 2023:

0    OP_NEGATE = 0x8f, // multiply the top stack item by -1

jonatack commented at 5:57 pm on February 16, 2023: member

Concept ACK

RandyMcMillan commented at 6:07 pm on February 16, 2023: contributor

Concept ACK

kristapsk commented at 6:12 pm on February 16, 2023: contributor

Concept ACK

in src/script/script.h:72 in bcff35a76e outdated

91-    OP_12 = 0x5c,
92-    OP_13 = 0x5d,
93-    OP_14 = 0x5e,
94-    OP_15 = 0x5f,
95-    OP_16 = 0x60,
96+    OP_0 = 0x00, // push "\x" onto the stack (which is an empty array of bytes)

sipa commented at 6:21 pm on February 16, 2023:

I’d just say "" or the empty array. “\x” isn’t any valid syntax I know.

in src/script/script.h:77 in bcff35a76e outdated

 96+    OP_0 = 0x00, // push "\x" onto the stack (which is an empty array of bytes)
 97+    OP_FALSE = OP_0, // push "\x" onto the stack (which is an empty array of bytes)
 98+    OP_PUSHDATA1 = 0x4c, // read the next byte as N and push the next N bytes as an array onto the stack
 99+    OP_PUSHDATA2 = 0x4d, // read the next 2 bytes as N and push the next N bytes as an array onto the stack
100+    OP_PUSHDATA4 = 0x4e, // // read the next 4 bytes as N and push the next N bytes as an array onto the stack
101+    OP_1NEGATE = 0x4f, // push "\x-1" onto the stack (which is interpreted as -1 by numerical opcodes)

sipa commented at 6:21 pm on February 16, 2023:

It pushes "\x81" (the byte array consisting of a single byte with value 129).

ChrisCho-H force-pushed on Feb 16, 2023

RandyMcMillan commented at 8:05 pm on February 16, 2023: contributor

applying clang-formatting: per src/.clang-format may be useful

https://github.com/RandyMcMillan/bitcoin/commit/99e3dc536dca94dffd703e77572f6dca91f4bb26

patch: https://raw.githubusercontent.com/bitcoin/bitcoin/433d01c67657f49773a649ba48c264cda833e783/clang-format.patch

in src/script/script.h:100 in 49bdfa51ef outdated

129-    OP_ENDIF = 0x68,
130-    OP_VERIFY = 0x69,
131-    OP_RETURN = 0x6a,
132+    OP_NOP = 0x61, // do nothing
133+    OP_VER = 0x62, // mark tx invalid unless occuring in an unexecuted OP_IF branch
134+    OP_IF = 0x63, // if top stack value true, execute the statement

john-moffett commented at 8:52 pm on February 16, 2023:

We should probably be consistent in describing how Script deals with true/false, since it’s later described as if top stack value != 0. Maybe add a section along the lines of:

0/** 
1* Opcodes that take a true/false value will evaluate the following as false:
2*     an empty vector
3*     a vector (of any length) of all zero bytes 
4*     a single byte of "\x80" ('negative zero')
5*     a vector (of any length) of all zero bytes except the last byte is "\x80"
6*
7* Any other value will evaluate to true.
8* /

I’d also say if the top stack value is true,, and indicate that it consumes the stack item.

ChrisCho-H force-pushed on Feb 17, 2023

ChrisCho-H commented at 2:47 am on February 17, 2023: none

clang-format applied and description for evaluating false added!

WM7586 approved

in src/script/script.h:85 in d6ab4d5157 outdated

104-    OP_16 = 0x60,
105+    OP_0 = 0x00,         // push "" onto the stack (which is an empty array of bytes)
106+    OP_FALSE = OP_0,     // push "" onto the stack (which is an empty array of bytes)
107+    OP_PUSHDATA1 = 0x4c, // read the next byte as N and push the next N bytes as an array onto the stack
108+    OP_PUSHDATA2 = 0x4d, // read the next 2 bytes as N and push the next N bytes as an array onto the stack
109+    OP_PUSHDATA4 = 0x4e, // // read the next 4 bytes as N and push the next N bytes as an array onto the stack

theStack commented at 8:46 am on February 17, 2023:

0    OP_PUSHDATA4 = 0x4e, // read the next 4 bytes as N and push the next N bytes as an array onto the stack

in src/script/script.h:158 in d6ab4d5157 outdated

249-    OP_MIN = 0xa3,
250-    OP_MAX = 0xa4,
251-
252-    OP_WITHIN = 0xa5,
253+    OP_1ADD = 0x8b,      // 1 added to the top stack item
254+    OP_1SUB = 0x8c,      // 1 subtracted from the top stack item

theStack commented at 8:48 am on February 17, 2023:

would also use the active form here

0    OP_1ADD = 0x8b,      // add 1 to the top stack item
1    OP_1SUB = 0x8c,      // subtract 1 from the top stack item

theStack commented at 8:51 am on February 17, 2023: contributor

Concept ACK

ChrisCho-H force-pushed on Feb 17, 2023

darosior commented at 10:06 am on February 17, 2023: member

It will help a lot for developers who want to do script programming.

No strong opinion, but this purpose is already served by this detailed wiki article presenting how each opcode works along with a bit of history. Using a “opcode, input, output, description” table it is even probably better at explaining what each opcode does than we can ever get in code comments.

ChrisCho-H commented at 10:27 am on February 17, 2023: none

Detailed wiki does help dev! However, it’s just a complementary material. If simple description on script.h,

Bitcoin repo itself leads the standard description of opcodes, does not depend on external ones
Newly added opcode can be described here first
dev can get info easier and publicly discuss more to utilize script

These are my personal opinion, and any opinion is welcome!

ChrisCho-H force-pushed on Feb 20, 2023

ghost commented at 11:07 am on February 20, 2023: none

It will help a lot for developers who want to do script programming.

No strong opinion, but this purpose is already served by this detailed wiki article presenting how each opcode works along with a bit of history. Using a “opcode, input, output, description” table it is even probably better at explaining what each opcode does than we can ever get in code comments.

Agree with this. So many single line comments look weird. If wiki has some outdated or incorrect information, maybe that can be updated and use its link in the comment above all opcodes.

ChrisCho-H commented at 1:20 pm on February 20, 2023: none

I followed the way miniscript.h comments. There are so many single lines as well, and look fine & helpful in my opinion. However, I think it’s also not bad to add a simple overall description and wiki link above all opcodes, only if wiki is well managed. I prefer single line comments, though.

brunoerg commented at 1:27 pm on February 20, 2023: contributor

Concept ACK

RandyMcMillan commented at 4:05 pm on February 20, 2023: contributor

NIT

0cd src/ && clang-format --lines 68:148 -i script/script.h

 0diff --git a/src/script/script.h b/src/script/script.h
 1index 35211ff376..5143593029 100644
 2--- a/src/script/script.h
 3+++ b/src/script/script.h
 4@@ -66,17 +66,16 @@ std::vector<unsigned char> ToByteVector(const T& in)
 5 }
 6 
 7 /** Script opcodes */
 8-enum opcodetype
 9-{
10-/**
11- * Opcodes that take a true/false value will evaluate the following as false:
12- *     an empty vector
13- *     a vector (of any length) of all zero bytes
14- *     a single byte of "\x80" ('negative zero')
15- *     a vector (of any length) of all zero bytes except the last byte is "\x80"
16- *
17- * Any other value will evaluate to true.
18- */
19+enum opcodetype {
20+    /**
21+     * Opcodes that take a true/false value will evaluate the following as false:
22+     *     an empty vector
23+     *     a vector (of any length) of all zero bytes
24+     *     a single byte of "\x80" ('negative zero')
25+     *     a vector (of any length) of all zero bytes except the last byte is "\x80"
26+     *
27+     * Any other value will evaluate to true.
28+     */
29     // push value
30     OP_0 = 0x00,         // push "" onto the stack (which is an empty array of bytes)
31     OP_FALSE = OP_0,     // push "" onto the stack (which is an empty array of bytes)

ChrisCho-H commented at 2:08 am on February 21, 2023: none

NIT

0cd src/ && clang-format --lines 68:148 -i script/script.h

 0diff --git a/src/script/script.h b/src/script/script.h
 1index 35211ff376..5143593029 100644
 2--- a/src/script/script.h
 3+++ b/src/script/script.h
 4@@ -66,17 +66,16 @@ std::vector<unsigned char> ToByteVector(const T& in)
 5 }
 6 
 7 /** Script opcodes */
 8-enum opcodetype
 9-{
10-/**
11- * Opcodes that take a true/false value will evaluate the following as false:
12- *     an empty vector
13- *     a vector (of any length) of all zero bytes
14- *     a single byte of "\x80" ('negative zero')
15- *     a vector (of any length) of all zero bytes except the last byte is "\x80"
16- *
17- * Any other value will evaluate to true.
18- */
19+enum opcodetype {
20+    /**
21+     * Opcodes that take a true/false value will evaluate the following as false:
22+     *     an empty vector
23+     *     a vector (of any length) of all zero bytes
24+     *     a single byte of "\x80" ('negative zero')
25+     *     a vector (of any length) of all zero bytes except the last byte is "\x80"
26+     *
27+     * Any other value will evaluate to true.
28+     */
29     // push value
30     OP_0 = 0x00,         // push "" onto the stack (which is an empty array of bytes)
31     OP_FALSE = OP_0,     // push "" onto the stack (which is an empty array of bytes)

Because it’s global comment for opcodes, I didn’t indent. Reference might be this.

ChrisCho-H force-pushed on Feb 23, 2023

RandyMcMillan commented at 11:46 pm on February 24, 2023: contributor

ACK 40f36d0a3dac568bc9b05dea021d83986d8add84

in src/script/script.h:108 in 40f36d0a3d outdated

137-    OP_ELSE = 0x67,
138-    OP_ENDIF = 0x68,
139-    OP_VERIFY = 0x69,
140-    OP_RETURN = 0x6a,
141+    OP_NOP = 0x61,      // do nothing
142+    OP_VER = 0x62,      // mark tx invalid unless occuring in an unexecuted OP_IF branch

vostrnad commented at 5:10 am on March 10, 2023:

misspelled occurring (more instances further below)

in src/script/script.h:197 in 40f36d0a3d outdated

298+    OP_HASH160 = 0xa9,             // hash input using SHA-256, then RIPEMD-160
299+    OP_HASH256 = 0xaa,             // hash input using SHA-256 twice
300+    OP_CODESEPARATOR = 0xab,       // all of the signature checking words will only match signatures to the data after the most recently-executed OP_CODESEPARATOR
301+    OP_CHECKSIG = 0xac,            // push 1 if signature is valid for tx hash and public key, otherwise 0
302+    OP_CHECKSIGVERIFY = 0xad,      // execute OP_CHECKSIG, then OP_VERIFY afterward
303+    OP_CHECKMULTISIG = 0xae,       // OP_CHECKSIG for muti-signature

vostrnad commented at 5:10 am on March 10, 2023:

typo in multi

vostrnad commented at 4:48 pm on March 31, 2023:

Additionally, at least according to Bitcoin Optech, “multisignature” should not be confused with “multisig”: https://bitcoinops.org/en/topics/multisignature/

Sjors commented at 4:50 pm on March 31, 2023:

I guess we can say “for multiple signatures” (but nobody reading this deep inside the codebase will be confused)

ChrisCho-H commented at 1:44 am on March 31, 2023: none

Need more reviewers to be merged?

in src/script/script.h:527 in 40f36d0a3d outdated

328+    OP_NOP8 = 0xb7,                   // do nothing
329+    OP_NOP9 = 0xb8,                   // do nothing
330+    OP_NOP10 = 0xb9,                  // do nothing
331 
332     // Opcode added by BIP 342 (Tapscript)
333     OP_CHECKSIGADD = 0xba,

Sjors commented at 9:26 am on March 31, 2023:

Can be added in a followup if you don’t want to lose ACKs:

0OP_CHECKSIGADD = 0xba,            // pop the public key, N and a signature, push N if signature is empty, fail if it's invalid, otherwise push N + 1 (see BIP 342)

in src/script/script.h:72 in 40f36d0a3d outdated

67@@ -68,139 +68,148 @@ std::vector<unsigned char> ToByteVector(const T& in)
68 /** Script opcodes */
69 enum opcodetype
70 {
71+/**
72+ * Opcodes that take a true/false value will evaluate the following as false:

Sjors commented at 11:01 am on March 31, 2023:

Note to other reviewers: this is implemented in CastToBool() in interpreter.cpp, which is used by OP_IF, OP_NOTIF, OP_VERIFY, OP_IFDUP, OP_NUMEQUALVERIFY and after executing the scriptPubKey.

in src/script/script.h:109 in 40f36d0a3d outdated

138-    OP_ENDIF = 0x68,
139-    OP_VERIFY = 0x69,
140-    OP_RETURN = 0x6a,
141+    OP_NOP = 0x61,      // do nothing
142+    OP_VER = 0x62,      // mark tx invalid unless occuring in an unexecuted OP_IF branch
143+    OP_IF = 0x63,       // if top stack value is true, execute the statement

Sjors commented at 11:16 am on March 31, 2023:

Could add: is true (1 for tapscript)

Although we probably don’t want to go into too much detail here, this is still useful to point out since it directly contradicts the true / false explanation at the top.

in src/script/script.h:110 in 40f36d0a3d outdated

139-    OP_VERIFY = 0x69,
140-    OP_RETURN = 0x6a,
141+    OP_NOP = 0x61,      // do nothing
142+    OP_VER = 0x62,      // mark tx invalid unless occuring in an unexecuted OP_IF branch
143+    OP_IF = 0x63,       // if top stack value is true, execute the statement
144+    OP_NOTIF = 0x64,    // if top stack value is false, execute the statement

Sjors commented at 11:17 am on March 31, 2023:

Could add: is false ("" for tapscript)

in src/script/script.h:124 in 40f36d0a3d outdated

172+    OP_TOALTSTACK = 0x6b,   // pop an item from the main stack onto the alt stack
173+    OP_FROMALTSTACK = 0x6c, // pop an item from the alt stack onto the main stack
174+    OP_2DROP = 0x6d,        // remove the two top stack items
175+    OP_2DUP = 0x6e,         // duplicate top and second from top stack items
176+    OP_3DUP = 0x6f,         // duplicate top, second from top and third from top stack items
177+    OP_2OVER = 0x70,        // copy third and fourth from top stack items to the top

Sjors commented at 11:24 am on March 31, 2023:

Note for other reviewers, the Bitcoin Wiki provides a useful hint on how to read Satoshi’s comments in interpreter.cpp:

the inputs and outputs are both described by items as if they were pushed on the stack in that order. So for example, “x1 x2” indicates pushing value x1 on the stack, then x2, such that x1 is at the bottom of the stack, and x2 is at the top

This also explains why items from the stack are read “out of order”:

0valtype vch1 = stacktop(-4);
1valtype vch2 = stacktop(-3);

I.e. they are read in the sequence that they were pushed.

in src/script/script.h:194 in 40f36d0a3d outdated

295+    OP_RIPEMD160 = 0xa6,           // hash input using RIPEMD-160
296+    OP_SHA1 = 0xa7,                // hash input using SHA-1
297+    OP_SHA256 = 0xa8,              // hash input using SHA-256
298+    OP_HASH160 = 0xa9,             // hash input using SHA-256, then RIPEMD-160
299+    OP_HASH256 = 0xaa,             // hash input using SHA-256 twice
300+    OP_CODESEPARATOR = 0xab,       // all of the signature checking words will only match signatures to the data after the most recently-executed OP_CODESEPARATOR

Sjors commented at 12:10 pm on March 31, 2023:

I still have no idea how OP_CODESEPARATOR works, but that will take more than one code comment to explain.

Sjors approved

Sjors commented at 12:43 pm on March 31, 2023: member

ACK 40f36d0a3dac568bc9b05dea021d83986d8add84

It matches my (limited) understanding of what these opcodes do. I also walked through EvalScript() to compare.

vostrnad changes_requested

ChrisCho-H commented at 4:58 am on April 3, 2023: none

Added followup commit for tapscript(+ suggestion from @vostrnad )!

Sjors commented at 10:46 am on April 3, 2023: member

re-ACK 45926671dbf9e4919bf6aaee90e0d90f89830ce5

Nit: consider moving the typo fixes to the first commit (or just squash both).

ChrisCho-H force-pushed on Apr 4, 2023

Sjors approved

Sjors commented at 8:03 am on April 4, 2023: member

re-ACK 86b43d7fdd086c308f58414f3718a9cbf106bd05

ChrisCho-H requested review from sipa on Apr 13, 2023

ChrisCho-H commented at 6:04 am on April 24, 2023: none

Looking for code reviewers! appreciate voluntary dedication in advance 🙏

in src/script/script.h:127 in 74ce388681 outdated

175+    OP_2DUP = 0x6e,         // duplicate top and second from top stack items
176+    OP_3DUP = 0x6f,         // duplicate top, second from top and third from top stack items
177+    OP_2OVER = 0x70,        // copy third and fourth from top stack items to the top
178+    OP_2ROT = 0x71,         // move fifth and sixth from top stack items to the top
179+    OP_2SWAP = 0x72,        // swap: top and second from top <-> third and fourth from top items of stack
180+    OP_IFDUP = 0x73,        // if top stack value is false, duplicate it

theStack commented at 0:24 am on May 25, 2023:

0    OP_IFDUP = 0x73,        // if top stack value is true, duplicate it

in src/script/script.h:151 in 74ce388681 outdated

212-    OP_RESERVED2 = 0x8a,
213+    OP_INVERT = 0x83,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
214+    OP_AND = 0x84,         // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
215+    OP_OR = 0x85,          // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
216+    OP_XOR = 0x86,         // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
217+    OP_EQUAL = 0x87,       // return 1 if the inputs are equal, 0 otherwise

theStack commented at 0:30 am on May 25, 2023:

Should probably also mention here that the inputs are popped from the stack? E.g. something like pop two top stack items and push 1 if they are equal, 0 otherwise

ChrisCho-H commented at 2:19 am on May 25, 2023: none

Thanks! I’ve fixed it.

DrahtBot added the label CI failed on May 25, 2023

ChrisCho-H force-pushed on May 25, 2023

ChrisCho-H commented at 10:13 am on May 25, 2023: none

rebase done!

DrahtBot removed the label CI failed on May 25, 2023

ryanofsky commented at 7:46 pm on July 19, 2023: contributor

This PR has two stale acks and seems like a safe documentation change. I think it would be good to merge if original reviewers want to reack or anyone else wants to review.

in src/script/script.h:81 in b20eacdd3b outdated

74+ *     a vector (of any length) of all zero bytes
75+ *     a single byte of "\x80" ('negative zero')
76+ *     a vector (of any length) of all zero bytes except the last byte is "\x80"
77+ *
78+ * Any other value will evaluate to true.
79+ */

sipa commented at 3:00 pm on July 20, 2023:

In the text below it seems somewhat arbitrary whether opcodes have an effect when appearing in unexecuted branches. I suggest instead adding something like this up front:

0 * Unless specified otherwise, all opcodes below only have an effect when they occur in executed branches.

in src/script/script.h:87 in b20eacdd3b outdated

106+    OP_FALSE = OP_0,     // push "" onto the stack (which is an empty array of bytes)
107+    OP_PUSHDATA1 = 0x4c, // read the next byte as N and push the next N bytes as an array onto the stack
108+    OP_PUSHDATA2 = 0x4d, // read the next 2 bytes as N and push the next N bytes as an array onto the stack
109+    OP_PUSHDATA4 = 0x4e, // read the next 4 bytes as N and push the next N bytes as an array onto the stack
110+    OP_1NEGATE = 0x4f,   // push "\x81" onto the stack (which is interpreted as -1 by numerical opcodes)
111+    OP_RESERVED = 0x50,  // mark tx invalid unless occurring in an unexecuted OP_IF branch

sipa commented at 3:01 pm on July 20, 2023:

Suggestion: “mark transaction invalid.”

in src/script/script.h:108 in b20eacdd3b outdated

137-    OP_ELSE = 0x67,
138-    OP_ENDIF = 0x68,
139-    OP_VERIFY = 0x69,
140-    OP_RETURN = 0x6a,
141+    OP_NOP = 0x61,      // do nothing
142+    OP_VER = 0x62,      // mark tx invalid unless occurring in an unexecuted OP_IF branch

sipa commented at 3:02 pm on July 20, 2023:

Suggestion : “mark transaction invalid”

in src/script/script.h:109 in b20eacdd3b outdated

138-    OP_ENDIF = 0x68,
139-    OP_VERIFY = 0x69,
140-    OP_RETURN = 0x6a,
141+    OP_NOP = 0x61,      // do nothing
142+    OP_VER = 0x62,      // mark tx invalid unless occurring in an unexecuted OP_IF branch
143+    OP_IF = 0x63,       // if top stack value is true (1 for tapscript), execute the statement

sipa commented at 3:02 pm on July 20, 2023:

More precise: (exactly "\x01" for tapscript)

in src/script/script.h:116 in b20eacdd3b outdated

145+    OP_VERIF = 0x65,    // mark tx invalid even when occurring in an unexecuted OP_IF branch
146+    OP_VERNOTIF = 0x66, // mark tx invalid even when occurring in an unexecuted OP_IF branch
147+    OP_ELSE = 0x67,     // if the preceding OP_IF, OP_NOTIF or OP_ELSE not executed, execute the statement
148+    OP_ENDIF = 0x68,    // end if/else block (must include, otherwise tx becomes invalid)
149+    OP_VERIFY = 0x69,   // mark tx invalid if top stack value is false, unless occurring in an unexecuted OP_IF branch
150+    OP_RETURN = 0x6a,   // mark tx invalid unless occurring in an unexecuted OP_IF branch

sipa commented at 3:03 pm on July 20, 2023:

Suggestion: “mark tx invalid”

in src/script/script.h:122 in b20eacdd3b outdated

170-    OP_SWAP = 0x7c,
171-    OP_TUCK = 0x7d,
172+    OP_TOALTSTACK = 0x6b,   // pop an item from the main stack onto the alt stack
173+    OP_FROMALTSTACK = 0x6c, // pop an item from the alt stack onto the main stack
174+    OP_2DROP = 0x6d,        // remove the two top stack items
175+    OP_2DUP = 0x6e,         // duplicate top and second from top stack items

sipa commented at 3:04 pm on July 20, 2023:

I think it would be useful to use a notation like [ ... x0 x1] -> [... x0 x1 x0 x1] for these opcodes instead of trying to explain in English.

in src/script/script.h:140 in b20eacdd3b outdated

193-    OP_CAT = 0x7e,
194-    OP_SUBSTR = 0x7f,
195-    OP_LEFT = 0x80,
196-    OP_RIGHT = 0x81,
197-    OP_SIZE = 0x82,
198+    OP_CAT = 0x7e,    // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)

sipa commented at 3:05 pm on July 20, 2023:

Suggestion: “mark tx invalid, even when occurring in unexecuted branch” (here and below), for consistency with language elsewhere.

in src/script/script.h:151 in b20eacdd3b outdated

212-    OP_RESERVED2 = 0x8a,
213+    OP_INVERT = 0x83,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
214+    OP_AND = 0x84,         // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
215+    OP_OR = 0x85,          // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
216+    OP_XOR = 0x86,         // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
217+    OP_EQUAL = 0x87,       // pop two top stack items and push 1 if they are equal, 0 otherwise

sipa commented at 3:07 pm on July 20, 2023:

Use "\x01" and "" instead of 1 and 0 (as 1 and 0 are interpretations, which have multiple possible encodings).

in src/script/script.h:153 in b20eacdd3b outdated

214+    OP_AND = 0x84,         // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
215+    OP_OR = 0x85,          // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
216+    OP_XOR = 0x86,         // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
217+    OP_EQUAL = 0x87,       // pop two top stack items and push 1 if they are equal, 0 otherwise
218+    OP_EQUALVERIFY = 0x88, // execute OP_EQUAL, then OP_VERIFY afterward
219+    OP_RESERVED1 = 0x89,   // mark tx invalid unless occurring in an unexecuted OP_IF branch

sipa commented at 3:07 pm on July 20, 2023:

Suggestion: “mark tx invalid”

in src/script/script.h:159 in b20eacdd3b outdated

250-    OP_MAX = 0xa4,
251-
252-    OP_WITHIN = 0xa5,
253+    OP_1ADD = 0x8b,      // add 1 to the top stack item
254+    OP_1SUB = 0x8c,      // subtract 1 from the top stack item
255+    OP_2MUL = 0x8d,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)

sipa commented at 3:08 pm on July 20, 2023:

“mark tx invalid even when occurring in unexecuted branch”

in src/script/script.h:162 in b20eacdd3b outdated

253+    OP_1ADD = 0x8b,      // add 1 to the top stack item
254+    OP_1SUB = 0x8c,      // subtract 1 from the top stack item
255+    OP_2MUL = 0x8d,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
256+    OP_2DIV = 0x8e,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
257+    OP_NEGATE = 0x8f,    // multiply the top stack item by -1
258+    OP_ABS = 0x90,       // absolute the top stack item value

sipa commented at 3:08 pm on July 20, 2023:

“replace top stack item by its absolute value”

in src/script/script.h:163 in b20eacdd3b outdated

254+    OP_1SUB = 0x8c,      // subtract 1 from the top stack item
255+    OP_2MUL = 0x8d,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
256+    OP_2DIV = 0x8e,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
257+    OP_NEGATE = 0x8f,    // multiply the top stack item by -1
258+    OP_ABS = 0x90,       // absolute the top stack item value
259+    OP_NOT = 0x91,       // convert 0 to 1 and else to 0

sipa commented at 3:08 pm on July 20, 2023:

convert 0 to "\x01" and everything else to ""

in src/script/script.h:164 in b20eacdd3b outdated

255+    OP_2MUL = 0x8d,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
256+    OP_2DIV = 0x8e,      // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
257+    OP_NEGATE = 0x8f,    // multiply the top stack item by -1
258+    OP_ABS = 0x90,       // absolute the top stack item value
259+    OP_NOT = 0x91,       // convert 0 to 1 and else to 0
260+    OP_0NOTEQUAL = 0x92, // return 0 if the input is 0, otherwise 1

sipa commented at 3:10 pm on July 20, 2023:

“return” is imprecise I think. How about Replace top stack item by "" if its value is 0, otherwise by "\x01"?

in src/script/script.h:174 in b20eacdd3b outdated

265+    OP_DIV = 0x96,    // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
266+    OP_MOD = 0x97,    // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
267+    OP_LSHIFT = 0x98, // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
268+    OP_RSHIFT = 0x99, // disabled, fail the script unconditionally (apply in an unexecuted conditional branch)
269+
270+    OP_BOOLAND = 0x9a,            // return 1 if both top two stack items are not 0, otherwise 0

sipa commented at 3:11 pm on July 20, 2023:

Pop two top stack items and push "\x01" if they are both true, "" otherwise (and similar things below).

in src/script/script.h:189 in b20eacdd3b outdated

290-    OP_CODESEPARATOR = 0xab,
291-    OP_CHECKSIG = 0xac,
292-    OP_CHECKSIGVERIFY = 0xad,
293-    OP_CHECKMULTISIG = 0xae,
294-    OP_CHECKMULTISIGVERIFY = 0xaf,
295+    OP_RIPEMD160 = 0xa6,           // hash input using RIPEMD-160

sipa commented at 3:12 pm on July 20, 2023:

What “input”? I suggest Replace top stack item by its RIPEMD-160 hash.

ChrisCho-H force-pushed on Jul 21, 2023

ChrisCho-H commented at 12:46 pm on July 21, 2023: none

Thx! I updated

vostrnad commented at 11:55 pm on July 21, 2023: none

The more I look at this (and especially after the switch to notation), the more I think the descriptions should each be on a separate line above the opcode in a block comment (/** ... */). It would allow for longer descriptions where needed without ever exceeding the soft 100-per-line character limit.

The block comments could now include (where appropriate):

Both a textual description and notation (frankly, I find the notation-only comments quite difficult to read).
Historical context for an opcode (when was it enabled/disabled).
Differences between tapscript and pre-tapscript.

Additionally, block comments above declarations are shown by many IDEs when hovering above a symbol, providing immediate access to documentation. Inline comments don’t do this.

I’ve drafted a few examples for these longer descriptions:

 0/**
 1 * Mark transaction as invalid if the top stack item is greater than
 2 * the transaction's locktime. The top stack item is not removed.
 3 * 
 4 * Previously OP_NOP2. Specified in BIP65 and deployed in December 2015
 5 * at block 388380 as a soft fork.
 6 */
 7OP_CHECKLOCKTIMEVERIFY = 0xb1,
 8/**
 9 * See OP_CHECKLOCKTIMEVERIFY.
10 */
11OP_NOP2 = OP_CHECKLOCKTIMEVERIFY,
12
13/**
14 * Mark transaction as invalid even when occurring in an unexecuted branch.
15 * 
16 * Previously concatenated two byte arrays. Disabled in August 2010.
17 * 
18 * Turned into OP_SUCCESS126 in tapscript.
19 */
20OP_CAT = 0x7e,
21
22/**
23 * Duplicate the top three stack items:
24 * 
25 * [ ... x0 x1 x2 ] -> [ ... x0 x1 x2 x0 x1 x2 ]
26 */
27OP_3DUP = 0x6f,

I understand this will make the section longer by a good few hundred lines, but if Bitcoin Core is to maintain in perpetuity a documentation for all opcodes, it should be at least of similar quality to the one at Bitcoin Wiki, and that is not achievable with short single-line descriptions.

ChrisCho-H requested review from sipa on Jul 26, 2023

Sjors commented at 12:22 pm on July 28, 2023: member

@vostrnad’s comment makes sense to me. I’ll hold off re-reviewing until that’s either done or decided against.

in src/script/script.h:169 in 4db63a5a7c outdated

260+    OP_ABS = 0x90,       // replace top stack item by its absolute value
261+    OP_NOT = 0x91,       // replace top stack item by "\x01" if its value is 0, otherwise ""
262+    OP_0NOTEQUAL = 0x92, // replace top stack item by "" if its value is 0, otherwise by "\x01"
263+
264+    OP_ADD = 0x93,    // pop two top stack items and push their sum
265+    OP_SUB = 0x94,    // pop two top stack items and push the second minus the top

RandyMcMillan commented at 1:38 pm on July 28, 2023:

pop two doesn’t quite work (IMO).

it makes the order somewhat ambiguous.

for example:

pop pop add push is more correct. no?

Being more explicit in this way will make the OP_SUB make more sense, no?.

pop pop subtract push

for OP_ADD it isn’t much of an issue.

1 + 1 = 2

1 + -1 = 0

-1 + 1 = 0

for OP_SUB it may be an issue.

1 - 1 = 0

1 - -1 = 0

-1 - 1 = -2

*in common arithmetic notation

ChrisCho-H commented at 9:14 am on August 4, 2023:

I understand what you intended but it does not seem to be interpreted in that way for pop two top stack items. pop pop add push is more confusing to me. I appreciate your feedback

in src/script/script.h:176 in 4db63a5a7c outdated

267+    OP_DIV = 0x96,    // mark transaction invalid even when occurring in an unexecuted branch
268+    OP_MOD = 0x97,    // mark transaction invalid even when occurring in an unexecuted branch
269+    OP_LSHIFT = 0x98, // mark transaction invalid even when occurring in an unexecuted branch
270+    OP_RSHIFT = 0x99, // mark transaction invalid even when occurring in an unexecuted branch
271+
272+    OP_BOOLAND = 0x9a,            // pop two top stack items and push "\x01" if they are both true, "" otherwise

RandyMcMillan commented at 1:40 pm on July 28, 2023:

see above.

better boolean notation here would be great.

RandyMcMillan commented at 1:46 pm on July 28, 2023: contributor

Concept ACK

ChrisCho-H force-pushed on Aug 4, 2023

DrahtBot added the label CI failed on Aug 4, 2023

ChrisCho-H force-pushed on Aug 4, 2023

ChrisCho-H commented at 9:47 am on August 4, 2023: none

Seems no one’s against @vostrnad suggestion, I convert all single lines to block comments, add textual description with notation and reference for OP_SUCCESS opcodes in case of tapscript. I didn’t include history much. In my opinion, description only for function itself looks clean and explicit which fits with main purpose of this.

Sjors commented at 10:21 am on August 4, 2023: member

Probably worth just squashing this down to 1 commit.

Indeed.

DrahtBot removed the label CI failed on Aug 4, 2023

script: add description for the functionality of each opcode 0bf5534576

ChrisCho-H force-pushed on Aug 4, 2023

achow101 requested review from Sjors on Sep 20, 2023

darosior commented at 5:39 pm on September 20, 2023: member

Leaning toward Concept NACK. I’m sorry for the time you’ve invested in this (hope you got something out of it) but i think this is not a good use of review time and prone to bikeshedding.

achow101 closed this on Sep 20, 2023

ChrisCho-H commented at 2:25 am on September 21, 2023: none

Understood. Thanks for everyone who put efforts into here tgh🙏

ChrisCho-H commented at 4:28 am on September 21, 2023: none

For those who might look for this, I just created repo for it here.

vostrnad commented at 10:52 pm on November 8, 2023: none

For anyone interested in documenting opcodes, there is now https://opcodeexplained.com/ (GitHub repo here).

bitcoin locked this on Nov 7, 2024

script: add description for the functionality of each opcode #27109

Reviews

Conflicts