Skip to content

Commit

Permalink
fix: stateDB should not commit on ERC20 calls
Browse files Browse the repository at this point in the history
  • Loading branch information
k-yang committed Jan 17, 2025
1 parent 0d26abc commit ab913ba
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 77 deletions.
2 changes: 1 addition & 1 deletion x/evm/evmmodule/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (s *Suite) TestExportInitGenesis() {
s.Require().NoError(err)
erc20Addr := deployResp.ContractAddr

evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
totalSupply, err := deps.EvmKeeper.ERC20().LoadERC20BigInt(
deps.Ctx, evmObj, erc20Contract.ABI, erc20Addr, "totalSupply",
)
Expand Down
4 changes: 2 additions & 2 deletions x/evm/evmtest/test_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ func (deps TestDeps) NewStateDB() *statedb.StateDB {
)
}

func (deps TestDeps) NewEVM() *vm.EVM {
func (deps TestDeps) NewEVM() (*vm.EVM, *statedb.StateDB) {
stateDB := deps.EvmKeeper.NewStateDB(deps.Ctx, statedb.NewEmptyTxConfig(gethcommon.BytesToHash(deps.Ctx.HeaderHash())))
evmObj := deps.EvmKeeper.NewEVM(deps.Ctx, MOCK_GETH_MESSAGE, deps.EvmKeeper.GetEVMConfig(deps.Ctx), evm.NewNoOpTracer(), stateDB)
return evmObj
return evmObj, stateDB
}

func (deps *TestDeps) GethSigner() gethcore.Signer {
Expand Down
4 changes: 2 additions & 2 deletions x/evm/keeper/erc20.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (e erc20Calls) Mint(
if err != nil {
return nil, err
}
return e.CallContractWithInput(ctx, evmObj, sender, &erc20Contract, true /*commit*/, contractInput, getCallGasWithLimit(ctx, Erc20GasLimitExecute))
return e.CallContractWithInput(ctx, evmObj, sender, &erc20Contract, false /*commit*/, contractInput, getCallGasWithLimit(ctx, Erc20GasLimitExecute))
}

/*
Expand Down Expand Up @@ -163,7 +163,7 @@ func (e erc20Calls) Burn(
if err != nil {
return nil, err
}
return e.CallContractWithInput(ctx, evmObj, sender, &erc20Contract, false /*commit*/, contractInput, getCallGasWithLimit(ctx, Erc20GasLimitExecute))
return e.CallContractWithInput(ctx, evmObj, sender, &erc20Contract, true /*commit*/, contractInput, getCallGasWithLimit(ctx, Erc20GasLimitExecute))
}

