Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ccip: Add unknown address types #836

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
176 changes: 176 additions & 0 deletions pkg/types/ccipocr3/v2/common_types.go
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this API surface need to be exported? Who will use it?

Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package ccipocr3

import (
"encoding/hex"
"fmt"
"math/big"
"strings"
)

// UnknownAddress represents a raw address with an unknown encoding.
type UnknownAddress []byte

// NewUnknownAddressFromHex creates a new UnknownAddress from a hex string.
func NewUnknownAddressFromHex(s string) (UnknownAddress, error) {
b, err := NewBytesFromString(s)
if err != nil {
return nil, err
}
return UnknownAddress(b), nil
}

// String returns the hex representation of the unknown address.
func (a UnknownAddress) String() string {
return Bytes(a).String()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we prefix this by something to make it clear that its unknown?

Suggested change
return Bytes(a).String()
return fmt.Sprintf("unkown_%s", Bytes(a).String())

Copy link
Contributor

Choose a reason for hiding this comment

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

Should unknown addresses string representations ever be passed in somewhere that will make use of that representation (e.g like the contract reader)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that would be great.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would rather not attach semantics to the string representation. It's mainly kept here for convenience and backwards compatibility.

Copy link
Collaborator Author

@winder winder Oct 14, 2024

Choose a reason for hiding this comment

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

It's part of the CCIP message, so I guess we need to keep it without a prefix.

}

func (a UnknownAddress) MarshalJSON() ([]byte, error) {
return Bytes(a).MarshalJSON()
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here re: prefix

}

func (a *UnknownAddress) UnmarshalJSON(data []byte) error {
return (*Bytes)(a).UnmarshalJSON(data)
}

// UnknownEncodedAddress represents an encoded address with an unknown encoding.
type UnknownEncodedAddress string

type Bytes []byte

func NewBytesFromString(s string) (Bytes, error) {
if len(s) < 2 {
return nil, fmt.Errorf("Bytes must be of at least length 2 (i.e, '0x' prefix): %s", s)
}

if !strings.HasPrefix(s, "0x") {
return nil, fmt.Errorf("Bytes must start with '0x' prefix: %s", s)
}

b, err := hex.DecodeString(s[2:])
if err != nil {
return nil, fmt.Errorf("failed to decode hex: %w", err)
}

return Bytes(b), nil
}

func (b Bytes) String() string {
return "0x" + hex.EncodeToString(b)
}

func (b Bytes) MarshalJSON() ([]byte, error) {
return []byte(fmt.Sprintf(`"%s"`, b.String())), nil
}

func (b *Bytes) UnmarshalJSON(data []byte) error {
v := string(data)
if len(v) < 2 {
return fmt.Errorf("Bytes must be of at least length 2 (i.e, '0x' prefix): %s", v)
}

// trim the start and end double quotes
v = v[1 : len(v)-1]

if !strings.HasPrefix(v, "0x") {
return fmt.Errorf("Bytes must start with '0x' prefix: %s", v)
}

// Decode everything after the '0x' prefix.
bs, err := hex.DecodeString(v[2:])
if err != nil {
return fmt.Errorf("failed to decode hex: %w", err)
}

*b = bs
return nil
}

type Bytes32 [32]byte

func NewBytes32FromString(s string) (Bytes32, error) {
if len(s) < 2 {
return Bytes32{}, fmt.Errorf("Bytes32 must be of at least length 2 (i.e, '0x' prefix): %s", s)
}

if !strings.HasPrefix(s, "0x") {
return Bytes32{}, fmt.Errorf("Bytes32 must start with '0x' prefix: %s", s)
}

b, err := hex.DecodeString(s[2:])
if err != nil {
return Bytes32{}, fmt.Errorf("failed to decode hex: %w", err)
}

var res Bytes32
copy(res[:], b)

Choose a reason for hiding this comment

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

Would we want to put the bytes at the end of the array?
ie. should the result of the conversion be "0x1234" -> "0x000...01234" rather than "0x1234" -> "0x12340000..."?

Copy link
Contributor

Choose a reason for hiding this comment

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

The former seems to be more of what I would expect, at least if the hex bytes represented a number

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code isn't changed from v1. There aren't many places using Bytes32 after this change, we would have to review those to see if it makes sense to modify the behavior.

I guess my expectation would be s == NewBytes32FromString(s).String() which definitely isn't the case with the current implementation which also silently truncates the input.

return res, nil
}

