From fda977841a225cb5c34231d3b05ce6ebda387200 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Fri, 31 Jan 2025 10:45:26 -0500 Subject: [PATCH] fix(evm)!: correct EIP55 addr encoding (#2187) * fix: add extra case of converting straight from bytes to string * fix: eip55 encoding * Update CHANGELOG.md --- CHANGELOG.md | 3 +- eth/eip55.go | 17 +++--- eth/eip55_test.go | 130 +++++++++++++++++++++++++++++----------------- 3 files changed, 90 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9d5bb793..17af456cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,7 +93,8 @@ needed to include double quotes around the hexadecimal string. - [#2180](https://github.com/NibiruChain/nibiru/pull/2180) - fix(evm): apply gas consumption across the entire EVM codebase at `CallContractWithInput` - [#2183](https://github.com/NibiruChain/nibiru/pull/2183) - fix(evm): bank keeper extension gas meter type - [#2184](https://github.com/NibiruChain/nibiru/pull/2184) - test(evm): e2e tests configuration enhancements -- +- [#2187](https://github.com/NibiruChain/nibiru/pull/2187) - fix(evm): fix eip55 address encoding + #### Nibiru EVM | Before Audit 2 - 2024-12-06 The codebase went through a third-party [Code4rena diff --git a/eth/eip55.go b/eth/eip55.go index 36f781d24..2c801bbea 100644 --- a/eth/eip55.go +++ b/eth/eip55.go @@ -36,7 +36,7 @@ func NewEIP55AddrFromStr(input string) (EIP55Addr, error) { // Marshal implements the gogo proto custom type interface. // Ref: https://github.com/cosmos/gogoproto/blob/v1.5.0/custom_types.md func (h EIP55Addr) Marshal() ([]byte, error) { - return []byte(h.Address.Hex()), nil + return h.Bytes(), nil } // MarshalJSON returns the [EIP55Addr] as JSON bytes. @@ -51,18 +51,15 @@ func (h EIP55Addr) MarshalJSON() ([]byte, error) { // Implements the gogo proto custom type interface. // Ref: https://github.com/cosmos/gogoproto/blob/v1.5.0/custom_types.md func (h *EIP55Addr) MarshalTo(data []byte) (n int, err error) { - copy(data, []byte(h.Address.Hex())) + copy(data, h.Bytes()) return h.Size(), nil } // Unmarshal implements the gogo proto custom type interface. // Ref: https://github.com/cosmos/gogoproto/blob/v1.5.0/custom_types.md func (h *EIP55Addr) Unmarshal(data []byte) error { - addr, err := NewEIP55AddrFromStr(string(data)) - if err != nil { - return err - } - *h = addr + addr := gethcommon.BytesToAddress(data) + *h = EIP55Addr{Address: addr} return nil } @@ -71,9 +68,7 @@ func (h *EIP55Addr) Unmarshal(data []byte) error { func (h *EIP55Addr) UnmarshalJSON(bz []byte) error { var addrStr string if err := json.Unmarshal(bz, &addrStr); err != nil { - return fmt.Errorf( - "EIP55AddrError: UnmarhsalJSON had invalid input %s: %w", bz, err, - ) + return err } addr, err := NewEIP55AddrFromStr(addrStr) if err != nil { @@ -86,5 +81,5 @@ func (h *EIP55Addr) UnmarshalJSON(bz []byte) error { // Size implements the gogo proto custom type interface. // Ref: https://github.com/cosmos/gogoproto/blob/v1.5.0/custom_types.md func (h EIP55Addr) Size() int { - return len([]byte(h.Address.Hex())) + return len(h.Bytes()) } diff --git a/eth/eip55_test.go b/eth/eip55_test.go index a2970e5e2..af56344a4 100644 --- a/eth/eip55_test.go +++ b/eth/eip55_test.go @@ -1,9 +1,9 @@ package eth_test import ( + "encoding/json" "fmt" "strconv" - "strings" "testing" gethcommon "github.com/ethereum/go-ethereum/common" @@ -12,9 +12,9 @@ import ( "github.com/NibiruChain/nibiru/v2/eth" ) -// MustNewEIP55AddrFromStr is the same as [NewEIP55AddrFromStr], except it panics +// mustNewEIP55AddrFromStr is the same as [NewEIP55AddrFromStr], except it panics // when there's an error. -func MustNewEIP55AddrFromStr(input string) eth.EIP55Addr { +func mustNewEIP55AddrFromStr(input string) eth.EIP55Addr { addr, err := eth.NewEIP55AddrFromStr(input) if err != nil { panic(err) @@ -22,15 +22,9 @@ func MustNewEIP55AddrFromStr(input string) eth.EIP55Addr { return addr } -var threeValidAddrs []eth.EIP55Addr = []eth.EIP55Addr{ - MustNewEIP55AddrFromStr("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"), - MustNewEIP55AddrFromStr("0xAe967917c465db8578ca9024c205720b1a3651A9"), - MustNewEIP55AddrFromStr("0x1111111111111111111112222222222223333323"), -} - func (s *EIP55AddrSuite) TestEquivalence() { expectedGethAddr := gethcommon.HexToAddress("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed") - expectedEIP55Addr := MustNewEIP55AddrFromStr("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed") + expectedEIP55Addr := mustNewEIP55AddrFromStr("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed") equivalentAddrs := []string{ "0x5aaeb6053f3e94c9b9a09f33669435e7ef1beaed", @@ -110,54 +104,95 @@ func (s *EIP55AddrSuite) TestNewEIP55Addr() { } } -func (s *EIP55AddrSuite) TestProtobufEncoding() { +func (s *EIP55AddrSuite) TestJsonEncoding() { for tcIdx, tc := range []struct { input eth.EIP55Addr - expectedJson string + expectedJson json.RawMessage wantErr string }{ { - input: threeValidAddrs[0], - expectedJson: `"0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"`, + input: mustNewEIP55AddrFromStr("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"), + expectedJson: []byte("\"0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed\""), }, { - input: threeValidAddrs[1], - expectedJson: `"0xAe967917c465db8578ca9024c205720b1a3651A9"`, + input: mustNewEIP55AddrFromStr("0xAe967917c465db8578ca9024c205720b1a3651A9"), + expectedJson: []byte("\"0xAe967917c465db8578ca9024c205720b1a3651A9\""), }, { - input: threeValidAddrs[2], - expectedJson: `"0x1111111111111111111112222222222223333323"`, + input: mustNewEIP55AddrFromStr("0x1111111111111111111112222222222223333323"), + expectedJson: []byte("\"0x1111111111111111111112222222222223333323\""), }, } { s.Run(strconv.Itoa(tcIdx), func() { - givenMut := tc.input - jsonBz, err := givenMut.MarshalJSON() - s.NoError(err) - s.Equal(tc.expectedJson, string(jsonBz)) + jsonBz, err := tc.input.MarshalJSON() + s.Require().NoError(err) + s.Require().EqualValues(tc.expectedJson, jsonBz) eip55Addr := new(eth.EIP55Addr) - s.NoError(eip55Addr.UnmarshalJSON(jsonBz)) - s.Equal(givenMut, tc.input, - "Given -> MarshalJSON -> UnmarshalJSON returns a different value than the given when it should be an identity operation (no-op). test case #%d", tcIdx) + s.Require().NoError(eip55Addr.UnmarshalJSON(jsonBz)) + s.Require().EqualValues(tc.input, *eip55Addr) + }) + } +} +func (s *EIP55AddrSuite) TestProtobufEncoding() { + for tcIdx, tc := range []struct { + input eth.EIP55Addr + expectedProtoBz []byte + wantErr string + }{ + { + input: mustNewEIP55AddrFromStr("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"), + expectedProtoBz: []byte{90, 174, 182, 5, 63, 62, 148, 201, 185, 160, 159, 51, 102, 148, 53, 231, 239, 27, 234, 237}, + }, + { + input: mustNewEIP55AddrFromStr("0xAe967917c465db8578ca9024c205720b1a3651A9"), + expectedProtoBz: []byte{174, 150, 121, 23, 196, 101, 219, 133, 120, 202, 144, 36, 194, 5, 114, 11, 26, 54, 81, 169}, + }, + { + input: mustNewEIP55AddrFromStr("0x1111111111111111111112222222222223333323"), + expectedProtoBz: []byte{17, 17, 17, 17, 17, 17, 17, 17, 17, 17, 18, 34, 34, 34, 34, 34, 35, 51, 51, 35}, + }, + } { + s.Run(strconv.Itoa(tcIdx), func() { bz, err := tc.input.Marshal() - s.NoError(err) - s.Equal(strings.Trim(tc.expectedJson, `"`), string(bz), - "Marshaling to bytes gives different value than the test case specifies. test case #%d", tcIdx) + s.Require().NoError(err) + s.Require().EqualValues(tc.expectedProtoBz, bz) - err = eip55Addr.Unmarshal(bz) - s.NoError(err) - s.Equal(tc.input.Address, eip55Addr.Address, - "Given -> Marshal -> Unmarshal returns a different value than the given when it should be an identity operation (no-op). test case #%d", tcIdx) + eip55Addr := new(eth.EIP55Addr) + s.Require().NoError(eip55Addr.Unmarshal(bz)) + s.Require().Equal(tc.input.Address, eip55Addr.Address) + }) + } +} - s.Equal(len([]byte(tc.input.Hex())), tc.input.Size()) - s.Equal(len(tc.input.Hex()), tc.input.Size()) +func (s *EIP55AddrSuite) TestSize() { + for idx, tc := range []struct { + input eth.EIP55Addr + expectedSize int + wantErr string + }{ + { + input: mustNewEIP55AddrFromStr("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed"), + expectedSize: 20, + }, + { + input: mustNewEIP55AddrFromStr("0xAe967917c465db8578ca9024c205720b1a3651A9"), + expectedSize: 20, + }, + { + input: mustNewEIP55AddrFromStr("0x1111111111111111111112222222222223333323"), + expectedSize: 20, + }, + } { + s.Run(strconv.Itoa(idx), func() { + s.Require().EqualValues(tc.expectedSize, tc.input.Size()) }) } } // showcases how geth checks for valid hex addresses and treats invalid inputs -func (s *EIP55AddrSuite) TestIsEIP55Address() { +func (s *EIP55AddrSuite) TestHexAddress() { s.True(gethcommon.IsHexAddress("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed")) s.True(gethcommon.IsHexAddress("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAED")) s.False(gethcommon.IsHexAddress("0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed1234")) @@ -179,21 +214,20 @@ func TestEIP55AddrSuite(t *testing.T) { func (s *EIP55AddrSuite) TestStringEncoding() { addrHex := "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed" - addr := new(eth.EIP55Addr) - err := addr.Unmarshal([]byte(addrHex)) - s.NoError(err) - s.Equal(addrHex, addr.Address.Hex()) + addr, err := eth.NewEIP55AddrFromStr(addrHex) + s.Require().NoError(err) + s.Require().Equal(addrHex, addr.Address.Hex()) - addrBytes, err := addr.Marshal() - s.NoError(err) - s.Equal(addrHex, string(addrBytes)) + addrBz, err := addr.Marshal() + s.Require().NoError(err) + s.Require().EqualValues(addr.Bytes(), addrBz) bz, err := addr.MarshalJSON() - s.NoError(err) - s.Equal(fmt.Sprintf(`"%s"`, addrHex), string(bz)) + s.Require().NoError(err) + s.Require().Equal(fmt.Sprintf(`"%s"`, addrHex), string(bz)) - addrb := new(eth.EIP55Addr) - err = addrb.UnmarshalJSON([]byte(fmt.Sprintf(`"%s"`, addrHex))) - s.NoError(err) - s.EqualValues(addrb, addr) + newAddr := new(eth.EIP55Addr) + err = newAddr.UnmarshalJSON(bz) + s.Require().NoError(err) + s.Require().EqualValues(addrHex, newAddr.Hex()) }