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

fix(evm): commit temporary state on precompile start, avoiding bug stemming from uncommitted, dirty journal entries in the EVM StateDB #2086

Merged
merged 17 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
39 changes: 25 additions & 14 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### State Machine Breaking
### Nibiru EVM

#### For next mainnet version
#### Nibiru EVM | Before Audit 2 [Nov, 2024]

- [#1766](https://github.com/NibiruChain/nibiru/pull/1766) - refactor(app-wasmext)!: remove wasmbinding `CosmosMsg::Custom` bindings.
- [#1776](https://github.com/NibiruChain/nibiru/pull/1776) - feat(inflation): make inflation params a collection and add commands to update them
- [#1872](https://github.com/NibiruChain/nibiru/pull/1872) - chore(math): use cosmossdk.io/math to replace sdk types
- [#1874](https://github.com/NibiruChain/nibiru/pull/1874) - chore(proto): remove the proto stringer as per Cosmos SDK migration guidelines
- [#1932](https://github.com/NibiruChain/nibiru/pull/1932) - fix(gosdk): fix keyring import functions
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
preparation for a second audit starting in November 2024.

- [#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)
and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724).
- [#2086](https://github.com/NibiruChain/nibiru/pull/2086) - fix(evm-precompiles): It is possible for the `Run` function of a custom precompile to retrieve stale state because EVM state changes can occur before the precompile is called that are recorded as entries in the StateDB journal for the transaction without being reflected in the `sdk.Context`. This pull request makes sure that state is committed as expected.

#### Nibiru EVM
#### Nibiru EVM | Before Audit 1 - 2024-10-18

- [#1837](https://github.com/NibiruChain/nibiru/pull/1837) - feat(eth): protos, eth types, and evm module types
- [#1838](https://github.com/NibiruChain/nibiru/pull/1838) - feat(eth): Go-ethereum, crypto, encoding, and unit tests for evm/types
Expand All @@ -70,7 +77,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#1909](https://github.com/NibiruChain/nibiru/pull/1909) - chore(evm): set is_london true by default and removed from config
- [#1911](https://github.com/NibiruChain/nibiru/pull/1911) - chore(evm): simplified config by removing old eth forks
- [#1912](https://github.com/NibiruChain/nibiru/pull/1912) - test(evm): unit tests for evm_ante
- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler- [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code.
- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler
- [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code.
- [#1922](https://github.com/NibiruChain/nibiru/pull/1922) - feat(evm): tracer option is read from the config.
- [#1936](https://github.com/NibiruChain/nibiru/pull/1936) - feat(evm): EVM fungible token protobufs and encoding tests
Expand Down Expand Up @@ -128,12 +135,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#2060](https://github.com/NibiruChain/nibiru/pull/2060) - fix(evm-precompiles): add assertNumArgs validation
- [#2056](https://github.com/NibiruChain/nibiru/pull/2056) - feat(evm): add oracle precompile
- [#2065](https://github.com/NibiruChain/nibiru/pull/2065) - refactor(evm)!: Refactor out dead code from the evm.Params
- [#2073](https://github.com/NibiruChain/nibiru/pull/2073) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation
- [#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)
and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724).

### State Machine Breaking (Other)

#### For next mainnet version

- [#1766](https://github.com/NibiruChain/nibiru/pull/1766) - refactor(app-wasmext)!: remove wasmbinding `CosmosMsg::Custom` bindings.
- [#1776](https://github.com/NibiruChain/nibiru/pull/1776) - feat(inflation): make inflation params a collection and add commands to update them
- [#1872](https://github.com/NibiruChain/nibiru/pull/1872) - chore(math): use cosmossdk.io/math to replace sdk types
- [#1874](https://github.com/NibiruChain/nibiru/pull/1874) - chore(proto): remove the proto stringer as per Cosmos SDK migration guidelines
- [#1932](https://github.com/NibiruChain/nibiru/pull/1932) - fix(gosdk): fix keyring import functions

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

Expand Down
2 changes: 1 addition & 1 deletion x/evm/evmtest/erc20.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func AssertERC20BalanceEqual(
) {
actualBalance, err := deps.EvmKeeper.ERC20().BalanceOf(erc20, account, deps.Ctx)
assert.NoError(t, err)
assert.Zero(t, expectedBalance.Cmp(actualBalance), "expected %s, got %s", expectedBalance, actualBalance)
assert.Equal(t, expectedBalance.String(), actualBalance.String(), "expected %s, got %s", expectedBalance, actualBalance)
}

// CreateFunTokenForBankCoin: Uses the "TestDeps.Sender" account to create a
Expand Down
6 changes: 6 additions & 0 deletions x/evm/evmtest/test_deps.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package evmtest

import (
"context"

sdk "github.com/cosmos/cosmos-sdk/types"

gethcommon "github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -54,3 +56,7 @@
func (deps *TestDeps) GethSigner() gethcore.Signer {
return gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx))
}

func (deps TestDeps) GoCtx() context.Context {
return sdk.WrapSDKContext(deps.Ctx)

Check warning on line 61 in x/evm/evmtest/test_deps.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/test_deps.go#L60-L61

Added lines #L60 - L61 were not covered by tests
}
86 changes: 70 additions & 16 deletions x/evm/evmtest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@

srvconfig "github.com/NibiruChain/nibiru/v2/app/server/config"

"github.com/cosmos/cosmos-sdk/crypto/keyring"

"github.com/NibiruChain/nibiru/v2/x/evm"
"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
"github.com/NibiruChain/nibiru/v2/x/evm/statedb"
)

type GethTxType = uint8
Expand Down Expand Up @@ -123,7 +126,9 @@
To: &recipient,
Nonce: (*hexutil.Uint64)(&nonce),
}
ethTxMsg, err := GenerateAndSignEthTxMsg(txArgs, deps)
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(txArgs, deps, deps.Sender)
require.NoError(t, err)
err = ethTxMsg.Sign(gethSigner, krSigner)

Check warning on line 131 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L129-L131

Added lines #L129 - L131 were not covered by tests
k-yang marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)
Expand Down Expand Up @@ -153,18 +158,20 @@
bytecodeForCall := append(contract.Bytecode, packedArgs...)

nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr)
msgEthTx, err := GenerateAndSignEthTxMsg(
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(

Check warning on line 161 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L161

Added line #L161 was not covered by tests
evm.JsonTxArgs{
Nonce: (*hexutil.Uint64)(&nonce),
Input: (*hexutil.Bytes)(&bytecodeForCall),
From: &deps.Sender.EthAddr,
}, deps,
}, deps, deps.Sender,

Check warning on line 166 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L166

Added line #L166 was not covered by tests
)
if err != nil {
return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")
} else if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil {
return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")

Check warning on line 171 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L170-L171

Added lines #L170 - L171 were not covered by tests
Comment on lines +160 to +170
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Error Handling Improvement

In the DeployContract function, error handling can be streamlined by checking for the error immediately after the function call.

Apply this diff to enhance error handling:

 ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(
 	evm.JsonTxArgs{
 		Nonce: (*hexutil.Uint64)(&nonce),
 		Input: (*hexutil.Bytes)(&bytecodeForCall),
 		From:  &deps.Sender.EthAddr,
-	}, deps, deps.Sender,
-)
-if err != nil {
-	return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")
-} else if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil {
+	}, deps, deps.Sender)
+if err != nil {
+	return nil, errors.Wrap(err, "failed to generate eth tx msg")
+}
+if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil {
 	return nil, errors.Wrap(err, "failed to sign eth tx msg")
 }
📝 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
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(
evm.JsonTxArgs{
Nonce: (*hexutil.Uint64)(&nonce),
Input: (*hexutil.Bytes)(&bytecodeForCall),
From: &deps.Sender.EthAddr,
}, deps,
}, deps, deps.Sender,
)
if err != nil {
return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")
} else if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil {
return nil, errors.Wrap(err, "failed to generate and sign eth tx msg")
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(
evm.JsonTxArgs{
Nonce: (*hexutil.Uint64)(&nonce),
Input: (*hexutil.Bytes)(&bytecodeForCall),
From: &deps.Sender.EthAddr,
}, deps, deps.Sender)
if err != nil {
return nil, errors.Wrap(err, "failed to generate eth tx msg")
}
if err := ethTxMsg.Sign(gethSigner, krSigner); err != nil {
return nil, errors.Wrap(err, "failed to sign eth tx msg")
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 161-161: x/evm/evmtest/tx.go#L161
Added line #L161 was not covered by tests


[warning] 166-166: x/evm/evmtest/tx.go#L166
Added line #L166 was not covered by tests


[warning] 170-171: x/evm/evmtest/tx.go#L170-L171
Added lines #L170 - L171 were not covered by tests

}

resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), msgEthTx)
resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)

Check warning on line 174 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L174

Added line #L174 was not covered by tests
if err != nil {
return nil, errors.Wrap(err, "failed to execute ethereum tx")
}
Expand All @@ -174,7 +181,7 @@

return &DeployContractResult{
TxResp: resp,
EthTxMsg: msgEthTx,
EthTxMsg: ethTxMsg,

Check warning on line 184 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L184

Added line #L184 was not covered by tests
ContractData: contract,
Nonce: nonce,
ContractAddr: crypto.CreateAddress(deps.Sender.EthAddr, nonce),
Expand Down Expand Up @@ -210,23 +217,70 @@
Nonce: (*hexutil.Uint64)(&nonce),
Data: (*hexutil.Bytes)(&input),
}
erc20Transfer, err = GenerateAndSignEthTxMsg(txArgs, deps)
erc20Transfer, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(txArgs, deps, deps.Sender)
require.NoError(t, err)
err = erc20Transfer.Sign(gethSigner, krSigner)

Check warning on line 222 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L220-L222

Added lines #L220 - L222 were not covered by tests
onikonychev marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), erc20Transfer)
resp, err := deps.App.EvmKeeper.EthereumTx(deps.GoCtx(), erc20Transfer)

Check warning on line 225 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L225

Added line #L225 was not covered by tests
require.NoError(t, err)
require.Empty(t, resp.VmError)

return erc20Transfer, predecessors
}

// GenerateAndSignEthTxMsg estimates gas, sets gas limit and sings the tx
func GenerateAndSignEthTxMsg(
jsonTxArgs evm.JsonTxArgs, deps *TestDeps,
) (*evm.MsgEthereumTx, error) {
func CallContractTx(
deps *TestDeps,
contractAddr gethcommon.Address,
input []byte,
sender EthPrivKeyAcc,
) (ethTxMsg *evm.MsgEthereumTx, resp *evm.MsgEthereumTxResponse, err error) {
nonce := deps.StateDB().GetNonce(sender.EthAddr)
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(evm.JsonTxArgs{
From: &sender.EthAddr,
To: &contractAddr,
Nonce: (*hexutil.Uint64)(&nonce),
Data: (*hexutil.Bytes)(&input),
}, deps, sender)
if err != nil {
err = fmt.Errorf("CallContract error during tx generation: %w", err)
return
}

Check warning on line 248 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L237-L248

Added lines #L237 - L248 were not covered by tests

txConfig := deps.EvmKeeper.TxConfig(deps.Ctx, gethcommon.HexToHash(ethTxMsg.Hash))
stateDB := statedb.New(deps.Ctx, &deps.EvmKeeper, txConfig)
err = stateDB.Commit()
if err != nil {
return
}

Check warning on line 255 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L250-L255

Added lines #L250 - L255 were not covered by tests

err = ethTxMsg.Sign(gethSigner, krSigner)
if err != nil {
err = fmt.Errorf("CallContract error during signature: %w", err)
return
}

Check warning on line 261 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L257-L261

Added lines #L257 - L261 were not covered by tests

resp, err = deps.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg)
return ethTxMsg, resp, err

Check warning on line 264 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L263-L264

Added lines #L263 - L264 were not covered by tests
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Streamline the CallContractTx Function

The CallContractTx function contains redundant state commits and error handling that can be improved for clarity and efficiency.

Consider the following refactor:

  • Remove the manual stateDB commit; it may not be necessary as the state is managed within EthereumTx.
  • Consolidate error checks immediately after function calls for better readability.

Apply this diff to implement the refactor:

 func CallContractTx(
 	deps *TestDeps,
 	contractAddr gethcommon.Address,
 	input []byte,
 	sender EthPrivKeyAcc,
 ) (ethTxMsg *evm.MsgEthereumTx, resp *evm.MsgEthereumTxResponse, err error) {
 	nonce := deps.StateDB().GetNonce(sender.EthAddr)
 	ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(evm.JsonTxArgs{
 		From:  &sender.EthAddr,
 		To:    &contractAddr,
 		Nonce: (*hexutil.Uint64)(&nonce),
 		Data:  (*hexutil.Bytes)(&input),
 	}, deps, sender)
 	if err != nil {
 		err = fmt.Errorf("CallContract error during tx generation: %w", err)
 		return
 	}

-	txConfig := deps.EvmKeeper.TxConfig(deps.Ctx, gethcommon.HexToHash(ethTxMsg.Hash))
-	stateDB := statedb.New(deps.Ctx, &deps.EvmKeeper, txConfig)
-	err = stateDB.Commit()
-	if err != nil {
-		return
-	}

 	err = ethTxMsg.Sign(gethSigner, krSigner)
 	if err != nil {
 		err = fmt.Errorf("CallContract error during signature: %w", err)
 		return
 	}

 	resp, err = deps.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg)
 	return ethTxMsg, resp, 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
func CallContractTx(
deps *TestDeps,
contractAddr gethcommon.Address,
input []byte,
sender EthPrivKeyAcc,
) (ethTxMsg *evm.MsgEthereumTx, resp *evm.MsgEthereumTxResponse, err error) {
nonce := deps.StateDB().GetNonce(sender.EthAddr)
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(evm.JsonTxArgs{
From: &sender.EthAddr,
To: &contractAddr,
Nonce: (*hexutil.Uint64)(&nonce),
Data: (*hexutil.Bytes)(&input),
}, deps, sender)
if err != nil {
err = fmt.Errorf("CallContract error during tx generation: %w", err)
return
}
txConfig := deps.EvmKeeper.TxConfig(deps.Ctx, gethcommon.HexToHash(ethTxMsg.Hash))
stateDB := statedb.New(deps.Ctx, &deps.EvmKeeper, txConfig)
err = stateDB.Commit()
if err != nil {
return
}
err = ethTxMsg.Sign(gethSigner, krSigner)
if err != nil {
err = fmt.Errorf("CallContract error during signature: %w", err)
return
}
resp, err = deps.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg)
return ethTxMsg, resp, err
}
func CallContractTx(
deps *TestDeps,
contractAddr gethcommon.Address,
input []byte,
sender EthPrivKeyAcc,
) (ethTxMsg *evm.MsgEthereumTx, resp *evm.MsgEthereumTxResponse, err error) {
nonce := deps.StateDB().GetNonce(sender.EthAddr)
ethTxMsg, gethSigner, krSigner, err := GenerateEthTxMsgAndSigner(evm.JsonTxArgs{
From: &sender.EthAddr,
To: &contractAddr,
Nonce: (*hexutil.Uint64)(&nonce),
Data: (*hexutil.Bytes)(&input),
}, deps, sender)
if err != nil {
err = fmt.Errorf("CallContract error during tx generation: %w", err)
return
}
err = ethTxMsg.Sign(gethSigner, krSigner)
if err != nil {
err = fmt.Errorf("CallContract error during signature: %w", err)
return
}
resp, err = deps.EvmKeeper.EthereumTx(deps.GoCtx(), ethTxMsg)
return ethTxMsg, resp, err
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 237-248: x/evm/evmtest/tx.go#L237-L248
Added lines #L237 - L248 were not covered by tests


[warning] 250-255: x/evm/evmtest/tx.go#L250-L255
Added lines #L250 - L255 were not covered by tests


[warning] 257-261: x/evm/evmtest/tx.go#L257-L261
Added lines #L257 - L261 were not covered by tests


[warning] 263-264: x/evm/evmtest/tx.go#L263-L264
Added lines #L263 - L264 were not covered by tests


// GenerateEthTxMsgAndSigner estimates gas, sets gas limit and returns signer for
// the tx.
//
// Usage:
//
// ```go
// evmTxMsg, gethSigner, krSigner, _ := GenerateEthTxMsgAndSigner(
// jsonTxArgs, &deps, sender,
// )
// err := evmTxMsg.Sign(gethSigner, sender.KeyringSigner)
// ```
func GenerateEthTxMsgAndSigner(
jsonTxArgs evm.JsonTxArgs, deps *TestDeps, sender EthPrivKeyAcc,
) (evmTxMsg *evm.MsgEthereumTx, gethSigner gethcore.Signer, krSigner keyring.Signer, err error) {

Check warning on line 280 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L280

Added line #L280 was not covered by tests
estimateArgs, err := json.Marshal(&jsonTxArgs)
if err != nil {
return nil, err
return

Check warning on line 283 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L283

Added line #L283 was not covered by tests
}
res, err := deps.App.EvmKeeper.EstimateGas(
sdk.WrapSDKContext(deps.Ctx),
Expand All @@ -238,13 +292,13 @@
},
)
if err != nil {
return nil, err
return

Check warning on line 295 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L295

Added line #L295 was not covered by tests
}
jsonTxArgs.Gas = (*hexutil.Uint64)(&res.Gas)

msgEthTx := jsonTxArgs.ToMsgEthTx()
gethSigner := gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx))
return msgEthTx, msgEthTx.Sign(gethSigner, deps.Sender.KeyringSigner)
evmTxMsg = jsonTxArgs.ToMsgEthTx()
gethSigner = gethcore.LatestSignerForChainID(deps.App.EvmKeeper.EthChainID(deps.Ctx))
return evmTxMsg, gethSigner, sender.KeyringSigner, nil

Check warning on line 301 in x/evm/evmtest/tx.go

View check run for this annotation

Codecov / codecov/patch

x/evm/evmtest/tx.go#L299-L301

Added lines #L299 - L301 were not covered by tests
}

func TransferWei(
Expand Down
6 changes: 5 additions & 1 deletion x/evm/keeper/erc20.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ func (k Keeper) CallContractWithInput(
return nil, errors.Wrapf(err, "failed to load evm config")
}

txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash()))
blockHash := gethcommon.BytesToHash(ctx.HeaderHash())
txConfig := statedb.NewEmptyTxConfig(blockHash)
txConfig.TxIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0))
txConfig.LogIndex = uint(k.EvmState.BlockLogSize.GetOr(ctx, 0))
Unique-Divine marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure TxIndex and LogIndex are correctly set

You have assigned both txConfig.TxIndex and txConfig.LogIndex to k.EvmState.BlockLogSize.GetOr(ctx, 0). Typically, TxIndex represents the index of the transaction within the block, while LogIndex refers to the index of the log within the transaction. Using the same value for both may not accurately reflect their distinct purposes. Please verify that this alignment is intentional and consistent with how these indices are used elsewhere in the EVM state management.

evmResp, err = k.ApplyEvmMsg(
ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig,
)
Expand Down
2 changes: 1 addition & 1 deletion x/evm/keeper/funtoken_from_coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
// Check 3: erc-20 balance
balance, err = deps.EvmKeeper.ERC20().BalanceOf(funTokenErc20Addr.Address, alice.EthAddr, deps.Ctx)
s.Require().NoError(err)
s.Require().Zero(balance.Cmp(big.NewInt(0)))
s.Require().Equal("0", balance.String())

s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds")
_, err = deps.EvmKeeper.CallContract(
Expand Down
15 changes: 8 additions & 7 deletions x/evm/keeper/funtoken_from_erc20_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() {
s.ErrorContains(err, "either the \"from_erc20\" or \"from_bank_denom\" must be set")
}

func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank() {
deps := evmtest.NewTestDeps()

s.T().Log("Deploy ERC20")
Expand Down Expand Up @@ -210,7 +210,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {

randomAcc := testutil.AccAddress()

s.T().Log("send erc20 tokens to cosmos")
s.T().Log("send erc20 tokens to Bank")
_, err = deps.EvmKeeper.CallContract(
deps.Ctx,
embeds.SmartContract_FunToken.ABI,
Expand All @@ -231,8 +231,8 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount,
)

s.T().Log("sad: send too many erc20 tokens to cosmos")
_, err = deps.EvmKeeper.CallContract(
s.T().Log("sad: send too many erc20 tokens to Bank")
evmResp, err := deps.EvmKeeper.CallContract(
deps.Ctx,
embeds.SmartContract_FunToken.ABI,
deps.Sender.EthAddr,
Expand All @@ -243,9 +243,10 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
big.NewInt(70_000),
randomAcc.String(),
)
s.Require().Error(err)
s.T().Log("check balances")
s.Require().Error(err, evmResp.String())

s.T().Log("send cosmos tokens back to erc20")
s.T().Log("send Bank tokens back to erc20")
_, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx),
&evm.MsgConvertCoinToEvm{
ToEthAddr: eth.EIP55Addr{
Expand All @@ -264,7 +265,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToCosmos() {
deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount.Equal(sdk.NewInt(0)),
)

s.T().Log("sad: send too many cosmos tokens back to erc20")
s.T().Log("sad: send too many Bank tokens back to erc20")
_, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx),
&evm.MsgConvertCoinToEvm{
ToEthAddr: eth.EIP55Addr{
Expand Down
8 changes: 0 additions & 8 deletions x/evm/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@ import (
"github.com/NibiruChain/nibiru/v2/eth"
)

// NewTransactionLogs creates a new NewTransactionLogs instance.
func NewTransactionLogs(hash gethcommon.Hash, logs []*Log) TransactionLogs {
return TransactionLogs{
Hash: hash.String(),
Logs: logs,
}
}

// NewTransactionLogsFromEth creates a new NewTransactionLogs instance using []*ethtypes.Log.
func NewTransactionLogsFromEth(hash gethcommon.Hash, ethlogs []*gethcore.Log) TransactionLogs {
return TransactionLogs{
Expand Down
5 changes: 4 additions & 1 deletion x/evm/logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,10 @@ func TestConversionFunctions(t *testing.T) {
conversionErr := conversionLogs.Validate()

// create new transaction logs as copy of old valid one (and validate)
copyLogs := evm.NewTransactionLogs(common.BytesToHash([]byte("tx_hash")), txLogs.Logs)
copyLogs := evm.TransactionLogs{
Hash: common.BytesToHash([]byte("tx_hash")).Hex(),
Logs: txLogs.Logs,
}
copyErr := copyLogs.Validate()

require.Nil(t, conversionErr)
Expand Down
13 changes: 13 additions & 0 deletions x/evm/precompile/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,24 @@
import (
"errors"
"fmt"
"reflect"

gethabi "github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/core/vm"
)

// ErrPrecompileRun is error function intended for use in a `defer` pattern,
// which modifies the input error in the event that its value becomes non-nil.
// This creates a concise way to prepend extra information to the original error.
func ErrPrecompileRun(err error, p vm.PrecompiledContract) func() {
return func() {
if err != nil {
precompileType := reflect.TypeOf(p).Name()
err = fmt.Errorf("precompile error: failed to run %s: %w", precompileType, err)
}

Check warning on line 20 in x/evm/precompile/errors.go

View check run for this annotation

Codecov / codecov/patch

x/evm/precompile/errors.go#L18-L20

Added lines #L18 - L20 were not covered by tests
}
}

// Error short-hand for type validation
func ErrArgTypeValidation(solidityHint string, arg any) error {
return fmt.Errorf("type validation failed for (%s) argument: %s", solidityHint, arg)
Expand Down
Loading
Loading