func (e erc20Calls) LoadERC20Name(
Expand Down
56 changes: 29 additions & 27 deletions x/evm/keeper/erc20_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,90 +12,92 @@ func (s *Suite) TestERC20Calls() {
deps := evmtest.NewTestDeps()
bankDenom := "ibc/btc"
funtoken := evmtest.CreateFunTokenForBankCoin(deps, bankDenom, &s.Suite)
contract := funtoken.Erc20Addr.Address
erc20 := funtoken.Erc20Addr.Address

s.Run("Mint tokens - Fail from non-owner", func() {
evmObj, _ := deps.NewEVM()
_, err := deps.EvmKeeper.ERC20().Mint(
contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS,
big.NewInt(69_420), deps.Ctx, deps.NewEVM(),
erc20, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS,
big.NewInt(69_420), deps.Ctx, evmObj,
)
s.ErrorContains(err, "Ownable: caller is not the owner")
})

s.Run("Mint tokens - Success", func() {
evmObj := deps.NewEVM()
s.Run("successfully mint 69420 tokens", func() {
evmObj, stateDB := deps.NewEVM()
_, err := deps.EvmKeeper.ERC20().Mint(
contract, /*erc20Addr*/
erc20, /*erc20Addr*/
evm.EVM_MODULE_ADDRESS, /*sender*/
evm.EVM_MODULE_ADDRESS, /*recipient*/
big.NewInt(69_420), /*amount*/
deps.Ctx,
evmObj,
)
s.Require().NoError(err)
stateDB.Commit()

Check failure on line 37 in x/evm/keeper/erc20_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `stateDB.Commit` is not checked (errcheck)

evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, deps.Sender.EthAddr, big.NewInt(0), "expect zero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), "expect 69420 tokens")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, deps.Sender.EthAddr, big.NewInt(0), "expect zero tokens")
})

s.Run("Transfer - Not enough funds", func() {
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
_, _, err := deps.EvmKeeper.ERC20().Transfer(
contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS,
erc20, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS,
big.NewInt(9_420), deps.Ctx, evmObj,
)
s.ErrorContains(err, "ERC20: transfer amount exceeds balance")
// balances unchanged
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, deps.Sender.EthAddr, big.NewInt(0), "expect zero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, deps.Sender.EthAddr, big.NewInt(0), "expect zero balance")
})

s.Run("Transfer - Success (sanity check)", func() {
evmObj := deps.NewEVM()
evmObj, stateDB := deps.NewEVM()
sentAmt, _, err := deps.EvmKeeper.ERC20().Transfer(
contract, /*erc20Addr*/
erc20, /*erc20Addr*/
evm.EVM_MODULE_ADDRESS, /*sender*/
deps.Sender.EthAddr, /*recipient*/
big.NewInt(9_420), /*amount*/
deps.Ctx,
evmObj,
)
s.Require().NoError(err)

stateDB.Commit()

Check failure on line 66 in x/evm/keeper/erc20_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `stateDB.Commit` is not checked (errcheck)
evmtest.AssertERC20BalanceEqualWithDescription(
s.T(), deps, evmObj, contract, deps.Sender.EthAddr, big.NewInt(9_420), "expect nonzero balance")
s.T(), deps, evmObj, erc20, deps.Sender.EthAddr, big.NewInt(9_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(
s.T(), deps, evmObj, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000), "expect nonzero balance")
s.T(), deps, evmObj, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000), "expect nonzero balance")
s.Require().EqualValues(big.NewInt(9_420), sentAmt)
})

s.Run("Burn tokens - Allowed as non-owner", func() {
evmObj := deps.NewEVM()
evmObj, stateDB := deps.NewEVM()
_, err := deps.EvmKeeper.ERC20().Burn(
contract, /*erc20Addr*/
erc20, /*erc20Addr*/
deps.Sender.EthAddr, /*sender*/
big.NewInt(6_000), /*amount*/
deps.Ctx,
evmObj,
)
s.Require().NoError(err)

evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, deps.Sender.EthAddr, big.NewInt(3_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000), "expect nonzero balance")
stateDB.Commit()

Check failure on line 84 in x/evm/keeper/erc20_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `stateDB.Commit` is not checked (errcheck)
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, deps.Sender.EthAddr, big.NewInt(3_420), "expect 3420 tokens")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000), "expect 60000 tokens")
})