func (b Bytes32) String() string {
return "0x" + hex.EncodeToString(b[:])
}

func (b Bytes32) IsEmpty() bool {
return b == Bytes32{}
}

func (b Bytes32) MarshalJSON() ([]byte, error) {
return []byte(fmt.Sprintf(`"%s"`, b.String())), nil
}

func (b *Bytes32) UnmarshalJSON(data []byte) error {
v := string(data)
if len(v) < 4 {
return fmt.Errorf("invalid MerkleRoot: %s", v)
}

bCp, err := hex.DecodeString(v[1 : len(v)-1][2:])
if err != nil {
return err
}

copy(b[:], bCp)
return nil
}

type BigInt struct {
*big.Int
}

func NewBigInt(i *big.Int) BigInt {
return BigInt{Int: i}
}

func NewBigIntFromInt64(i int64) BigInt {
return BigInt{Int: big.NewInt(i)}
}

func (b BigInt) MarshalJSON() ([]byte, error) {
if b.Int == nil {
return []byte("null"), nil
}
return []byte(fmt.Sprintf(`"%s"`, b.String())), nil
}

func (b *BigInt) UnmarshalJSON(p []byte) error {
if string(p) == "null" {
return nil
}

if len(p) < 2 {
return fmt.Errorf("invalid BigInt: %s", p)
}
p = p[1 : len(p)-1]

z := big.NewInt(0)
_, ok := z.SetString(string(p), 10)
if !ok {
return fmt.Errorf("not a valid big integer: %s", p)
}
b.Int = z
return nil
}

func (b BigInt) IsEmpty() bool {
return b.Int == nil
}
146 changes: 146 additions & 0 deletions pkg/types/ccipocr3/v2/common_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
package ccipocr3

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestNewBytes32FromString(t *testing.T) {
testCases := []struct {
name string
input string
expected Bytes32
expErr bool
}{
{
name: "valid input",
input: "0x200000000000000000000000",
expected: Bytes32{0x20, 0},
expErr: false,
},
{
name: "invalid hex characters",
input: "lrfv",
expected: Bytes32{},
expErr: true,
},
{
name: "invalid input, no 0x prefix",
input: "200000000000000000000000",
expected: Bytes32{},
expErr: true,
},
{
name: "invalid input, not enough hex chars",
input: "0x2",
expected: Bytes32{},
expErr: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
actual, err := NewBytes32FromString(tc.input)
if tc.expErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
assert.Equal(t, tc.expected, actual)
})
}
}

func TestBytes32_IsEmpty(t *testing.T) {
testCases := []struct {
name string
input Bytes32
expected bool
}{
{
name: "empty",
input: Bytes32{},
expected: true,
},
{
name: "not empty",
input: Bytes32{0x20, 0},
expected: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, tc.input.IsEmpty())
})
}
}

func TestNewBytesFromString(t *testing.T) {
tests := []struct {
name string
arg string
want Bytes
wantErr bool
}{
{
"valid input",
"0x20",
Bytes{0x20},
false,
},
{
"valid long input",
"0x2010201020",
Bytes{0x20, 0x10, 0x20, 0x10, 0x20},
false,
},
{
"invalid input",
"0",
nil,
true,
},
{
"invalid input, not enough hex chars",
"0x2",
nil,
true,
},
{
"invalid input, no 0x prefix",
"20",
nil,
true,
},
{
"invalid hex characters",
"0x2g",
nil,
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := NewBytesFromString(tt.arg)
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tt.want, got)
}
})

t.Run(tt.name, func(t *testing.T) {
got, err := NewUnknownAddressFromHex(tt.arg)
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, UnknownAddress(tt.want), got)
}
})
}
}
Loading
Loading