From debaa4d67a8b4488f7c1b139e79910351dd2d41a Mon Sep 17 00:00:00 2001 From: Calico Nino <54007257+CalicoNino@users.noreply.github.com> Date: Fri, 6 Dec 2024 18:55:19 -0500 Subject: [PATCH] fix: Use canonical heaxdecimal strings for Eip155 address encoding (#2120) * refactor: funtoken erc20 addr to be bytes in proto * refactor: marshall to return address * fix: eip55 proto encoding using hex string * fix: typos * --amend * Update x/evm/keeper/funtoken_from_erc20_test.go --------- Co-authored-by: Unique Divine <51418232+Unique-Divine@users.noreply.github.com> --- CHANGELOG.md | 9 +++++---- eth/eip55.go | 25 ++++++++++--------------- eth/eip55_test.go | 32 +++++++++++++++++++++++++++----- x/evm/keeper/grpc_query.go | 2 +- 4 files changed, 43 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ee83562c..59766fdb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,22 +45,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#2119](https://github.com/NibiruChain/nibiru/pull/2119) - fix(evm): Guarantee that gas consumed during any send operation of the "NibiruBankKeeper" depends only on the "bankkeeper.BaseKeeper"'s gas consumption. +- [#2120](https://github.com/NibiruChain/nibiru/pull/2120) - fix: Use canonical hexadecimal strings for Eip155 address encoding #### Nibiru EVM | Before Audit 2 - 2024-12-06 The codebase went through a third-party [Code4rena Zenith](https://code4rena.com/zenith) Audit, running from 2024-10-07 until 2024-11-01 and including both a primary review period and mitigation/remission -period. This section describes code changes that occured after that audit in +period. This section describes code changes that occurred after that audit in preparation for a second audit starting in November 2024. - [#2068](https://github.com/NibiruChain/nibiru/pull/2068) - feat: enable wasm light clients on IBC (08-wasm) - [#2074](https://github.com/NibiruChain/nibiru/pull/2074) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation. The bank metadata for a new FunToken mapping ties a connection between the Bank Coin's `DenomUnit` and the ERC20 contract metadata like the name, decimals, and symbol. This change brings parity between EVM wallets, such as MetaMask, and Interchain wallets like Keplr and Leap. - [#2076](https://github.com/NibiruChain/nibiru/pull/2076) - fix(evm-gas-fees): Use effective gas price in RefundGas and make sure that units are properly - reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059) + reflected on all occurrences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059) and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724). -- [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM develoment +- [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM development - [#2086](https://github.com/NibiruChain/nibiru/pull/2086) - fix(evm-precomples): Fix state consistency in precompile execution by ensuring proper journaling of state changes in the StateDB. This pull request makes sure that state is @@ -115,7 +116,7 @@ tests for race conditions within funtoken precompile timestamps resulting from ctx.WithBlock* don't actually correspond to the block header information from specified blocks in the chain's history, so the oracle exchange rates need a way to correctly retrieve this information. This change -fixes that discrepency, giving the expected block timesamp for the EVM's oracle +fixes that discrepancy, giving the expected block timestamp for the EVM's oracle precompiled contract. The change also simplifies and corrects the code in x/oracle. #### Nibiru EVM | Before Audit 1 - 2024-10-18 diff --git a/eth/eip55.go b/eth/eip55.go index 5a47664b9..b4e66a4d0 100644 --- a/eth/eip55.go +++ b/eth/eip55.go @@ -1,7 +1,6 @@ package eth import ( - "encoding/json" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" @@ -36,14 +35,14 @@ 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 h.Bytes(), nil + return []byte(h.Address.Hex()), nil } // MarshalJSON returns the [EIP55Addr] as JSON bytes. // Implements the gogo proto custom type interface. // Ref: https://github.com/cosmos/gogoproto/blob/v1.5.0/custom_types.md func (h EIP55Addr) MarshalJSON() ([]byte, error) { - return json.Marshal(h.String()) + return []byte(h.String()), nil } // MarshalTo serializes a EIP55Addr directly into a pre-allocated byte slice ("data"). @@ -51,38 +50,34 @@ 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, h.Bytes()) + copy(data, []byte(h.Address.Hex())) 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 := gethcommon.BytesToAddress(data) - *h = EIP55Addr{Address: addr} + addr, err := NewEIP55AddrFromStr(string(data)) + if err != nil { + return err + } + *h = addr return nil } // UnmarshalJSON implements the gogo proto custom type interface. // Ref: https://github.com/cosmos/gogoproto/blob/v1.5.0/custom_types.md func (h *EIP55Addr) UnmarshalJSON(bz []byte) error { - text := new(string) - if err := json.Unmarshal(bz, text); err != nil { - return err - } - - addr, err := NewEIP55AddrFromStr(*text) + addr, err := NewEIP55AddrFromStr(string(bz)) if err != nil { return err } - *h = addr - return nil } // 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(h.Bytes()) + return len([]byte(h.Address.Hex())) } diff --git a/eth/eip55_test.go b/eth/eip55_test.go index c0f1b9de4..d927d9447 100644 --- a/eth/eip55_test.go +++ b/eth/eip55_test.go @@ -116,15 +116,15 @@ func (s *EIP55AddrSuite) TestProtobufEncoding() { }{ { input: threeValidAddrs[0], - expectedJson: "\"0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed\"", + expectedJson: "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed", }, { input: threeValidAddrs[1], - expectedJson: "\"0xAe967917c465db8578ca9024c205720b1a3651A9\"", + expectedJson: "0xAe967917c465db8578ca9024c205720b1a3651A9", }, { input: threeValidAddrs[2], - expectedJson: "\"0x1111111111111111111112222222222223333323\"", + expectedJson: "0x1111111111111111111112222222222223333323", }, } { s.Run(strconv.Itoa(tcIdx), func() { @@ -140,7 +140,7 @@ func (s *EIP55AddrSuite) TestProtobufEncoding() { bz, err := tc.input.Marshal() s.NoError(err) - s.Equal(tc.input.Bytes(), bz, + s.Equal(tc.expectedJson, string(bz), "Marshaling to bytes gives different value than the test case specifies. test case #%d", tcIdx) err = eip55Addr.Unmarshal(bz) @@ -148,7 +148,8 @@ func (s *EIP55AddrSuite) TestProtobufEncoding() { 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) - s.Equal(len(tc.input.Bytes()), tc.input.Size()) + s.Equal(len([]byte(tc.input.Hex())), tc.input.Size()) + s.Equal(len(tc.input.Hex()), tc.input.Size()) }) } } @@ -173,3 +174,24 @@ type EIP55AddrSuite struct { func TestEIP55AddrSuite(t *testing.T) { suite.Run(t, new(EIP55AddrSuite)) } + +func (s *EIP55AddrSuite) TestStringEncoding() { + addrHex := "0x5aAeb6053F3E94C9b9A09f33669435E7Ef1BeAed" + addr := new(eth.EIP55Addr) + err := addr.Unmarshal([]byte(addrHex)) + s.NoError(err) + s.Equal(addrHex, addr.Address.Hex()) + + addrBytes, err := addr.Marshal() + s.NoError(err) + s.Equal(addrHex, string(addrBytes)) + + bz, err := addr.MarshalJSON() + s.NoError(err) + s.Equal(addrHex, string(bz)) + + addrb := new(eth.EIP55Addr) + err = addrb.UnmarshalJSON([]byte(addrHex)) + s.NoError(err) + s.EqualValues(addrb, addr) +} diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index 31a887bbc..6c0f005e5 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -650,7 +650,7 @@ const DefaultGethTraceTimeout = 5 * time.Second // Configures a Nibiru EVM tracer that is used to "trace" and analyze // the execution of transactions within a given block. Block information is read // from the context (goCtx). [TraceBlock] is responsible iterates over each Eth -// transacion message and calls [TraceEthTxMsg] on it. +// transaction message and calls [TraceEthTxMsg] on it. func (k Keeper) TraceBlock( goCtx context.Context, req *evm.QueryTraceBlockRequest, ) (*evm.QueryTraceBlockResponse, error) {