s.Run("Burn tokens - Allowed as owner", func() {
evmObj := deps.NewEVM()
evmObj, stateDB := deps.NewEVM()
_, err := deps.EvmKeeper.ERC20().Burn(
contract, /*erc20Addr*/
erc20, /*erc20Addr*/
evm.EVM_MODULE_ADDRESS, /*sender*/
big.NewInt(6_000), /*amount*/
deps.Ctx,
evmObj,
)
s.Require().NoError(err)

evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, deps.Sender.EthAddr, big.NewInt(3_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(54_000), "expect nonzero balance")
stateDB.Commit()

Check failure on line 99 in x/evm/keeper/erc20_test.go

View workflow job for this annotation

GitHub Actions / lint

Error return value of `stateDB.Commit` is not checked (errcheck)
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, deps.Sender.EthAddr, big.NewInt(3_420), "expect 3420 tokens")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(54_000), "expect 54000 tokens")
})
}
20 changes: 10 additions & 10 deletions x/evm/keeper/funtoken_from_coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (s *FunTokenFromCoinSuite) TestCreateFunTokenFromCoin() {
// Compute contract address. FindERC20 should fail
nonce := deps.NewStateDB().GetNonce(deps.Sender.EthAddr)
contractAddress := crypto.CreateAddress(deps.Sender.EthAddr, nonce)
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
metadata, err := deps.EvmKeeper.FindERC20Metadata(deps.Ctx, evmObj, contractAddress)
s.Require().Error(err)
s.Require().Nil(metadata)
Expand Down Expand Up @@ -169,7 +169,7 @@ func (s *FunTokenFromCoinSuite) TestCreateFunTokenFromCoin() {

func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
deps := evmtest.NewTestDeps()
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
alice := evmtest.NewEthPrivAcc()

// Initial setup
Expand Down Expand Up @@ -236,7 +236,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
deps.Sender.NibiruAddr.String(),
)
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -264,7 +264,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
deps.ResetGasMeter()

s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds")
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -297,7 +297,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() {
// - Module account: 0 NIBI escrowed
func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
deps := evmtest.NewTestDeps()
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
bankDenom := evm.EVMBankDenom

// Initial setup
Expand Down Expand Up @@ -375,7 +375,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
newSendAmtSendToBank, /*amount*/
)
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
evmResp, err := deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -418,7 +418,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
newSendAmtSendToBank, /*amount*/
)
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
evmResp, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -473,7 +473,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() {
// - Module account: 1 NIBI escrowed (which Alice holds as 1 WNIBI)
func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() {
deps := evmtest.NewTestDeps()
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()

funToken := s.fundAndCreateFunToken(deps, 10e6)

Expand Down Expand Up @@ -531,7 +531,7 @@ func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() {
big.NewInt(9e6), /*amount*/
)
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -587,7 +587,7 @@ func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() {
// - Module account: 10 NIBI escrowed (which Test contract holds as 10 WNIBI)
func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() {
deps := evmtest.NewTestDeps()
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()

// Initial setup
funToken := s.fundAndCreateFunToken(deps, 10e6)
Expand Down
20 changes: 10 additions & 10 deletions x/evm/keeper/funtoken_from_erc20_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() {
s.Require().NoError(err)
s.Require().Equal(expectedERC20Addr, deployResp.ContractAddr)

evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()

actualMetadata, err := deps.EvmKeeper.FindERC20Metadata(deps.Ctx, evmObj, deployResp.ContractAddr)
s.Require().NoError(err)
Expand Down Expand Up @@ -201,7 +201,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank_MadeFromErc20() {
s.T().Logf("mint erc20 tokens to %s", deps.Sender.EthAddr.String())
contractInput, err := embeds.SmartContract_ERC20Minter.ABI.Pack("mint", deps.Sender.EthAddr, big.NewInt(69_420))
s.Require().NoError(err)
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand All @@ -219,7 +219,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank_MadeFromErc20() {
deps.ResetGasMeter()
contractInput, err = embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(1), randomAcc.String())
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand All @@ -243,7 +243,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank_MadeFromErc20() {
s.T().Log("sad: send too many erc20 tokens to Bank")
contractInput, err = embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(70_000), randomAcc.String())
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
evmResp, err := deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand All @@ -270,7 +270,7 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank_MadeFromErc20() {
s.Require().NoError(err)

s.T().Log("check balances")
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(69_420), "expect nonzero balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(0), "expect nonzero balance")
s.Require().True(
Expand Down Expand Up @@ -374,7 +374,7 @@ func (s *FunTokenFromErc20Suite) TestFunTokenFromERC20MaliciousTransfer() {
s.T().Log("send erc20 tokens to cosmos")
input, err := embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(1), randomAcc.String())
s.Require().NoError(err)
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -429,7 +429,7 @@ func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() {
s.T().Log("happy: call attackBalance()")
contractInput, err := embeds.SmartContract_TestInfiniteRecursionERC20.ABI.Pack("attackBalance")
s.Require().NoError(err)
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand All @@ -444,7 +444,7 @@ func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() {
s.T().Log("sad: call attackTransfer()")
contractInput, err = embeds.SmartContract_TestInfiniteRecursionERC20.ABI.Pack("attackTransfer")
s.Require().NoError(err)
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -503,7 +503,7 @@ func (s *FunTokenFromErc20Suite) TestSendERC20WithFee() {
randomAcc.String(), /*to*/
)
s.Require().NoError(err)
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
_, err = deps.EvmKeeper.CallContractWithInput(
deps.Ctx,
evmObj,
Expand Down Expand Up @@ -537,7 +537,7 @@ func (s *FunTokenFromErc20Suite) TestSendERC20WithFee() {
s.Require().NoError(err)

s.T().Log("check balances")
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(981), "expect 981 balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deployResp.ContractAddr, big.NewInt(19), "expect 19 balance")
evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(0), "expect 0 balance")
Expand Down
4 changes: 2 additions & 2 deletions x/evm/keeper/random_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// TestRandom tests the random value generation within the EVM.
func (s *Suite) TestRandom() {
deps := evmtest.NewTestDeps()
evmObj := deps.NewEVM()
evmObj, _ := deps.NewEVM()
deployResp, err := evmtest.DeployContract(&deps, embeds.SmartContract_TestRandom)
s.Require().NoError(err)
randomContractAddr := deployResp.ContractAddr
Expand All @@ -26,7 +26,7 @@ func (s *Suite) TestRandom() {

// Update block time to check that random changes
deps.Ctx = deps.Ctx.WithBlockTime(deps.Ctx.BlockTime().Add(1 * time.Second))
evmObj = deps.NewEVM()
evmObj, _ = deps.NewEVM()
random2, err := deps.EvmKeeper.ERC20().LoadERC20BigInt(
deps.Ctx, evmObj, embeds.SmartContract_TestRandom.ABI, randomContractAddr, "getRandom",
)
Expand Down
Loading

0 comments on commit ab913ba

Please sign in to comment.