Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions common/hexutil/hexutil.libevm.go
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the motivation is to support JSON marshalling then these functions are unnecessary.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2025 the libevm authors.
//
// The libevm additions to go-ethereum are free software: you can redistribute
// them and/or modify them under the terms of the GNU Lesser General Public License
// as published by the Free Software Foundation, either version 3 of the License,
// or (at your option) any later version.
//
// The libevm additions are distributed in the hope that they will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
// General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see
// <http://www.gnu.org/licenses/>.

package hexutil

import "strconv"

var ErrUint16Range = &decError{"hex number > 16 bits"}

// DecodeUint16 decodes a hex string with 0x prefix as a quantity.
func DecodeUint16(input string) (uint16, error) {
raw, err := checkNumber(input)
if err != nil {
return 0, err
}
dec, err := strconv.ParseUint(raw, 16, 16)
if err != nil {
err = mapError(err)
if err == ErrUint64Range {
return 0, ErrUint16Range
}
}
return uint16(dec), err //nolint:gosec // G115 won't overflow uint16 as ParseUint uses 16 bits
Comment on lines +30 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err != nil {
err = mapError(err)
if err == ErrUint64Range {
return 0, ErrUint16Range
}
}
return uint16(dec), err //nolint:gosec // G115 won't overflow uint16 as ParseUint uses 16 bits
if err != nil {
err = mapError(err)
if err == ErrUint64Range {
err = ErrUint16Range
}
return 0, err
}
return uint16(dec), nil //nolint:gosec // G115 won't overflow uint16 as ParseUint uses 16 bits

Rationale

}

// EncodeUint16 encodes i as a hex string with 0x prefix.
func EncodeUint16(i uint16) string {
enc := make([]byte, 2, 6)
copy(enc, "0x")
return string(strconv.AppendUint(enc, uint64(i), 16))
}
76 changes: 76 additions & 0 deletions common/hexutil/hexutil.libevm_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright 2025 the libevm authors.
//
// The libevm additions to go-ethereum are free software: you can redistribute
// them and/or modify them under the terms of the GNU Lesser General Public License
// as published by the Free Software Foundation, either version 3 of the License,
// or (at your option) any later version.
//
// The libevm additions are distributed in the hope that they will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
// General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see
// <http://www.gnu.org/licenses/>.

package hexutil

import "testing"

var (
encodeUint16Tests = []marshalTest{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test tables should be in the test function, not separated out like this. It adds unnecessary distance for the reader, who can't simply check the test loop and its inputs at the same time.

{uint16(0), "0x0"},
{uint16(1), "0x1"},
{uint16(0xff), "0xff"},
{uint16(0x1122), "0x1122"},
}

decodeUint16Tests = []unmarshalTest{
// invalid
{input: `0`, wantErr: ErrMissingPrefix},
{input: `0x`, wantErr: ErrEmptyNumber},
{input: `0x01`, wantErr: ErrLeadingZero},
{input: `0xfffff`, wantErr: ErrUint16Range},
{input: `0xz1`, wantErr: ErrSyntax},
// valid
{input: `0x0`, want: uint16(0)},
{input: `0x2`, want: uint16(0x2)},
{input: `0x2F2`, want: uint16(0x2f2)},
{input: `0X2F2`, want: uint16(0x2f2)},
{input: `0xff`, want: uint16(0xff)},
{input: `0x12af`, want: uint16(0x12af)},
{input: `0xbbb`, want: uint16(0xbbb)},
{input: `0xffff`, want: uint16(0xffff)},
}
)

func TestEncodeUint16(t *testing.T) {
for _, test := range encodeUint16Tests {
input, ok := test.input.(uint16)
if !ok {
t.Errorf("input %v: not a uint16", test.input)
}
Comment on lines +50 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a code smell suggesting that the marshalTest struct type isn't fit for purpose. The only benefit to using it is saving 2 lines in declaring the fields, while the cost is this extra check as well as having to explicitly wrap each input in uint(...).

enc := EncodeUint16(input)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
enc := EncodeUint16(input)
got := EncodeUint16(input)

The value under test should be named got, unless there is a need for disambiguation between multiple values, in which case use a suffix like gotFoo.

if enc != test.want {
t.Errorf("input %x: wrong encoding %s", test.input, enc)
}
}
}

func TestDecodeUint16(t *testing.T) {
for _, test := range decodeUint16Tests {
dec, err := DecodeUint16(test.input)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dec, err := DecodeUint16(test.input)
got, err := DecodeUint16(test.input)

if !checkError(t, test.input, err, test.wantErr) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use if !errors.Is(err, test.wantErr) { instead as it's idiomatic and part of the standard library. libevm code leans towards style consistency across the modifications over consistency with the upstream, unless there is good reason not to.

continue
}
want, ok := test.want.(uint16)
if !ok {
t.Errorf("want %v: not a uint16", test.want)
}
Comment on lines +67 to +70
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above re use of an inappropriate test struct.

if dec != want {
t.Errorf("input %s: value mismatch: got %x, want %x", test.input, dec, want)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.Errorf("input %s: value mismatch: got %x, want %x", test.input, dec, want)
t.Errorf("DecodeUint16(%q) got %x, want %x", test.input, dec, want)

Rationale A and B.

continue
}
}
}
72 changes: 72 additions & 0 deletions common/hexutil/json.libevm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2025 the libevm authors.
//
// The libevm additions to go-ethereum are free software: you can redistribute
// them and/or modify them under the terms of the GNU Lesser General Public License
// as published by the Free Software Foundation, either version 3 of the License,
// or (at your option) any later version.
//
// The libevm additions are distributed in the hope that they will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
// General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see
// <http://www.gnu.org/licenses/>.

package hexutil

