From ceb073cd6bef6516fa83a074ee587cf882829d19 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Tue, 19 Jul 2022 18:49:48 -0400 Subject: [PATCH 01/14] introduce decodeCompactUint32 function --- pkg/scale/decode.go | 61 +++++++++++++++++++++++++++++++++++- pkg/scale/decode_test.go | 67 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index 4e427e1b35..62964a233c 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "math" "math/big" "reflect" ) @@ -523,10 +524,68 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { return } +var errorU32OutOfRange = errors.New("uint32 out of range") + +// decodeCompactUint32 will decode compact unsigned 32-bit integer +func (ds *decodeState) decodeCompactUint32(dstv reflect.Value) (err error) { + b, err := ds.ReadByte() + if err != nil { + return + } + + in := dstv.Interface() + temp := reflect.New(reflect.TypeOf(in)) + // check mode of encoding, stored at 2 least significant bits + mode := b & 3 + switch { + case mode <= 2: + var val int64 + val, err = ds.decodeSmallInt(b, mode) + if err != nil { + return + } + if mode == 1 { + if !(val > 0b0011_1111 && val <= 0b0011_1111_1111_1111) { + return errorU32OutOfRange + } + } + if mode == 2 { + if !(val > 0b0011_1111_1111_1111 && val <= math.MaxUint32>>2) { + return errorU32OutOfRange + } + } + temp.Elem().Set(reflect.ValueOf(val).Convert(reflect.TypeOf(in))) + dstv.Set(temp.Elem()) + default: + // >4 byte mode + topSixBits := b >> 2 + if mode == 3 && topSixBits != 0 { + return errorU32OutOfRange + } + byteLen := uint(topSixBits) + 4 + + buf := make([]byte, byteLen) + _, err = ds.Read(buf) + if err != nil { + return + } + + var o uint64 + if byteLen == 4 { + o = uint64(binary.LittleEndian.Uint32(buf)) + } + if !(o > math.MaxUint32>>2) { + return errorU32OutOfRange + } + dstv.Set(reflect.ValueOf(o).Convert(reflect.TypeOf(in))) + } + return +} + // decodeLength is helper method which calls decodeUint and casts to int func (ds *decodeState) decodeLength() (l int, err error) { dstv := reflect.New(reflect.TypeOf(l)) - err = ds.decodeUint(dstv.Elem()) + err = ds.decodeCompactUint32(dstv.Elem()) if err != nil { return } diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 669bddb3a0..7a0c7cbd4a 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -302,3 +302,70 @@ func Test_Decoder_Decode_MultipleCalls(t *testing.T) { }) } } + +var ( + decodeUint32Tests = tests{ + { + name: "int(1) mode 0", + in: uint32(1), + want: []byte{0x04}, + }, + { + name: "int(16383) mode 1", + in: int(16383), + want: []byte{0xfd, 0xff}, + }, + { + name: "int(1073741823) mode 2", + in: int(1073741823), + want: []byte{0xfe, 0xff, 0xff, 0xff}, + }, + { + name: "int(4294967295) mode 3", + in: int(4294967295), + want: []byte{0x3, 0xff, 0xff, 0xff, 0xff}, + }, + { + name: "myCustomInt(9223372036854775807) out of range", + in: myCustomInt(0), + want: []byte{19, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}, + wantErr: true, + }, + { + name: "uint(overload)", + in: int(0), + want: []byte{0x07, 0x08, 0x09, 0x10, 0x0, 0x40}, + wantErr: true, + }, + { + name: "uint(16384) mode 2", + in: int(16384), + want: []byte{0x02, 0x00, 0x01, 0x0}, + }, + } +) + +func Test_decodeState_decodeUInt32(t *testing.T) { + for _, tt := range decodeUint32Tests { + t.Run(tt.name, func(t *testing.T) { + dst := reflect.New(reflect.TypeOf(tt.in)).Elem().Interface() + dstv := reflect.ValueOf(&dst) + elem := indirect(dstv) + + buf := &bytes.Buffer{} + ds := decodeState{} + _, err := buf.Write(tt.want) + if err != nil { + return + } + ds.Reader = buf + err = ds.decodeCompactUint32(elem) + if (err != nil) != tt.wantErr { + t.Errorf("decodeState.decodeCompactUint32 error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(dst, tt.in) { + t.Errorf("decodeState.decodeCompactUint32 = %v, want %v", dst, tt.in) + } + }) + } +} From 1ea546d2d377bfc153bd2a0f220e12fedee75b59 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Wed, 20 Jul 2022 10:15:50 -0400 Subject: [PATCH 02/14] replace use of int64 with uint32 --- pkg/scale/decode.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index 62964a233c..7df0198a58 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -570,9 +570,9 @@ func (ds *decodeState) decodeCompactUint32(dstv reflect.Value) (err error) { return } - var o uint64 + var o uint32 if byteLen == 4 { - o = uint64(binary.LittleEndian.Uint32(buf)) + o = binary.LittleEndian.Uint32(buf) } if !(o > math.MaxUint32>>2) { return errorU32OutOfRange From 3c79993d74ddd34da989c4dadbc136db2d861213 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Wed, 20 Jul 2022 12:06:09 -0400 Subject: [PATCH 03/14] update node decode_test --- internal/trie/node/decode_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/trie/node/decode_test.go b/internal/trie/node/decode_test.go index 31b3e4c102..ccda8fda2b 100644 --- a/internal/trie/node/decode_test.go +++ b/internal/trie/node/decode_test.go @@ -333,7 +333,7 @@ func Test_decodeLeaf(t *testing.T) { variant: leafVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeValue, - errMessage: "cannot decode value: could not decode invalid integer", + errMessage: "cannot decode value: uint32 out of range", }, "zero value": { reader: bytes.NewBuffer([]byte{ From ea5836adb08acee85a46bd5dae0d0fb8c2c2660a Mon Sep 17 00:00:00 2001 From: edwardmack Date: Fri, 22 Jul 2022 14:41:20 -0400 Subject: [PATCH 04/14] refactor decodeUint to conform with parity scale codec --- internal/trie/node/decode_test.go | 6 +- pkg/scale/decode.go | 133 +++++++++++------------------- pkg/scale/decode_test.go | 24 +++--- pkg/scale/encode_test.go | 21 +++-- 4 files changed, 79 insertions(+), 105 deletions(-) diff --git a/internal/trie/node/decode_test.go b/internal/trie/node/decode_test.go index ccda8fda2b..082093d8dc 100644 --- a/internal/trie/node/decode_test.go +++ b/internal/trie/node/decode_test.go @@ -166,7 +166,7 @@ func Test_decodeBranch(t *testing.T) { variant: branchVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeChildHash, - errMessage: "cannot decode child hash: at index 10: EOF", + errMessage: "cannot decode child hash: at index 10: reading from buffer: EOF", }, "success for branch variant": { reader: bytes.NewBuffer( @@ -203,7 +203,7 @@ func Test_decodeBranch(t *testing.T) { variant: branchWithValueVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeValue, - errMessage: "cannot decode value: EOF", + errMessage: "cannot decode value: reading from buffer: EOF", }, "success for branch with value": { reader: bytes.NewBuffer(concatByteSlices([][]byte{ @@ -333,7 +333,7 @@ func Test_decodeLeaf(t *testing.T) { variant: leafVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeValue, - errMessage: "cannot decode value: uint32 out of range", + errMessage: "cannot decode value: unexpected prefix decoding compact uint: 255", }, "zero value": { reader: bytes.NewBuffer([]byte{ diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index 7df0198a58..e835bded60 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "io" - "math" "math/big" "reflect" ) @@ -479,113 +478,81 @@ func (ds *decodeState) decodeBool(dstv reflect.Value) (err error) { // decodeUint will decode unsigned integer func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { - b, err := ds.ReadByte() + const maxUint32 = ^uint32(0) + const maxUint64 = ^uint64(0) + prefix, err := ds.ReadByte() if err != nil { - return + return fmt.Errorf("reading from buffer: %w", err) } in := dstv.Interface() temp := reflect.New(reflect.TypeOf(in)) // check mode of encoding, stored at 2 least significant bits - mode := b & 3 - switch { - case mode <= 2: - var val int64 - val, err = ds.decodeSmallInt(b, mode) - if err != nil { - return - } - temp.Elem().Set(reflect.ValueOf(val).Convert(reflect.TypeOf(in))) - dstv.Set(temp.Elem()) - default: - // >4 byte mode - topSixBits := b >> 2 - byteLen := uint(topSixBits) + 4 - - buf := make([]byte, byteLen) - _, err = ds.Read(buf) + mode := prefix % 4 + var value uint + switch mode { + case 0: + value = uint(prefix >> 2) + case 1: + var buf byte + buf, err = ds.ReadByte() if err != nil { - return + return fmt.Errorf("reading from buffer: %w", err) } - - var o uint64 - if byteLen == 4 { - o = uint64(binary.LittleEndian.Uint32(buf)) - } else if byteLen > 4 && byteLen <= 8 { - tmp := make([]byte, 8) - copy(tmp, buf) - o = binary.LittleEndian.Uint64(tmp) - } else { - err = errors.New("could not decode invalid integer") - return + value = uint(binary.LittleEndian.Uint16([]byte{prefix, buf}) >> 2) + if value <= 0b0011_1111 || value > 0b0111_1111_1111_1111 { + return ErrU64OutOfRange } - dstv.Set(reflect.ValueOf(o).Convert(reflect.TypeOf(in))) - } - return -} - -var errorU32OutOfRange = errors.New("uint32 out of range") - -// decodeCompactUint32 will decode compact unsigned 32-bit integer -func (ds *decodeState) decodeCompactUint32(dstv reflect.Value) (err error) { - b, err := ds.ReadByte() - if err != nil { - return - } - - in := dstv.Interface() - temp := reflect.New(reflect.TypeOf(in)) - // check mode of encoding, stored at 2 least significant bits - mode := b & 3 - switch { - case mode <= 2: - var val int64 - val, err = ds.decodeSmallInt(b, mode) + case 2: + buf := make([]byte, 3) + _, err = ds.Read(buf) if err != nil { - return - } - if mode == 1 { - if !(val > 0b0011_1111 && val <= 0b0011_1111_1111_1111) { - return errorU32OutOfRange - } - } - if mode == 2 { - if !(val > 0b0011_1111_1111_1111 && val <= math.MaxUint32>>2) { - return errorU32OutOfRange - } + break } - temp.Elem().Set(reflect.ValueOf(val).Convert(reflect.TypeOf(in))) - dstv.Set(temp.Elem()) - default: - // >4 byte mode - topSixBits := b >> 2 - if mode == 3 && topSixBits != 0 { - return errorU32OutOfRange + value = uint(binary.LittleEndian.Uint32(append([]byte{prefix}, buf...)) >> 2) + if value <= 0b0011_1111_1111_1111 || value > uint(maxUint32>>2) { + return ErrU64OutOfRange } - byteLen := uint(topSixBits) + 4 - + case 3: + byteLen := (prefix >> 2) + 4 buf := make([]byte, byteLen) _, err = ds.Read(buf) if err != nil { - return + return fmt.Errorf("reading from buffer: %w", err) } + switch byteLen { + case 4: + value = uint(binary.LittleEndian.Uint32(buf)) + if value <= uint(maxUint32>>2) { + return ErrU64OutOfRange + } + case 8: + uintSize := 32 << (^uint(0) >> 32 & 1) + if uintSize == 32 { + return fmt.Errorf("uint64 is not supported") + } + tmp := make([]byte, 8) + copy(tmp, buf) + value = uint(binary.LittleEndian.Uint64(tmp)) + if value <= uint(maxUint64>>8) { + return ErrU64OutOfRange + } + default: + return fmt.Errorf("unexpected prefix decoding compact uint: %d", prefix) - var o uint32 - if byteLen == 4 { - o = binary.LittleEndian.Uint32(buf) - } - if !(o > math.MaxUint32>>2) { - return errorU32OutOfRange } - dstv.Set(reflect.ValueOf(o).Convert(reflect.TypeOf(in))) } + temp.Elem().Set(reflect.ValueOf(value).Convert(reflect.TypeOf(in))) + dstv.Set(temp.Elem()) return } +var ErrU64OutOfRange = errors.New("uint64 out of range") + // decodeLength is helper method which calls decodeUint and casts to int func (ds *decodeState) decodeLength() (l int, err error) { dstv := reflect.New(reflect.TypeOf(l)) - err = ds.decodeCompactUint32(dstv.Elem()) + err = ds.decodeUint(dstv.Elem()) if err != nil { return } diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 7a0c7cbd4a..12b55ff322 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/assert" ) func Test_decodeState_decodeFixedWidthInt(t *testing.T) { @@ -303,8 +304,8 @@ func Test_Decoder_Decode_MultipleCalls(t *testing.T) { } } -var ( - decodeUint32Tests = tests{ +func Test_decodeState_decodeUint(t *testing.T) { + decodeUint32Tests := tests{ { name: "int(1) mode 0", in: uint32(1), @@ -326,10 +327,9 @@ var ( want: []byte{0x3, 0xff, 0xff, 0xff, 0xff}, }, { - name: "myCustomInt(9223372036854775807) out of range", - in: myCustomInt(0), - want: []byte{19, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}, - wantErr: true, + name: "myCustomInt(9223372036854775807) mode 3, 64bit", + in: myCustomInt(9223372036854775807), + want: []byte{19, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}, }, { name: "uint(overload)", @@ -343,11 +343,11 @@ var ( want: []byte{0x02, 0x00, 0x01, 0x0}, }, } -) - -func Test_decodeState_decodeUInt32(t *testing.T) { + t.Parallel() for _, tt := range decodeUint32Tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() dst := reflect.New(reflect.TypeOf(tt.in)).Elem().Interface() dstv := reflect.ValueOf(&dst) elem := indirect(dstv) @@ -355,11 +355,9 @@ func Test_decodeState_decodeUInt32(t *testing.T) { buf := &bytes.Buffer{} ds := decodeState{} _, err := buf.Write(tt.want) - if err != nil { - return - } + assert.NoError(t, err) ds.Reader = buf - err = ds.decodeCompactUint32(elem) + err = ds.decodeUint(elem) if (err != nil) != tt.wantErr { t.Errorf("decodeState.decodeCompactUint32 error = %v, wantErr %v", err, tt.wantErr) } diff --git a/pkg/scale/encode_test.go b/pkg/scale/encode_test.go index 4d56b40e77..fd0d6bc13d 100644 --- a/pkg/scale/encode_test.go +++ b/pkg/scale/encode_test.go @@ -176,6 +176,11 @@ var ( in: int(1), want: []byte{0x04}, }, + { + name: "int(42)", + in: int(42), + want: []byte{0xa8}, + }, { name: "int(16383)", in: int(16383), @@ -820,9 +825,11 @@ var ( want: []byte{0x10, 0x03, 0x00, 0x00, 0x00, 0x40, 0x08, 0x0c, 0x10}, }, { - name: "[]int{1 << 32, 2, 3, 1 << 32}", - in: []int{1 << 32, 2, 3, 1 << 32}, - want: []byte{0x10, 0x07, 0x00, 0x00, 0x00, 0x00, 0x01, 0x08, 0x0c, 0x07, 0x00, 0x00, 0x00, 0x00, 0x01}, + name: "[]int64{1 << 32, 2, 3, 1 << 32}", + in: []int64{1 << 32, 2, 3, 1 << 32}, + want: []byte{0x10, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, + 0x00}, }, { name: "[]bool{true, false, true}", @@ -863,9 +870,11 @@ var ( want: []byte{0x03, 0x00, 0x00, 0x00, 0x40, 0x08, 0x0c, 0x10}, }, { - name: "[4]int{1 << 32, 2, 3, 1 << 32}", - in: [4]int{1 << 32, 2, 3, 1 << 32}, - want: []byte{0x07, 0x00, 0x00, 0x00, 0x00, 0x01, 0x08, 0x0c, 0x07, 0x00, 0x00, 0x00, 0x00, 0x01}, + name: "[4]int64{1 << 32, 2, 3, 1 << 32}", + in: [4]int64{1 << 32, 2, 3, 1 << 32}, + want: []byte{0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, + 0x00}, }, { name: "[3]bool{true, false, true}", From a289eceefc83b6e9669a3afada99fa5115e25a05 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Fri, 22 Jul 2022 15:04:14 -0400 Subject: [PATCH 05/14] add test for changed []int encodings --- pkg/scale/decode_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 12b55ff322..1158b9300d 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -342,6 +342,17 @@ func Test_decodeState_decodeUint(t *testing.T) { in: int(16384), want: []byte{0x02, 0x00, 0x01, 0x0}, }, + { + name: "[]int{1 << 32, 2, 3, 1 << 32}", + in: uint(4), + want: []byte{0x10, 0x07, 0x00, 0x00, 0x00, 0x00, 0x01, 0x08, 0x0c, 0x07, 0x00, 0x00, 0x00, 0x00, 0x01}, + }, + { + name: "[4]int{1 << 32, 2, 3, 1 << 32}", + in: [4]int{0, 0, 0, 0}, + want: []byte{0x07, 0x00, 0x00, 0x00, 0x00, 0x01, 0x08, 0x0c, 0x07, 0x00, 0x00, 0x00, 0x00, 0x01}, + wantErr: true, + }, } t.Parallel() for _, tt := range decodeUint32Tests { @@ -359,10 +370,10 @@ func Test_decodeState_decodeUint(t *testing.T) { ds.Reader = buf err = ds.decodeUint(elem) if (err != nil) != tt.wantErr { - t.Errorf("decodeState.decodeCompactUint32 error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("decodeState.decodeUint error = %v, wantErr %v", err, tt.wantErr) } if !reflect.DeepEqual(dst, tt.in) { - t.Errorf("decodeState.decodeCompactUint32 = %v, want %v", dst, tt.in) + t.Errorf("decodeState.decodeUint = %v, want %v", dst, tt.in) } }) } From 8b77f71924a71d33f9ff7f9f472bd94e6937c3f1 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Mon, 25 Jul 2022 15:02:13 -0400 Subject: [PATCH 06/14] add error --- pkg/scale/decode.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index e835bded60..e8621f798b 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -507,7 +507,7 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { buf := make([]byte, 3) _, err = ds.Read(buf) if err != nil { - break + return fmt.Errorf("reading from buffer: %w", err) } value = uint(binary.LittleEndian.Uint32(append([]byte{prefix}, buf...)) >> 2) if value <= 0b0011_1111_1111_1111 || value > uint(maxUint32>>2) { From 9f53087a37e4de23b4becc03408b8ba8f6d53b0a Mon Sep 17 00:00:00 2001 From: edwardmack Date: Wed, 27 Jul 2022 10:09:59 -0400 Subject: [PATCH 07/14] update error checking and messages --- pkg/scale/decode.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index e8621f798b..a8c9f31d4e 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -489,19 +489,18 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { temp := reflect.New(reflect.TypeOf(in)) // check mode of encoding, stored at 2 least significant bits mode := prefix % 4 - var value uint + var value uint64 switch mode { case 0: - value = uint(prefix >> 2) + value = uint64(prefix >> 2) case 1: - var buf byte - buf, err = ds.ReadByte() + buf, err := ds.ReadByte() if err != nil { return fmt.Errorf("reading from buffer: %w", err) } - value = uint(binary.LittleEndian.Uint16([]byte{prefix, buf}) >> 2) + value = uint64(binary.LittleEndian.Uint16([]byte{prefix, buf}) >> 2) if value <= 0b0011_1111 || value > 0b0111_1111_1111_1111 { - return ErrU64OutOfRange + return fmt.Errorf("%w: %d (%b)", ErrU16OutOfRange, value, value) } case 2: buf := make([]byte, 3) @@ -509,9 +508,9 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { if err != nil { return fmt.Errorf("reading from buffer: %w", err) } - value = uint(binary.LittleEndian.Uint32(append([]byte{prefix}, buf...)) >> 2) - if value <= 0b0011_1111_1111_1111 || value > uint(maxUint32>>2) { - return ErrU64OutOfRange + value = uint64(binary.LittleEndian.Uint32(append([]byte{prefix}, buf...)) >> 2) + if value <= 0b0011_1111_1111_1111 || value > uint64(maxUint32>>2) { + return fmt.Errorf("%w: %d (%b)", ErrU32OutOfRange, value, value) } case 3: byteLen := (prefix >> 2) + 4 @@ -522,19 +521,19 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { } switch byteLen { case 4: - value = uint(binary.LittleEndian.Uint32(buf)) - if value <= uint(maxUint32>>2) { - return ErrU64OutOfRange + value = uint64(binary.LittleEndian.Uint32(buf)) + if value <= uint64(maxUint32>>2) { + return ErrU32OutOfRange } case 8: uintSize := 32 << (^uint(0) >> 32 & 1) if uintSize == 32 { - return fmt.Errorf("uint64 is not supported") + return ErrU64NotSupported } tmp := make([]byte, 8) copy(tmp, buf) - value = uint(binary.LittleEndian.Uint64(tmp)) - if value <= uint(maxUint64>>8) { + value = binary.LittleEndian.Uint64(tmp) + if value <= maxUint64>>8 { return ErrU64OutOfRange } default: @@ -547,7 +546,10 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { return } +var ErrU16OutOfRange = errors.New("uint16 out of range") +var ErrU32OutOfRange = errors.New("uint32 out of range") var ErrU64OutOfRange = errors.New("uint64 out of range") +var ErrU64NotSupported = errors.New("uint64 is not supported") // decodeLength is helper method which calls decodeUint and casts to int func (ds *decodeState) decodeLength() (l int, err error) { From 9787bd39b83b150e085b440b0a4acf2ec171572a Mon Sep 17 00:00:00 2001 From: edwardmack Date: Wed, 27 Jul 2022 15:57:02 -0400 Subject: [PATCH 08/14] update decodeLength to return uint --- pkg/scale/decode.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index a8c9f31d4e..fb2aed23ae 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -335,7 +335,7 @@ func (ds *decodeState) decodeVaryingDataTypeSlice(dstv reflect.Value) (err error if err != nil { return } - for i := 0; i < l; i++ { + for i := uint(0); i < l; i++ { vdt := vdts.VaryingDataType vdtv := reflect.New(reflect.TypeOf(vdt)) vdtv.Elem().Set(reflect.ValueOf(vdt)) @@ -397,7 +397,7 @@ func (ds *decodeState) decodeSlice(dstv reflect.Value) (err error) { } in := dstv.Interface() temp := reflect.New(reflect.ValueOf(in).Type()) - for i := 0; i < l; i++ { + for i := uint(0); i < l; i++ { tempElemType := reflect.TypeOf(in).Elem() tempElem := reflect.New(tempElemType).Elem() @@ -552,13 +552,13 @@ var ErrU64OutOfRange = errors.New("uint64 out of range") var ErrU64NotSupported = errors.New("uint64 is not supported") // decodeLength is helper method which calls decodeUint and casts to int -func (ds *decodeState) decodeLength() (l int, err error) { +func (ds *decodeState) decodeLength() (l uint, err error) { dstv := reflect.New(reflect.TypeOf(l)) err = ds.decodeUint(dstv.Elem()) if err != nil { return } - l = dstv.Elem().Interface().(int) + l = dstv.Elem().Interface().(uint) return } From 6a542d4d373f6e4eaa592f9ed0daec538c1b12f9 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Mon, 22 Aug 2022 14:52:10 -0400 Subject: [PATCH 09/14] address PR comments --- pkg/scale/decode.go | 27 +++++++++++++++------------ pkg/scale/decode_test.go | 3 ++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pkg/scale/decode.go b/pkg/scale/decode.go index fb2aed23ae..e10cf4c4e6 100644 --- a/pkg/scale/decode.go +++ b/pkg/scale/decode.go @@ -482,7 +482,7 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { const maxUint64 = ^uint64(0) prefix, err := ds.ReadByte() if err != nil { - return fmt.Errorf("reading from buffer: %w", err) + return fmt.Errorf("reading byte: %w", err) } in := dstv.Interface() @@ -496,7 +496,7 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { case 1: buf, err := ds.ReadByte() if err != nil { - return fmt.Errorf("reading from buffer: %w", err) + return fmt.Errorf("reading byte: %w", err) } value = uint64(binary.LittleEndian.Uint16([]byte{prefix, buf}) >> 2) if value <= 0b0011_1111 || value > 0b0111_1111_1111_1111 { @@ -506,7 +506,7 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { buf := make([]byte, 3) _, err = ds.Read(buf) if err != nil { - return fmt.Errorf("reading from buffer: %w", err) + return fmt.Errorf("reading bytes: %w", err) } value = uint64(binary.LittleEndian.Uint32(append([]byte{prefix}, buf...)) >> 2) if value <= 0b0011_1111_1111_1111 || value > uint64(maxUint32>>2) { @@ -517,16 +517,16 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { buf := make([]byte, byteLen) _, err = ds.Read(buf) if err != nil { - return fmt.Errorf("reading from buffer: %w", err) + return fmt.Errorf("reading bytes: %w", err) } switch byteLen { case 4: value = uint64(binary.LittleEndian.Uint32(buf)) if value <= uint64(maxUint32>>2) { - return ErrU32OutOfRange + return fmt.Errorf("%w: %d (%b)", ErrU32OutOfRange, value, value) } case 8: - uintSize := 32 << (^uint(0) >> 32 & 1) + const uintSize = 32 << (^uint(0) >> 32 & 1) if uintSize == 32 { return ErrU64NotSupported } @@ -534,10 +534,10 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { copy(tmp, buf) value = binary.LittleEndian.Uint64(tmp) if value <= maxUint64>>8 { - return ErrU64OutOfRange + return fmt.Errorf("%w: %d (%b)", ErrU64OutOfRange, value, value) } default: - return fmt.Errorf("unexpected prefix decoding compact uint: %d", prefix) + return fmt.Errorf("%w: %d", ErrCompactUintPrefixUnknown, prefix) } } @@ -546,10 +546,13 @@ func (ds *decodeState) decodeUint(dstv reflect.Value) (err error) { return } -var ErrU16OutOfRange = errors.New("uint16 out of range") -var ErrU32OutOfRange = errors.New("uint32 out of range") -var ErrU64OutOfRange = errors.New("uint64 out of range") -var ErrU64NotSupported = errors.New("uint64 is not supported") +var ( + ErrU16OutOfRange = errors.New("uint16 out of range") + ErrU32OutOfRange = errors.New("uint32 out of range") + ErrU64OutOfRange = errors.New("uint64 out of range") + ErrU64NotSupported = errors.New("uint64 is not supported") + ErrCompactUintPrefixUnknown = errors.New("unknown prefix for compact uint") +) // decodeLength is helper method which calls decodeUint and casts to int func (ds *decodeState) decodeLength() (l uint, err error) { diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 1158b9300d..7c8d7d18f7 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -305,6 +305,7 @@ func Test_Decoder_Decode_MultipleCalls(t *testing.T) { } func Test_decodeState_decodeUint(t *testing.T) { + t.Parallel() decodeUint32Tests := tests{ { name: "int(1) mode 0", @@ -354,7 +355,7 @@ func Test_decodeState_decodeUint(t *testing.T) { wantErr: true, }, } - t.Parallel() + for _, tt := range decodeUint32Tests { tt := tt t.Run(tt.name, func(t *testing.T) { From 6494fe290050cb8b73840dd4c6afc08f4aff2d5d Mon Sep 17 00:00:00 2001 From: edwardmack Date: Mon, 22 Aug 2022 15:24:29 -0400 Subject: [PATCH 10/14] add to unit tests --- pkg/scale/decode_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 7c8d7d18f7..259d72944f 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -343,6 +343,30 @@ func Test_decodeState_decodeUint(t *testing.T) { in: int(16384), want: []byte{0x02, 0x00, 0x01, 0x0}, }, + { + name: "uint(0) mode 1, error", + in: int(0), + want: []byte{0x01, 0x00}, + wantErr: true, + }, + { + name: "uint(0) mode 2, error", + in: int(0), + want: []byte{0x02, 0x00, 0x00, 0x0}, + wantErr: true, + }, + { + name: "uint(0) mode 3, error", + in: int(0), + want: []byte{0x03, 0x00, 0x00, 0x0}, + wantErr: true, + }, + { + name: "mode 3, 64bit, error", + in: int(0), + want: []byte{19, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + wantErr: true, + }, { name: "[]int{1 << 32, 2, 3, 1 << 32}", in: uint(4), From f11c61b0390bf9786ad5492752f5b66860be4ca7 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Mon, 22 Aug 2022 15:50:47 -0400 Subject: [PATCH 11/14] fix unit tests to match error messages --- internal/trie/node/decode_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/trie/node/decode_test.go b/internal/trie/node/decode_test.go index 082093d8dc..b81be55ea1 100644 --- a/internal/trie/node/decode_test.go +++ b/internal/trie/node/decode_test.go @@ -166,7 +166,7 @@ func Test_decodeBranch(t *testing.T) { variant: branchVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeChildHash, - errMessage: "cannot decode child hash: at index 10: reading from buffer: EOF", + errMessage: "cannot decode child hash: at index 10: reading byte: EOF", }, "success for branch variant": { reader: bytes.NewBuffer( @@ -203,7 +203,7 @@ func Test_decodeBranch(t *testing.T) { variant: branchWithValueVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeValue, - errMessage: "cannot decode value: reading from buffer: EOF", + errMessage: "cannot decode value: reading byte: EOF", }, "success for branch with value": { reader: bytes.NewBuffer(concatByteSlices([][]byte{ @@ -333,7 +333,7 @@ func Test_decodeLeaf(t *testing.T) { variant: leafVariant.bits, partialKeyLength: 1, errWrapped: ErrDecodeValue, - errMessage: "cannot decode value: unexpected prefix decoding compact uint: 255", + errMessage: "cannot decode value: unknown prefix for compact uint: 255", }, "zero value": { reader: bytes.NewBuffer([]byte{ From 3d5f6875a48f0a27b8df9c01b6adcdb73712d3c5 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Mon, 22 Aug 2022 16:23:08 -0400 Subject: [PATCH 12/14] update error messages to match tests --- lib/runtime/version_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/runtime/version_test.go b/lib/runtime/version_test.go index 412941995e..88050b7b5e 100644 --- a/lib/runtime/version_test.go +++ b/lib/runtime/version_test.go @@ -39,7 +39,7 @@ func Test_DecodeVersion(t *testing.T) { {255, 255}, // error }), errWrapped: ErrDecodingVersionField, - errMessage: "decoding version field impl name: could not decode invalid integer", + errMessage: "decoding version field impl name: unknown prefix for compact uint: 255", }, // TODO add transaction version decode error once // https://github.com/ChainSafe/gossamer/pull/2683 From 8ae19603365d269cf09b7d41eb848ccf82830833 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Wed, 7 Sep 2022 11:09:45 -0400 Subject: [PATCH 13/14] implement PR comments --- pkg/scale/decode_test.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 259d72944f..42f37a59e3 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -5,13 +5,13 @@ package scale import ( "bytes" + "github.com/stretchr/testify/assert" "math/big" "reflect" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/stretchr/testify/assert" ) func Test_decodeState_decodeFixedWidthInt(t *testing.T) { @@ -388,18 +388,16 @@ func Test_decodeState_decodeUint(t *testing.T) { dstv := reflect.ValueOf(&dst) elem := indirect(dstv) - buf := &bytes.Buffer{} - ds := decodeState{} - _, err := buf.Write(tt.want) - assert.NoError(t, err) - ds.Reader = buf - err = ds.decodeUint(elem) - if (err != nil) != tt.wantErr { - t.Errorf("decodeState.decodeUint error = %v, wantErr %v", err, tt.wantErr) + ds := decodeState{ + Reader: bytes.NewBuffer(tt.want), } - if !reflect.DeepEqual(dst, tt.in) { - t.Errorf("decodeState.decodeUint = %v, want %v", dst, tt.in) + err := ds.decodeUint(elem) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } + assert.Equal(t, tt.in, dst) }) } } From dd4c99ab15ef7b8752bf85c44dfef979c0db1b39 Mon Sep 17 00:00:00 2001 From: edwardmack Date: Wed, 7 Sep 2022 11:13:50 -0400 Subject: [PATCH 14/14] run goimports --- pkg/scale/decode_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scale/decode_test.go b/pkg/scale/decode_test.go index 42f37a59e3..da808d8514 100644 --- a/pkg/scale/decode_test.go +++ b/pkg/scale/decode_test.go @@ -5,13 +5,13 @@ package scale import ( "bytes" - "github.com/stretchr/testify/assert" "math/big" "reflect" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/assert" ) func Test_decodeState_decodeFixedWidthInt(t *testing.T) {