Skip to content

Commit

Permalink
fix(evm): commit temporary state on precompile start, avoiding bug st…
Browse files Browse the repository at this point in the history
…emming from uncommitted, dirty journal entries in the EVM StateDB (#2086)
  • Loading branch information
Unique-Divine authored Oct 24, 2024
1 parent dd27f4b commit 7c1b12b
Show file tree
Hide file tree
Showing 33 changed files with 1,298 additions and 686 deletions.
46 changes: 29 additions & 17 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,28 @@ 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]

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).
- [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM develoment
- [#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.
- [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
- [#2089](https://github.com/NibiruChain/nibiru/pull/2089) - better handling of gas consumption within erc20 contract execution

- [#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

#### 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 +81,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,15 +139,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).
- [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM develoment
- [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
- [#2089](https://github.com/NibiruChain/nibiru/pull/2089) - better handling of gas consumption within erc20 contract execution

### 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
12 changes: 10 additions & 2 deletions x/evm/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,17 @@ var (
func NewExecErrorWithReason(revertReason []byte) *RevertError {
result := common.CopyBytes(revertReason)
reason, errUnpack := abi.UnpackRevert(result)
err := errors.New("execution reverted")

var err error
errPrefix := "execution reverted"
if errUnpack == nil {
err = fmt.Errorf("execution reverted: %v", reason)
reasonStr := reason
err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr)
} else if string(result) != "" {
reasonStr := string(result)
err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr)
} else {
err = errors.New(errPrefix)
}
return &RevertError{
error: err,
Expand Down
8 changes: 7 additions & 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 Expand Up @@ -99,3 +99,9 @@ func AssertBankBalanceEqual(
actualBalance := deps.App.BankKeeper.GetBalance(deps.Ctx, bech32Addr, denom).Amount.BigInt()
assert.Zero(t, expectedBalance.Cmp(actualBalance), "expected %s, got %s", expectedBalance, actualBalance)
}

// BigPow multiplies "amount" by 10 to the "pow10Exp".
func BigPow(amount *big.Int, pow10Exp uint8) (powAmount *big.Int) {
pow10 := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(pow10Exp)), nil)
return new(big.Int).Mul(amount, pow10)
}
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) StateDB() *statedb.StateDB {
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)
}
90 changes: 70 additions & 20 deletions x/evm/evmtest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (

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"
)
Expand Down Expand Up @@ -123,7 +125,9 @@ func ExecuteNibiTransfer(deps *TestDeps, t *testing.T) *evm.MsgEthereumTx {
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)
require.NoError(t, err)

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

nonce := deps.StateDB().GetNonce(deps.Sender.EthAddr)
msgEthTx, err := GenerateAndSignEthTxMsg(
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")
}

resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), msgEthTx)
resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), ethTxMsg)
if err != nil {
return nil, errors.Wrap(err, "failed to execute ethereum tx")
}
Expand All @@ -174,7 +180,7 @@ func DeployContract(

return &DeployContractResult{
TxResp: resp,
EthTxMsg: msgEthTx,
EthTxMsg: ethTxMsg,
ContractData: contract,
Nonce: nonce,
ContractAddr: crypto.CreateAddress(deps.Sender.EthAddr, nonce),
Expand All @@ -184,15 +190,19 @@ func DeployContract(
// DeployAndExecuteERC20Transfer deploys contract, executes transfer and returns tx hash
func DeployAndExecuteERC20Transfer(
deps *TestDeps, t *testing.T,
) (erc20Transfer *evm.MsgEthereumTx, predecessors []*evm.MsgEthereumTx) {
) (
erc20Transfer *evm.MsgEthereumTx,
predecessors []*evm.MsgEthereumTx,
contractAddr gethcommon.Address,
) {
// TX 1: Deploy ERC-20 contract
deployResp, err := DeployContract(deps, embeds.SmartContract_TestERC20)
require.NoError(t, err)
contractData := deployResp.ContractData
nonce := deployResp.Nonce

// Contract address is deterministic
contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce)
contractAddr = crypto.CreateAddress(deps.Sender.EthAddr, nonce)
deps.App.Commit()
predecessors = []*evm.MsgEthereumTx{
deployResp.EthTxMsg,
Expand All @@ -206,27 +216,67 @@ func DeployAndExecuteERC20Transfer(
nonce = deps.StateDB().GetNonce(deps.Sender.EthAddr)
txArgs := evm.JsonTxArgs{
From: &deps.Sender.EthAddr,
To: &contractAddress,
To: &contractAddr,
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)
require.NoError(t, err)

resp, err := deps.App.EvmKeeper.EthereumTx(sdk.WrapSDKContext(deps.Ctx), erc20Transfer)
resp, err := deps.App.EvmKeeper.EthereumTx(deps.GoCtx(), erc20Transfer)
require.NoError(t, err)
require.Empty(t, resp.VmError)

return erc20Transfer, predecessors
return erc20Transfer, predecessors, contractAddr
}

// 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
}

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
}

// 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) {
estimateArgs, err := json.Marshal(&jsonTxArgs)
if err != nil {
return nil, err
return
}
res, err := deps.App.EvmKeeper.EstimateGas(
sdk.WrapSDKContext(deps.Ctx),
Expand All @@ -238,13 +288,13 @@ func GenerateAndSignEthTxMsg(
},
)
if err != nil {
return nil, err
return
}
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
}

func TransferWei(
Expand Down
90 changes: 90 additions & 0 deletions x/evm/evmtest/tx_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright (c) 2023-2024 Nibi, Inc.
package evmtest_test

import (
"math/big"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ethereum/go-ethereum/crypto"

"github.com/NibiruChain/nibiru/v2/x/common/testutil/testapp"
"github.com/NibiruChain/nibiru/v2/x/evm"
"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
"github.com/NibiruChain/nibiru/v2/x/evm/evmtest"
)

func (s *Suite) TestCallContractTx() {
deps := evmtest.NewTestDeps()

s.T().Log("Deploy some ERC20")
deployArgs := []any{"name", "SYMBOL", uint8(18)}
deployResp, err := evmtest.DeployContract(
&deps,
embeds.SmartContract_ERC20Minter,
deployArgs...,
)
s.Require().NoError(err, deployResp)
contractAddr := crypto.CreateAddress(deps.Sender.EthAddr, deployResp.Nonce)
gotContractAddr := deployResp.ContractAddr
s.Require().Equal(contractAddr, gotContractAddr)

s.T().Log("expect zero balance")
{
wantBal := big.NewInt(0)
evmtest.AssertERC20BalanceEqual(
s.T(), deps, contractAddr, deps.Sender.EthAddr, wantBal,
)
}

abi := deployResp.ContractData.ABI
s.T().Log("mint some tokens")
{
amount := big.NewInt(69_420)
to := deps.Sender.EthAddr
callArgs := []any{to, amount}
input, err := abi.Pack(
"mint", callArgs...,
)
s.Require().NoError(err)
_, resp, err := evmtest.CallContractTx(
&deps,
contractAddr,
input,
deps.Sender,
)
s.Require().NoError(err)
s.Require().Empty(resp.VmError)
}

s.T().Log("expect nonzero balance")
{
wantBal := big.NewInt(69_420)
evmtest.AssertERC20BalanceEqual(
s.T(), deps, contractAddr, deps.Sender.EthAddr, wantBal,
)
}
}

func (s *Suite) TestTransferWei() {
deps := evmtest.NewTestDeps()

s.Require().NoError(testapp.FundAccount(
deps.App.BankKeeper,
deps.Ctx,
deps.Sender.NibiruAddr,
sdk.NewCoins(sdk.NewCoin(evm.EVMBankDenom, sdk.NewInt(69_420))),
))

randomAcc := evmtest.NewEthPrivAcc()
to := randomAcc.EthAddr
err := evmtest.TransferWei(&deps, to, evm.NativeToWei(big.NewInt(420)))
s.Require().NoError(err)

evmtest.AssertBankBalanceEqual(
s.T(), deps, evm.EVMBankDenom, deps.Sender.EthAddr, big.NewInt(69_000),
)

s.Run("DeployAndExecuteERC20Transfer", func() {
evmtest.DeployAndExecuteERC20Transfer(&deps, s.T())
})
}
Loading

0 comments on commit 7c1b12b

Please sign in to comment.