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.

Sign up for GitHub

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

Jump to bottom

test(evm): backend tests with test network and real txs #2045

Merged
merged 2 commits into from
Sep 21, 2024
Merged

test(evm): backend tests with test network and real txs #2045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74bf9e5
test(evm): backend tests with test network and real txs
onikonychev Sep 20, 2024
8bde049
chore: changelog update
onikonychev Sep 20, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#2031](https://github.com/NibiruChain/nibiru/pull/2031) - fix(evm): debug calls with custom tracer and tracer options
- [#2039](https://github.com/NibiruChain/nibiru/pull/2039) - refactor(rpc-backend): remove unnecessary interface code
- [#2039](https://github.com/NibiruChain/nibiru/pull/2039) - refactor(rpc-backend): Remove mocks from eth/rpc/backend, partially completing [nibiru#2037](https://github.com/NibiruChain/nibiru/issue/2037).
- [#2045](https://github.com/NibiruChain/nibiru/pull/2045) - test(evm): backend tests with test network and real txs

#### Dapp modules: perp, spot, oracle, etc

Expand Down
8 changes: 4 additions & 4 deletions eth/chain_id.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@
return nibiruEvmChainId.MatchString(chainID)
}

// ParseEthChainID parses a string chain identifier's epoch to an
// Ethereum-compatible chain-id in *big.Int format.
// ParseEthChainID parses a string chain identifier's
// to an Ethereum-compatible chain-id in *big.Int format.
//
// This function uses Nibiru's map of chain IDs defined in Nibiru/app/appconst
// rather than the regex of EIP155, which is implemented by
// ParseEthChainIDStrict.
func ParseEthChainID(chainID string) (*big.Int, error) {
return appconst.GetEthChainID(chainID), nil
func ParseEthChainID(chainID string) *big.Int {
return appconst.GetEthChainID(chainID)

Check warning on line 48 in eth/chain_id.go

View check run for this annotation

Codecov / codecov/patch

eth/chain_id.go#L47-L48 

Added lines #L47 - L48 were not covered by tests
}

// ParseEthChainIDStrict parses a string chain identifier's epoch to an
Expand Down
10 changes: 2 additions & 8 deletions eth/eip712/encoding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) {
return apitypes.TypedData{}, err
}

chainID, err := eth.ParseEthChainID(aminoDoc.ChainID)
if err != nil {
return apitypes.TypedData{}, errors.New("invalid chain ID passed as argument")
}
chainID := eth.ParseEthChainID(aminoDoc.ChainID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add back the error handling for parsing the chain ID.

Directly assigning the result of eth.ParseEthChainID to the chainID variable without checking for errors assumes that the parsing will always succeed. If the parsing fails, the function will proceed with an invalid chainID value, potentially leading to unexpected behavior.

Consider adding back the error handling:

chainID, err := eth.ParseEthChainID(aminoDoc.ChainID)
if err != nil {
    return apitypes.TypedData{}, fmt.Errorf("failed to parse chain ID: %w", err)
}


typedData, err := WrapTxToTypedData(
chainID.Uint64(),
Expand Down Expand Up @@ -167,10 +164,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) {

signerInfo := authInfo.SignerInfos[0]

chainID, err := eth.ParseEthChainID(signDoc.ChainId)
if err != nil {
return apitypes.TypedData{}, fmt.Errorf("invalid chain ID passed as argument: %w", err)
}
chainID := eth.ParseEthChainID(signDoc.ChainId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add back the error handling for parsing the chain ID.

Similar to the previous comment, directly assigning the result of eth.ParseEthChainID to the chainID variable without checking for errors assumes that the parsing will always succeed. If the parsing fails, the function will proceed with an invalid chainID value, potentially leading to unexpected behavior.

Consider adding back the error handling:

chainID, err := eth.ParseEthChainID(signDoc.ChainId)
if err != nil {
    return apitypes.TypedData{}, fmt.Errorf("failed to parse chain ID: %w", err)
}


stdFee := &legacytx.StdFee{
Amount: authInfo.Fee.Amount,
Expand Down
11 changes: 2 additions & 9 deletions eth/eip712/encoding_legacy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,7 @@ func legacyDecodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) {
feeDelegation := &FeeDelegationOptions{
FeePayer: feePayer,
}

chainID, err := eth.ParseEthChainID(aminoDoc.ChainID)
if err != nil {
return apitypes.TypedData{}, errors.New("invalid chain ID passed as argument")
}
chainID := eth.ParseEthChainID(aminoDoc.ChainID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Handle the error returned by eth.ParseEthChainID.

Parsing the chain ID without checking for errors could lead to runtime panics if the chain ID is invalid. It's important to handle the error and return it to the caller for proper error handling.

Apply this diff to handle the error:

-chainID := eth.ParseEthChainID(aminoDoc.ChainID)
+chainID, err := eth.ParseEthChainID(aminoDoc.ChainID)
+if err != nil {
+	return apitypes.TypedData{}, fmt.Errorf("failed to parse chain ID: %w", err)
+}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
chainID := eth.ParseEthChainID(aminoDoc.ChainID)
chainID, err := eth.ParseEthChainID(aminoDoc.ChainID)
if err != nil {
return apitypes.TypedData{}, fmt.Errorf("failed to parse chain ID: %w", err)
}


typedData, err := LegacyWrapTxToTypedData(
protoCodec,
Expand Down Expand Up @@ -165,10 +161,7 @@ func legacyDecodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error

signerInfo := authInfo.SignerInfos[0]

chainID, err := eth.ParseEthChainID(signDoc.ChainId)
if err != nil {
return apitypes.TypedData{}, fmt.Errorf("invalid chain ID passed as argument: %w", err)
}
chainID := eth.ParseEthChainID(signDoc.ChainId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Handle the error returned by eth.ParseEthChainID.

Similar to the previous comment, parsing the chain ID without checking for errors could lead to runtime panics if the chain ID is invalid. It's important to handle the error and return it to the caller for proper error handling.

Apply this diff to handle the error:

-chainID := eth.ParseEthChainID(signDoc.ChainId)
+chainID, err := eth.ParseEthChainID(signDoc.ChainId)
+if err != nil {
+	return apitypes.TypedData{}, fmt.Errorf("failed to parse chain ID: %w", err)
+}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
chainID := eth.ParseEthChainID(signDoc.ChainId)
chainID, err := eth.ParseEthChainID(signDoc.ChainId)
if err != nil {
return apitypes.TypedData{}, fmt.Errorf("failed to parse chain ID: %w", err)
}


stdFee := &legacytx.StdFee{
Amount: authInfo.Fee.Amount,
Expand Down
22 changes: 11 additions & 11 deletions eth/rpc/backend/account_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/ethereum/go-ethereum/common"
gethcommon "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/pkg/errors"

Expand All @@ -22,7 +22,7 @@ import (

// GetCode returns the contract code at the given address and block number.
func (b *Backend) GetCode(
address common.Address, blockNrOrHash rpc.BlockNumberOrHash,
address gethcommon.Address, blockNrOrHash rpc.BlockNumberOrHash,
) (hexutil.Bytes, error) {
blockNum, err := b.BlockNumberFromTendermint(blockNrOrHash)
if err != nil {
Expand All @@ -43,7 +43,7 @@ func (b *Backend) GetCode(

// GetProof returns an account object with proof and any storage proofs
func (b *Backend) GetProof(
address common.Address,
address gethcommon.Address,
storageKeys []string,
blockNrOrHash rpc.BlockNumberOrHash,
) (*rpc.AccountResult, error) {
Expand Down Expand Up @@ -81,7 +81,7 @@ func (b *Backend) GetProof(
storageProofs := make([]rpc.StorageResult, len(storageKeys))

for i, key := range storageKeys {
hexKey := common.HexToHash(key)
hexKey := gethcommon.HexToHash(key)
valueBz, proof, err := b.queryClient.GetProof(clientCtx, evm.StoreKey, evm.StateKey(address, hexKey.Bytes()))
if err != nil {
return nil, err
Expand All @@ -105,7 +105,7 @@ func (b *Backend) GetProof(
}

// query account proofs
accountKey := authtypes.AddressStoreKey(sdk.AccAddress(address.Bytes()))
accountKey := authtypes.AddressStoreKey(address.Bytes())
_, proof, err := b.queryClient.GetProof(clientCtx, authtypes.StoreKey, accountKey)
if err != nil {
return nil, err
Expand All @@ -120,18 +120,18 @@ func (b *Backend) GetProof(
Address: address,
AccountProof: GetHexProofs(proof),
Balance: (*hexutil.Big)(balance.BigInt()),
CodeHash: common.HexToHash(res.CodeHash),
CodeHash: gethcommon.HexToHash(res.CodeHash),
Nonce: hexutil.Uint64(res.Nonce),
// NOTE: The StorageHash is blank. Consider whether this is useful in the
// future. Currently, all storage is handles by persistent and transient
// `sdk.KVStore` objects.
StorageHash: common.Hash{},
StorageHash: gethcommon.Hash{},
StorageProof: storageProofs,
}, nil
}

// GetStorageAt returns the contract storage at the given address, block number, and key.
func (b *Backend) GetStorageAt(address common.Address, key string, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) {
func (b *Backend) GetStorageAt(address gethcommon.Address, key string, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) {
blockNum, err := b.BlockNumberFromTendermint(blockNrOrHash)
if err != nil {
return nil, err
Expand All @@ -147,13 +147,13 @@ func (b *Backend) GetStorageAt(address common.Address, key string, blockNrOrHash
return nil, err
}

value := common.HexToHash(res.Value)
value := gethcommon.HexToHash(res.Value)
return value.Bytes(), nil
}

// GetBalance returns the provided account's balance up to the provided block number.
func (b *Backend) GetBalance(
address common.Address,
address gethcommon.Address,
blockNrOrHash rpc.BlockNumberOrHash,
) (*hexutil.Big, error) {
blockNum, err := b.BlockNumberFromTendermint(blockNrOrHash)
Expand Down Expand Up @@ -189,7 +189,7 @@ func (b *Backend) GetBalance(
}

// GetTransactionCount returns the number of transactions at the given address up to the given block number.
func (b *Backend) GetTransactionCount(address common.Address, blockNum rpc.BlockNumber) (*hexutil.Uint64, error) {
func (b *Backend) GetTransactionCount(address gethcommon.Address, blockNum rpc.BlockNumber) (*hexutil.Uint64, error) {
n := hexutil.Uint64(0)
bn, err := b.BlockNumber()
if err != nil {
Expand Down
201 changes: 201 additions & 0 deletions eth/rpc/backend/account_info_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,202 @@
package backend_test

import (
"math/big"

gethcommon "github.com/ethereum/go-ethereum/common"
"golang.org/x/crypto/sha3"

"github.com/NibiruChain/nibiru/v2/x/evm/evmtest"

rpc "github.com/NibiruChain/nibiru/v2/eth/rpc"
)

func (s *BackendSuite) TestGetCode() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
codeFound bool
}{
{
name: "happy: valid contract address",
contractAddr: testContractAddress,
blockNumber: deployContractBlockNumber,
codeFound: true,
},
{
name: "sad: not a contract address",
contractAddr: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
codeFound: false,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
code, err := s.backend.GetCode(
tc.contractAddr,
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
if !tc.codeFound {
s.Require().Nil(code)
return
}
s.Require().NoError(err)
s.Require().NotNil(code)
})
}
}
Comment on lines +14 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

Adjust code existence checks to validate code length instead of nil comparison

In the TestGetCode function, when verifying whether code exists, comparing code to nil may not be sufficient, as an empty byte slice ([]byte{}) is not nil. To accurately check if code exists at the given address, consider checking the length of the code:

if !tc.codeFound {
-    s.Require().Nil(code)
+    s.Require().Equal(0, len(code))
    return
}
s.Require().NoError(err)
-s.Require().NotNil(code)
+s.Require().Greater(len(code), 0)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (s *BackendSuite) TestGetCode() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
codeFound bool
}{
{
name: "happy: valid contract address",
contractAddr: testContractAddress,
blockNumber: deployContractBlockNumber,
codeFound: true,
},
{
name: "sad: not a contract address",
contractAddr: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
codeFound: false,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
code, err := s.backend.GetCode(
tc.contractAddr,
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
if !tc.codeFound {
s.Require().Nil(code)
return
}
s.Require().NoError(err)
s.Require().NotNil(code)
})
}
}
func (s *BackendSuite) TestGetCode() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
codeFound bool
}{
{
name: "happy: valid contract address",
contractAddr: testContractAddress,
blockNumber: deployContractBlockNumber,
codeFound: true,
},
{
name: "sad: not a contract address",
contractAddr: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
codeFound: false,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
code, err := s.backend.GetCode(
tc.contractAddr,
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
if !tc.codeFound {
s.Require().Equal(0, len(code))
return
}
s.Require().NoError(err)
s.Require().Greater(len(code), 0)
})
}
}


func (s *BackendSuite) TestGetProof() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
address gethcommon.Address
slot uint64
wantValue string
}{
{
name: "happy: balance of the contract deployer",
contractAddr: testContractAddress,
address: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
slot: 0, // _balances is the first slot in ERC20
wantValue: "0xd3c21bcecceda1000000", // = 1000000 * (10**18), initial supply
},
{
name: "sad: address which is not in contract storage",
contractAddr: s.fundedAccEthAddr,
address: recipient,
blockNumber: deployContractBlockNumber,
slot: 0,
wantValue: "0x0",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
proof, err := s.backend.GetProof(
tc.contractAddr,
[]string{generateStorageKey(tc.address, tc.slot)},
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(proof)
s.Require().Equal(tc.wantValue, proof.StorageProof[0].Value.String())
})
}
}
Comment on lines +52 to +92
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure accurate value assertion in TestGetProof

In the TestGetProof function, when comparing the expected value with the retrieved proof value, consider using common.HexToHash(tc.wantValue) for consistency and to handle leading zeros correctly:

s.Require().Equal(
-    tc.wantValue,
-    proof.StorageProof[0].Value.String()
+    gethcommon.HexToHash(tc.wantValue),
+    proof.StorageProof[0].Value
)

This ensures that the comparison accounts for the full hash value, including any leading zeros that may be omitted when using String().

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (s *BackendSuite) TestGetProof() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
address gethcommon.Address
slot uint64
wantValue string
}{
{
name: "happy: balance of the contract deployer",
contractAddr: testContractAddress,
address: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
slot: 0, // _balances is the first slot in ERC20
wantValue: "0xd3c21bcecceda1000000", // = 1000000 * (10**18), initial supply
},
{
name: "sad: address which is not in contract storage",
contractAddr: s.fundedAccEthAddr,
address: recipient,
blockNumber: deployContractBlockNumber,
slot: 0,
wantValue: "0x0",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
proof, err := s.backend.GetProof(
tc.contractAddr,
[]string{generateStorageKey(tc.address, tc.slot)},
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(proof)
s.Require().Equal(tc.wantValue, proof.StorageProof[0].Value.String())
})
}
}
func (s *BackendSuite) TestGetProof() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
address gethcommon.Address
slot uint64
wantValue string
}{
{
name: "happy: balance of the contract deployer",
contractAddr: testContractAddress,
address: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
slot: 0, // _balances is the first slot in ERC20
wantValue: "0xd3c21bcecceda1000000", // = 1000000 * (10**18), initial supply
},
{
name: "sad: address which is not in contract storage",
contractAddr: s.fundedAccEthAddr,
address: recipient,
blockNumber: deployContractBlockNumber,
slot: 0,
wantValue: "0x0",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
proof, err := s.backend.GetProof(
tc.contractAddr,
[]string{generateStorageKey(tc.address, tc.slot)},
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(proof)
s.Require().Equal(
gethcommon.HexToHash(tc.wantValue),
proof.StorageProof[0].Value
)
})
}
}


func (s *BackendSuite) TestGetStorageAt() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
address gethcommon.Address
slot uint64
wantValue string
}{
{
name: "happy: balance of the contract deployer",
contractAddr: testContractAddress,
address: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
// _balances is the first slot in ERC20
slot: 0,
// = 1000000 * (10**18), initial supply
wantValue: "0x00000000000000000000000000000000000000000000d3c21bcecceda1000000",
},
{
name: "sad: address which is not in contract storage",
contractAddr: s.fundedAccEthAddr,
address: recipient,
blockNumber: deployContractBlockNumber,
slot: 0,
wantValue: "0x0000000000000000000000000000000000000000000000000000000000000000",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
value, err := s.backend.GetStorageAt(
tc.contractAddr,
generateStorageKey(tc.address, tc.slot),
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(value)
s.Require().Equal(tc.wantValue, value.String())
})
}
}
Comment on lines +94 to +136
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct the comparison of storage values in TestGetStorageAt

Similar to the previous comment, when asserting the equality of storage values, use the raw byte representation or convert both values to a common format to prevent discrepancies due to formatting:

s.Require().Equal(
-    tc.wantValue,
-    value.String()
+    tc.wantValue,
+    value.Hex()
)

Alternatively, if value is a byte slice, compare it directly to the expected byte slice.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (s *BackendSuite) TestGetStorageAt() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
address gethcommon.Address
slot uint64
wantValue string
}{
{
name: "happy: balance of the contract deployer",
contractAddr: testContractAddress,
address: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
// _balances is the first slot in ERC20
slot: 0,
// = 1000000 * (10**18), initial supply
wantValue: "0x00000000000000000000000000000000000000000000d3c21bcecceda1000000",
},
{
name: "sad: address which is not in contract storage",
contractAddr: s.fundedAccEthAddr,
address: recipient,
blockNumber: deployContractBlockNumber,
slot: 0,
wantValue: "0x0000000000000000000000000000000000000000000000000000000000000000",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
value, err := s.backend.GetStorageAt(
tc.contractAddr,
generateStorageKey(tc.address, tc.slot),
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(value)
s.Require().Equal(tc.wantValue, value.String())
})
}
}
func (s *BackendSuite) TestGetStorageAt() {
testCases := []struct {
name string
contractAddr gethcommon.Address
blockNumber rpc.BlockNumber
address gethcommon.Address
slot uint64
wantValue string
}{
{
name: "happy: balance of the contract deployer",
contractAddr: testContractAddress,
address: s.fundedAccEthAddr,
blockNumber: deployContractBlockNumber,
// _balances is the first slot in ERC20
slot: 0,
// = 1000000 * (10**18), initial supply
wantValue: "0x00000000000000000000000000000000000000000000d3c21bcecceda1000000",
},
{
name: "sad: address which is not in contract storage",
contractAddr: s.fundedAccEthAddr,
address: recipient,
blockNumber: deployContractBlockNumber,
slot: 0,
wantValue: "0x0000000000000000000000000000000000000000000000000000000000000000",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
value, err := s.backend.GetStorageAt(
tc.contractAddr,
generateStorageKey(tc.address, tc.slot),
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(value)
s.Require().Equal(tc.wantValue, value.Hex())
})
}
}


func (s *BackendSuite) TestGetBalance() {
testCases := []struct {
name string
blockNumber rpc.BlockNumber
address gethcommon.Address
wantPositiveBalance bool
}{
{
name: "happy: funded account balance",
address: s.fundedAccEthAddr,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: true,
},
{
name: "happy: recipient balance at block 1",
address: recipient,
blockNumber: rpc.NewBlockNumber(big.NewInt(1)),
wantPositiveBalance: false,
},
{
name: "happy: recipient balance after transfer",
address: recipient,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: true,
},
{
name: "sad: not existing account",
address: evmtest.NewEthPrivAcc().EthAddr,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: false,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
balance, err := s.backend.GetBalance(
tc.address,
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(balance)
if tc.wantPositiveBalance {
s.Require().Greater(balance.ToInt().Int64(), int64(0))
} else {
s.Require().Equal(balance.ToInt().Int64(), int64(0))
}
})
}
}
Comment on lines +138 to +187
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid potential overflow in balance comparisons by using Cmp method

In the TestGetBalance function, using Int64() may lead to incorrect results if the balance exceeds the maximum value of int64. To safely compare big integers, use the Cmp method:

if tc.wantPositiveBalance {
-    s.Require().Greater(balance.ToInt().Int64(), int64(0))
+    s.Require().Equal(1, balance.ToInt().Cmp(big.NewInt(0)))
} else {
-    s.Require().Equal(balance.ToInt().Int64(), int64(0))
+    s.Require().Equal(0, balance.ToInt().Cmp(big.NewInt(0)))
}

This approach accurately compares big integers without the risk of overflow.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (s *BackendSuite) TestGetBalance() {
testCases := []struct {
name string
blockNumber rpc.BlockNumber
address gethcommon.Address
wantPositiveBalance bool
}{
{
name: "happy: funded account balance",
address: s.fundedAccEthAddr,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: true,
},
{
name: "happy: recipient balance at block 1",
address: recipient,
blockNumber: rpc.NewBlockNumber(big.NewInt(1)),
wantPositiveBalance: false,
},
{
name: "happy: recipient balance after transfer",
address: recipient,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: true,
},
{
name: "sad: not existing account",
address: evmtest.NewEthPrivAcc().EthAddr,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: false,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
balance, err := s.backend.GetBalance(
tc.address,
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(balance)
if tc.wantPositiveBalance {
s.Require().Greater(balance.ToInt().Int64(), int64(0))
} else {
s.Require().Equal(balance.ToInt().Int64(), int64(0))
}
})
}
}
func (s *BackendSuite) TestGetBalance() {
testCases := []struct {
name string
blockNumber rpc.BlockNumber
address gethcommon.Address
wantPositiveBalance bool
}{
{
name: "happy: funded account balance",
address: s.fundedAccEthAddr,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: true,
},
{
name: "happy: recipient balance at block 1",
address: recipient,
blockNumber: rpc.NewBlockNumber(big.NewInt(1)),
wantPositiveBalance: false,
},
{
name: "happy: recipient balance after transfer",
address: recipient,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: true,
},
{
name: "sad: not existing account",
address: evmtest.NewEthPrivAcc().EthAddr,
blockNumber: transferTxBlockNumber,
wantPositiveBalance: false,
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
balance, err := s.backend.GetBalance(
tc.address,
rpc.BlockNumberOrHash{
BlockNumber: &tc.blockNumber,
},
)
s.Require().NoError(err)
s.Require().NotNil(balance)
if tc.wantPositiveBalance {
s.Require().Equal(1, balance.ToInt().Cmp(big.NewInt(0)))
} else {
s.Require().Equal(0, balance.ToInt().Cmp(big.NewInt(0)))
}
})
}
}


// generateStorageKey produces the storage key from address and slot (order of the variable in solidity declaration)
func generateStorageKey(key gethcommon.Address, slot uint64) string {
// Prepare the key and slot as 32-byte values
keyBytes := gethcommon.LeftPadBytes(key.Bytes(), 32)
slotBytes := gethcommon.LeftPadBytes(new(big.Int).SetUint64(slot).Bytes(), 32)

// Concatenate key and slot
data := append(keyBytes, slotBytes...)

// Hash the data using Keccak256
hash := sha3.NewLegacyKeccak256()
hash.Write(data)
return gethcommon.BytesToHash(hash.Sum(nil)).Hex()
}
6 changes: 1 addition & 5 deletions eth/rpc/backend/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@ func NewBackend(
allowUnprotectedTxs bool,
indexer eth.EVMTxIndexer,
) *Backend {
chainID, err := eth.ParseEthChainID(clientCtx.ChainID)
if err != nil {
panic(err)
}

chainID := eth.ParseEthChainID(clientCtx.ChainID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Handle the potential error returned by eth.ParseEthChainID.

The updated code directly assigns the result of eth.ParseEthChainID(clientCtx.ChainID) to chainID without checking for errors. If eth.ParseEthChainID returns an error, it will not be caught and handled properly, which could lead to unexpected behavior or crashes.

Consider handling the error to ensure proper error handling and prevent potential issues:

chainID, err := eth.ParseEthChainID(clientCtx.ChainID)
if err != nil {
    // Handle the error appropriately, e.g., return an error or use a default value
    // panic(err) // Only panic if it's a critical error that should stop execution
}

appConf, err := config.GetConfig(ctx.Viper)
if err != nil {
panic(err)
Expand Down
Loading
Loading