Skip to content

Commit

Permalink
txscript: Cleanup and add tests for rshift opcode.
Browse files Browse the repository at this point in the history
This cleans up the code for handling the right shift opcode to
explicitly call out its semantics which are likely not otherwise obvious
as well as improve its readability.

It also adds several tests to the reference script tests which exercise
the semantics of the right shift opcode including both positive and
negative tests.
  • Loading branch information
davecgh committed Jun 13, 2018
1 parent f8d8dbc commit 0ee1468
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 12 deletions.
10 changes: 10 additions & 0 deletions txscript/data/script_invalid.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,16 @@
["2147483648 1", "LSHIFT TRUE", "P2SH,STRICTENC", "LSHIFT must fail with input value >4 bytes"],
["1 2147483648", "LSHIFT TRUE", "P2SH,STRICTENC", "LSHIFT must fail with shift count >4 bytes"],

["Right bit shift related test coverage"],
["", "RSHIFT NOT", "P2SH,STRICTENC", "RSHIFT requires an input value"],
["1", "RSHIFT TRUE", "P2SH,STRICTENC", "RSHIFT requires a shift count"],
["NOP 1", "RSHIFT NOT", "P2SH,STRICTENC", "RSHIFT input value must be numeric"],
["1 NOP", "RSHIFT TRUE", "P2SH,STRICTENC", "RSHIFT shift count must be numeric"],
["1 -1", "RSHIFT 2 EQUAL", "P2SH,STRICTENC", "RSHIFT must fail with negative shift count"],
["1 33", "RSHIFT 0 EQUAL", "P2SH,STRICTENC", "RSHIFT must fail with shift count >32"],
["2147483648 1", "RSHIFT TRUE", "P2SH,STRICTENC", "RSHIFT must fail with input value >4 bytes"],
["1 2147483648", "RSHIFT TRUE", "P2SH,STRICTENC", "RSHIFT must fail with shift count >4 bytes"],

["1", "IF 0x50 ENDIF 1", "P2SH,STRICTENC", "0x50 is reserved"],
["0x52", "0x5f ADD 0x60 EQUAL", "P2SH,STRICTENC", "0x51 through 0x60 push 1 through 16 onto stack"],
["0","NOP", "P2SH,STRICTENC"],
Expand Down
41 changes: 40 additions & 1 deletion txscript/data/script_valid.json
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@
["2 DUP MUL", "4 EQUAL", "P2SH,STRICTENC"],
["2 DUP DIV", "1 EQUAL", "P2SH,STRICTENC"],
["7 3 MOD", "1 EQUAL", "P2SH,STRICTENC"],
["2 1 RSHIFT", "1 EQUAL", "P2SH,STRICTENC"],

["Left bit shift related test coverage"],
["1 1", "LSHIFT 2 EQUAL", "P2SH,STRICTENC", "LSHIFT 0x1 << 1"],
Expand Down Expand Up @@ -191,6 +190,46 @@
["-1431655766 1", "LSHIFT 1431655764 EQUAL", "P2SH,STRICTENC", "LSHIFT 0xaaaaaaaa << 1"],
["1431655765 1", "LSHIFT -1431655766 EQUAL", "P2SH,STRICTENC", "LSHIFT 0x55555555 << 1"],