import (
"reflect"
"strconv"
)

var uint16T = reflect.TypeOf(Uint16(0))

// Uint16 marshals/unmarshals as a JSON string with 0x prefix.
// The zero value marshals as "0x0".
type Uint16 uint16

// MarshalText implements encoding.TextMarshaler.
func (b Uint16) MarshalText() ([]byte, error) {
buf := make([]byte, 2, 6)
copy(buf, `0x`)
buf = strconv.AppendUint(buf, uint64(b), 16)
return buf, nil
Comment on lines +32 to +35
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the EncodeUint16() function were to be kept then this should just call it instead of reimplementing the identical code.

Comment on lines +34 to +35
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
buf = strconv.AppendUint(buf, uint64(b), 16)
return buf, nil
return strconv.AppendUint(buf, uint64(b), 16), nil

}

// UnmarshalJSON implements json.Unmarshaler.
func (b *Uint16) UnmarshalJSON(input []byte) error {
if !isString(input) {
return errNonString(uint16T)
}
return wrapTypeError(b.UnmarshalText(input[1:len(input)-1]), uint16T)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(subjective) I would explicitly check the error and only call the wrapTypeError function if non-nil, even though it checks for this. The first time I read the code I was really confused by why there was always an error being returned.

}

// UnmarshalText implements encoding.TextUnmarshaler.
func (b *Uint16) UnmarshalText(input []byte) error {
raw, err := checkNumberText(input)
if err != nil {
return err
}
if len(raw) > 4 {
return ErrUint16Range
}
var dec uint16
for _, byte := range raw {
nib := decodeNibble(byte)
if nib == badNibble {
return ErrSyntax
}
dec *= 16
dec += uint16(nib) //nolint:gosec // G115 won't overflow uint16 as decodeNibble uses 4 bits
}

*b = Uint16(dec)
return nil
Comment on lines +48 to +66
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above re reusing DecodeUint64 if it were to be kept. If not, then strconv.ParseUint should be used in place of the nibble decoding.

}

// String returns the hex encoding of b.
func (b Uint16) String() string {
return EncodeUint16(uint16(b))
}
103 changes: 103 additions & 0 deletions common/hexutil/json.libevm_test.go
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most comments on hexutil.libevm_test.go apply to this whole file too.

Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright 2025 the libevm authors.
//
// The libevm additions to go-ethereum are free software: you can redistribute
// them and/or modify them under the terms of the GNU Lesser General Public License
// as published by the Free Software Foundation, either version 3 of the License,
// or (at your option) any later version.
//
// The libevm additions are distributed in the hope that they will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
// General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see
// <http://www.gnu.org/licenses/>.

package hexutil

import (
"encoding/json"
"testing"
)

var unmarshalUint16Tests = []unmarshalTest{
// invalid encoding
{input: "", wantErr: errJSONEOF},
{input: "null", wantErr: errNonString(uint16T)},
{input: "10", wantErr: errNonString(uint16T)},
{input: `"0"`, wantErr: wrapTypeError(ErrMissingPrefix, uint16T)},
{input: `"0x"`, wantErr: wrapTypeError(ErrEmptyNumber, uint16T)},
{input: `"0x01"`, wantErr: wrapTypeError(ErrLeadingZero, uint16T)},
{input: `"0x10000"`, wantErr: wrapTypeError(ErrUint16Range, uint16T)},
{input: `"0xx"`, wantErr: wrapTypeError(ErrSyntax, uint16T)},
{input: `"0xz1"`, wantErr: wrapTypeError(ErrSyntax, uint16T)},

// valid encoding
{input: `""`, want: uint16(0)},
{input: `"0x0"`, want: uint16(0)},
{input: `"0x2"`, want: uint16(0x2)},
{input: `"0x2F2"`, want: uint16(0x2f2)},
{input: `"0X2F2"`, want: uint16(0x2f2)},
{input: `"0x1122"`, want: uint16(0x1122)},
{input: `"0xbbb"`, want: uint16(0xbbb)},
{input: `"0xffff"`, want: uint16(0xffff)},
}

func TestUnmarshalUint16(t *testing.T) {
for _, test := range unmarshalUint16Tests {
var v Uint16
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var v Uint16
var got Uint16

err := json.Unmarshal([]byte(test.input), &v)
if !checkError(t, test.input, err, test.wantErr) {
continue
}
want, ok := test.want.(uint16)
if !ok {
t.Errorf("want %v: not a uint16", test.want)
}
if uint16(v) != want {
t.Errorf("input %s: value mismatch: got %d, want %d", test.input, v, want)
continue
}
}
}

func BenchmarkUnmarshalUint16(b *testing.B) {
input := []byte(`"0x1234"`)
for i := 0; i < b.N; i++ {
var v Uint16
err := v.UnmarshalJSON(input)
if err != nil {
b.Errorf("error unmarshalling: %v", err)
}
}
}

func TestMarshalUint16(t *testing.T) {
tests := []struct {
input uint16
want string
}{
{0, "0x0"},
{1, "0x1"},
{0xff, "0xff"},
{0x1122, "0x1122"},
{0xffff, "0xffff"},
}

for _, test := range tests {
out, err := json.Marshal(Uint16(test.input))
if err != nil {
t.Errorf("%d: %v", test.input, err)
continue
}
if want := `"` + test.want + `"`; string(out) != want {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if want := `"` + test.want + `"`; string(out) != want {
if want := strconv.Quote(test.want); string(got) != want {

Intent is explicit.

t.Errorf("%d: MarshalJSON output mismatch: got %q, want %q", test.input, out, want)
continue
}
if out := Uint16(test.input).String(); out != test.want {
t.Errorf("%d: String mismatch: got %q, want %q", test.input, out, test.want)
continue
}
}
}
Loading