["Right bit shift related test coverage"],
["1073741825 1", "RSHIFT 536870912 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 1"],
["1073741825 2", "RSHIFT 268435456 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 2"],
["1073741825 3", "RSHIFT 134217728 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 3"],
["1073741825 4", "RSHIFT 67108864 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 4"],
["1073741825 5", "RSHIFT 33554432 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 5"],
["1073741825 6", "RSHIFT 16777216 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 6"],
["1073741825 7", "RSHIFT 8388608 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 7"],
["1073741825 8", "RSHIFT 4194304 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 8"],
["1073741825 9", "RSHIFT 2097152 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 9"],
["1073741825 10", "RSHIFT 1048576 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 10"],
["1073741825 11", "RSHIFT 524288 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 11"],
["1073741825 12", "RSHIFT 262144 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 12"],
["1073741825 13", "RSHIFT 131072 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 13"],
["1073741825 14", "RSHIFT 65536 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 14"],
["1073741825 15", "RSHIFT 32768 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 15"],
["1073741825 16", "RSHIFT 16384 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 16"],
["1073741825 17", "RSHIFT 8192 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 17"],
["1073741825 18", "RSHIFT 4096 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 18"],
["1073741825 19", "RSHIFT 2048 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 19"],
["1073741825 20", "RSHIFT 1024 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 20"],
["1073741825 21", "RSHIFT 512 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 21"],
["1073741825 22", "RSHIFT 256 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 22"],
["1073741825 23", "RSHIFT 128 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 23"],
["1073741825 24", "RSHIFT 64 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 24"],
["1073741825 25", "RSHIFT 32 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 25"],
["1073741825 26", "RSHIFT 16 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 26"],
["1073741825 27", "RSHIFT 8 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 27"],
["1073741825 28", "RSHIFT 4 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 28"],
["1073741825 29", "RSHIFT 2 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 29"],
["1073741825 30", "RSHIFT 1 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 30"],
["1073741825 31", "RSHIFT 0 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x40000001 >> 31"],
["-1 1", "RSHIFT -1 EQUAL", "P2SH,STRICTENC", "RSHIFT of negative number must shift 1s into the high bits (0xffffffff >> 1)"],
["-1 2", "RSHIFT -1 EQUAL", "P2SH,STRICTENC", "RSHIFT of negative number must shift 1s into the high bits (0xffffffff >> 2)"],
["-1 3", "RSHIFT -1 EQUAL", "P2SH,STRICTENC", "RSHIFT of negative number must shift 1s into the high bits (0xffffffff >> 3)"],
["-2147483647 1", "RSHIFT -1073741824 EQUAL", "P2SH,STRICTENC", "RSHIFT of negative number must shift 1s into the high bits (0x80000001 >> 1)"],
["-2147483647 3", "RSHIFT -268435456 EQUAL", "P2SH,STRICTENC", "RSHIFT of negative number must shift 1s into the high bits (0x80000001 >> 3)"],
["-1431655766 1", "RSHIFT -715827883 EQUAL", "P2SH,STRICTENC", "RSHIFT of negative number must shit 1s into the high bits (0xaaaaaaaa >> 1)"],
["1431655765 1", "RSHIFT 715827882 EQUAL", "P2SH,STRICTENC", "RSHIFT 0x55555555 >> 1"],

["0x4c 0x00","0 EQUAL", "P2SH,STRICTENC"],
["0x4d 0x0000","0 EQUAL", "P2SH,STRICTENC"],
["0x4e 0x00000000","0 EQUAL", "P2SH,STRICTENC"],
Expand Down
35 changes: 24 additions & 11 deletions txscript/opcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -2038,34 +2038,47 @@ func opcodeLShift(op *parsedOpcode, vm *Engine) error {
return nil
}

// opcodeRShift pushes the top two items off the stack as integers. Both ints are
// interpreted as int32s. The first item becomes the depth to shift right, while
// the second item is shifted that depth to the right. The shifted item is pushed
// back to the stack as an integer.
// Stack transformation: [... x1 x2] -> [... x1 << x2]
// opcodeRShift treats the top two items of the data stack as 32-bit integers
// where the top item represents the number of bits to right shift (up to 32),
// and the second item represents the value to shift, and replaces them both
// with the result of the shift. Since the value to shift is treated as a
// signed 32-bit integer, negative values right shifted will be sign extended.
//
// Stack transformation: [... x1 x2] -> [... x1 >> x2]
func opcodeRShift(op *parsedOpcode, vm *Engine) error {
// WARNING: Since scriptNums are signed, a standard 4-byte scriptNum only
// supports up to a maximum of 2^31-1. The value (v1) really should allow
// 5-byte scriptNums and have an overflow check later to clamp it to uint32,
// so the full range of uint32 could be covered. This has undesirable
// consequences on the semantics of right shift such that attempting to
// do ((0x40000000 << 1) >> 1) will fail due to the first shift producing
// a value greater than the max int32.
//
// Unfortunately, a 4-byte scriptNum is now part of consensus, so changing
// it requires a consensus vote.
v0, err := vm.dstack.PopInt(mathOpCodeMaxScriptNumLen) // x2
if err != nil {
return err
}

v1, err := vm.dstack.PopInt(mathOpCodeMaxScriptNumLen) // x1
if err != nil {
return err
}

v032 := v0.Int32()
v132 := v1.Int32()
// The count and value are limited to int32 via the above, so it is safe to
// cast them.
count := v0.Int32()
value := v1.Int32()

// Don't allow invalid or pointless shifts.
if v032 < 0 {
if count < 0 {
return ErrNegativeShift
}
if v032 > 32 {
if count > 32 {
return ErrShiftOverflow
}

vm.dstack.PushInt(scriptNum(v132 >> uint(v032)))
vm.dstack.PushInt(scriptNum(value >> uint(count)))
return nil
}

Expand Down

0 comments on commit 0ee1468

Please sign in to comment.