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

Problem: tx format conversions not necessary #478

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Problem: tx format conversions not necessary #478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cffa68
Problem: tx format conversions not necessary
yihuang Apr 25, 2024
e2c4800
Merge branch 'develop' into cleanup-tx
yihuang Apr 29, 2024
c323cd4
fix lint
yihuang Apr 29, 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
6 changes: 3 additions & 3 deletions app/ante/ante_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,10 @@ func (suite *AnteTestSuite) TestAnteHandler() {

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)

txData, err := evmtypes.UnpackTxData(signedTx.Data)
suite.Require().NoError(err)
txData := signedTx.AsTransaction()
suite.Require().NotNil(txData)

expFee := txData.Fee()
expFee := signedTx.GetFee()
invalidFee := new(big.Int).Add(expFee, big.NewInt(1))
invalidFeeAmount := sdk.Coins{sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewIntFromBigInt(invalidFee))}
txBuilder.SetFeeAmount(invalidFeeAmount)
Expand Down
56 changes: 20 additions & 36 deletions app/ante/eth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,13 @@
return nil
}

for i, msg := range tx.GetMsgs() {
for _, msg := range tx.GetMsgs() {
msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx)
if !ok {
return errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
if err != nil {
return errorsmod.Wrapf(err, "failed to unpack tx data any for tx %d", i)
}
ethTx := msgEthTx.AsTransaction()

// sender address should be in the tx cache from the previous AnteHandle call
from := msgEthTx.GetFrom()
Expand All @@ -76,8 +73,8 @@
"the sender is not EOA: address %s, codeHash <%s>", fromAddr, acct.CodeHash)
}

balance := evmKeeper.GetBalance(ctx, sdk.AccAddress(fromAddr.Bytes()), evmDenom)
if err := keeper.CheckSenderBalance(sdkmath.NewIntFromBigInt(balance), txData); err != nil {
balance := evmKeeper.GetBalance(ctx, from, evmDenom)
if err := keeper.CheckSenderBalance(sdkmath.NewIntFromBigIntMut(balance), ethTx); err != nil {
return errorsmod.Wrap(err, "failed to check sender balance")
}
}
Expand Down Expand Up @@ -119,26 +116,22 @@
return ctx, errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
if err != nil {
return ctx, errorsmod.Wrap(err, "failed to unpack tx data")
}

priority := evmtypes.GetTxPriority(txData, baseFee)
priority := evmtypes.GetTxPriority(msgEthTx, baseFee)

if priority < minPriority {
minPriority = priority
}

gasLimit := msgEthTx.GetGas()
if ctx.IsCheckTx() && maxGasWanted != 0 {
// We can't trust the tx gas limit, because we'll refund the unused gas.
if txData.GetGas() > maxGasWanted {
if gasLimit > maxGasWanted {

Check warning on line 128 in app/ante/eth.go

View check run for this annotation

Codecov / codecov/patch

app/ante/eth.go#L128 

Added line #L128 was not covered by tests
gasWanted += maxGasWanted
} else {
gasWanted += txData.GetGas()
gasWanted += gasLimit

Check warning on line 131 in app/ante/eth.go

View check run for this annotation

Codecov / codecov/patch

app/ante/eth.go#L131 

Added line #L131 was not covered by tests
}
} else {
gasWanted += txData.GetGas()
gasWanted += gasLimit
}

// user balance is already checked during CheckTx so there's no need to
Expand All @@ -147,7 +140,7 @@
continue
}

fees, err := keeper.VerifyFee(txData, evmDenom, baseFee, rules.IsHomestead, rules.IsIstanbul, rules.IsShanghai, ctx.IsCheckTx())
fees, err := keeper.VerifyFee(msgEthTx, evmDenom, baseFee, rules.IsHomestead, rules.IsIstanbul, rules.IsShanghai, ctx.IsCheckTx())
if err != nil {
return ctx, errorsmod.Wrapf(err, "failed to verify the fees")
}
Expand Down Expand Up @@ -212,38 +205,32 @@
return errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

coreMsg, err := msgEthTx.AsMessage(baseFee)
if err != nil {
return errorsmod.Wrapf(
err,
"failed to create an ethereum core.Message",
)
}

tx := msgEthTx.AsTransaction()
if rules.IsLondon {
if baseFee == nil {
return errorsmod.Wrap(
evmtypes.ErrInvalidBaseFee,
"base fee is supported but evm block context value is nil",
)
}
if coreMsg.GasFeeCap.Cmp(baseFee) < 0 {
if tx.GasFeeCap().Cmp(baseFee) < 0 {
return errorsmod.Wrapf(
errortypes.ErrInsufficientFee,
"max fee per gas less than block base fee (%s < %s)",
coreMsg.GasFeeCap, baseFee,
tx.GasFeeCap(), baseFee,

Check warning on line 220 in app/ante/eth.go

View check run for this annotation

Codecov / codecov/patch

app/ante/eth.go#L220 

Added line #L220 was not covered by tests
)
}
}

from := common.BytesToAddress(msgEthTx.From)
// check that caller has enough balance to cover asset transfer for **topmost** call
// NOTE: here the gas consumed is from the context with the infinite gas meter
if coreMsg.Value.Sign() > 0 && !canTransfer(ctx, evmKeeper, evmParams.EvmDenom, coreMsg.From, coreMsg.Value) {
if tx.Value().Sign() > 0 && !canTransfer(ctx, evmKeeper, evmParams.EvmDenom, from, tx.Value()) {
return errorsmod.Wrapf(
errortypes.ErrInsufficientFunds,
"failed to transfer %s from address %s using the EVM block context transfer function",
coreMsg.Value,
coreMsg.From,
tx.Value(),
from,
)
}
}
Expand All @@ -269,10 +256,7 @@
return errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
if err != nil {
return errorsmod.Wrap(err, "failed to unpack tx data")
}
tx := msgEthTx.AsTransaction()

// increase sequence of sender
acc := ak.GetAccount(ctx, msgEthTx.GetFrom())
Expand All @@ -286,10 +270,10 @@

// we merged the nonce verification to nonce increment, so when tx includes multiple messages
// with same sender, they'll be accepted.
if txData.GetNonce() != nonce {
if tx.Nonce() != nonce {
return errorsmod.Wrapf(
errortypes.ErrInvalidSequence,
"invalid nonce; got %d, expected %d", txData.GetNonce(), nonce,
"invalid nonce; got %d, expected %d", tx.Nonce(), nonce,
)
}

Expand Down
6 changes: 3 additions & 3 deletions app/ante/eth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,11 @@ func (suite *AnteTestSuite) TestEthIncrementSenderSequenceDecorator() {
suite.Require().NoError(err)
msg := tc.tx.(*evmtypes.MsgEthereumTx)

txData, err := evmtypes.UnpackTxData(msg.Data)
suite.Require().NoError(err)
txData := msg.AsTransaction()
suite.Require().NotNil(txData)

nonce := suite.app.EvmKeeper.GetNonce(suite.ctx, addr)
suite.Require().Equal(txData.GetNonce()+1, nonce)
suite.Require().Equal(txData.Nonce()+1, nonce)
} else {
suite.Require().Error(err)
}
Expand Down
13 changes: 1 addition & 12 deletions app/ante/fees.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"

ethtypes "github.com/ethereum/go-ethereum/core/types"
evmtypes "github.com/evmos/ethermint/x/evm/types"
feemarkettypes "github.com/evmos/ethermint/x/feemarket/types"
)
Expand Down Expand Up @@ -114,8 +113,6 @@ func CheckEthMinGasPrice(tx sdk.Tx, minGasPrice sdkmath.LegacyDec, baseFee *big.
)
}

feeAmt := ethMsg.GetFee()

// For dynamic transactions, GetFee() uses the GasFeeCap value, which
// is the maximum gas price that the signer can pay. In practice, the
// signer can pay less, if the block's BaseFee is lower. So, in this case,
Expand All @@ -124,15 +121,7 @@ func CheckEthMinGasPrice(tx sdk.Tx, minGasPrice sdkmath.LegacyDec, baseFee *big.
// increase the GasTipCap (priority fee) until EffectivePrice > MinGasPrices.
// Transactions with MinGasPrices * gasUsed < tx fees < EffectiveFee are rejected
// by the feemarket AnteHandle

txData, err := evmtypes.UnpackTxData(ethMsg.Data)
if err != nil {
return errorsmod.Wrapf(err, "failed to unpack tx data %s", ethMsg.Hash)
}

if txData.TxType() != ethtypes.LegacyTxType {
mmsqe marked this conversation as resolved.
Show resolved Hide resolved
feeAmt = ethMsg.GetEffectiveFee(baseFee)
}
feeAmt := ethMsg.GetEffectiveFee(baseFee)

gasLimit := sdkmath.LegacyNewDecFromBigInt(new(big.Int).SetUint64(ethMsg.GetGas()))

Expand Down
17 changes: 6 additions & 11 deletions app/ante/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,30 +107,25 @@ func ValidateEthBasic(ctx sdk.Context, tx sdk.Tx, evmParams *evmtypes.Params, ba

txGasLimit += msgEthTx.GetGas()

txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
if err != nil {
return errorsmod.Wrap(err, "failed to unpack MsgEthereumTx Data")
}

tx := msgEthTx.AsTransaction()
// return error if contract creation or call are disabled through governance
if !enableCreate && txData.GetTo() == nil {
if !enableCreate && tx.To() == nil {
return errorsmod.Wrap(evmtypes.ErrCreateDisabled, "failed to create new contract")
} else if !enableCall && txData.GetTo() != nil {
} else if !enableCall && tx.To() != nil {
return errorsmod.Wrap(evmtypes.ErrCallDisabled, "failed to call contract")
}

if baseFee == nil && txData.TxType() == ethtypes.DynamicFeeTxType {
if baseFee == nil && tx.Type() == ethtypes.DynamicFeeTxType {
return errorsmod.Wrap(ethtypes.ErrTxTypeNotSupported, "dynamic fee tx not supported")
}

ethTx := ethtypes.NewTx(txData.AsEthereumData())
if !allowUnprotectedTxs && !ethTx.Protected() {
if !allowUnprotectedTxs && !tx.Protected() {
return errorsmod.Wrapf(
errortypes.ErrNotSupported,
"rejected unprotected Ethereum transaction. Please EIP155 sign your transaction to protect it against replay-attacks")
}

txFee = txFee.Add(sdk.Coin{Denom: evmDenom, Amount: sdkmath.NewIntFromBigInt(txData.Fee())})
txFee = txFee.Add(sdk.Coin{Denom: evmDenom, Amount: sdkmath.NewIntFromBigInt(msgEthTx.GetFee())})
}

if !authInfo.Fee.Amount.Equal(txFee) {
Expand Down
6 changes: 3 additions & 3 deletions app/ante/sigs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ func (suite *AnteTestSuite) TestSignatures() {
// signatures of cosmos tx should be empty
suite.Require().Equal(len(sigs), 0)

txData, err := evmtypes.UnpackTxData(msgEthereumTx.Data)
suite.Require().NoError(err)
txData := msgEthereumTx.AsTransaction()
suite.Require().NotNil(txData)

msgV, msgR, msgS := txData.GetRawSignatureValues()
msgV, msgR, msgS := txData.RawSignatureValues()

ethTx := msgEthereumTx.AsTransaction()
ethV, ethR, ethS := ethTx.RawSignatureValues()
Expand Down
12 changes: 6 additions & 6 deletions app/ante/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,10 @@ func (suite *AnteTestSuite) CreateTestTxBuilder(
err = builder.SetMsgs(msg)
suite.Require().NoError(err)

txData, err := evmtypes.UnpackTxData(msg.Data)
suite.Require().NoError(err)
txData := msg.AsTransaction()
suite.Require().NotNil(txData)

fees := sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewIntFromBigInt(txData.Fee())))
fees := sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewIntFromBigInt(msg.GetFee())))
builder.SetFeeAmount(fees)
builder.SetGasLimit(msg.GetGas())

Expand All @@ -250,7 +250,7 @@ func (suite *AnteTestSuite) CreateTestTxBuilder(
SignMode: defaultSignMode,
Signature: nil,
},
Sequence: txData.GetNonce(),
Sequence: txData.Nonce(),
}

sigsV2 := []signing.SignatureV2{sigV2}
Expand All @@ -263,12 +263,12 @@ func (suite *AnteTestSuite) CreateTestTxBuilder(
signerData := authsigning.SignerData{
ChainID: suite.ctx.ChainID(),
AccountNumber: accNum,
Sequence: txData.GetNonce(),
Sequence: txData.Nonce(),
}
sigV2, err = tx.SignWithPrivKey(
suite.ctx,
defaultSignMode, signerData,
txBuilder, priv, suite.clientCtx.TxConfig, txData.GetNonce(),
txBuilder, priv, suite.clientCtx.TxConfig, txData.Nonce(),
)
suite.Require().NoError(err)

Expand Down
8 changes: 2 additions & 6 deletions app/signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,10 @@
if len(opts) > 0 && opts[0].GetTypeUrl() == "/ethermint.evm.v1.ExtensionOptionsEthereumTx" {
for _, msg := range tx.GetMsgs() {
if ethMsg, ok := msg.(*evmtypes.MsgEthereumTx); ok {
txData, err := evmtypes.UnpackTxData(ethMsg.Data)
if err != nil {
return nil, err
}
return []mempool.SignerData{
mempool.NewSignerData(
sdk.AccAddress(ethMsg.From),
txData.GetNonce(),
ethMsg.GetFrom(),
ethMsg.AsTransaction().Nonce(),

Check warning on line 34 in app/signer.go

View check run for this annotation

Codecov / codecov/patch

app/signer.go#L33-L34 

Added lines #L33 - L34 were not covered by tests
),
}, nil
}
Expand Down
3 changes: 2 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ replace (
// use cosmos keyring
github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
github.com/cockroachdb/pebble => github.com/cockroachdb/pebble v0.0.0-20230209160836-829675f94811
github.com/ethereum/go-ethereum => github.com/crypto-org-chain/go-ethereum v1.10.20-0.20231207063621-43cf32d91c3e
// release/v1.11.x
github.com/ethereum/go-ethereum => github.com/crypto-org-chain/go-ethereum v1.10.20-0.20240425065928-ebb09502e7a7
// Fix upstream GHSA-h395-qcrw-5vmq vulnerability.
// TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409
github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.7.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,8 +421,8 @@ github.com/crypto-org-chain/cosmos-sdk/x/tx v0.0.0-20240415105151-0108877a3201 h
github.com/crypto-org-chain/cosmos-sdk/x/tx v0.0.0-20240415105151-0108877a3201/go.mod h1:CBCU6fsRVz23QGFIQBb1DNX2DztJCf3jWyEkHY2nJQ0=
github.com/crypto-org-chain/go-block-stm v0.0.0-20240408011717-9f11af197bde h1:sQIHTJfVt5VTrF7po9eZiFkZiPjlHbFvnXtGCOoBjNM=
github.com/crypto-org-chain/go-block-stm v0.0.0-20240408011717-9f11af197bde/go.mod h1:iwQTX9xMX8NV9k3o2BiWXA0SswpsZrDk5q3gA7nWYiE=
github.com/crypto-org-chain/go-ethereum v1.10.20-0.20231207063621-43cf32d91c3e h1:vnyepPQ/m25+19xcTuBUdRxmltZ/EjVWNqEjhg7Ummk=
github.com/crypto-org-chain/go-ethereum v1.10.20-0.20231207063621-43cf32d91c3e/go.mod h1:+a8pUj1tOyJ2RinsNQD4326YS+leSoKGiG/uVVb0x6Y=
github.com/crypto-org-chain/go-ethereum v1.10.20-0.20240425065928-ebb09502e7a7 h1:V43F3JFcqG4MUThf9W/DytnPblpR6CcaLBw2Wx6zTgE=
github.com/crypto-org-chain/go-ethereum v1.10.20-0.20240425065928-ebb09502e7a7/go.mod h1:+a8pUj1tOyJ2RinsNQD4326YS+leSoKGiG/uVVb0x6Y=
github.com/danieljoos/wincred v1.2.0 h1:ozqKHaLK0W/ii4KVbbvluM91W2H3Sh0BncbUNPS7jLE=
github.com/danieljoos/wincred v1.2.0/go.mod h1:FzQLLMKBFdvu+osBrnFODiv32YGwCfx0SkRa/eYHgec=
github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
4 changes: 2 additions & 2 deletions gomod2nix.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ schema = 3
version = "v1.6.1"
hash = "sha256-zOpoaepCfPLmU9iQji/Ait+SVEHI9eF3rwtW0h/8lho="
[mod."github.com/ethereum/go-ethereum"]
version = "v1.10.20-0.20231207063621-43cf32d91c3e"
hash = "sha256-lDIqRLUrXYCb9mmFBY/+WW+ee69+IkxOgqjHVyo4ij0="
version = "v1.10.20-0.20240425065928-ebb09502e7a7"
hash = "sha256-lE4G5FaRb3MVi9FFVn+WlwsSTOB4SbjmVboKyQ5yB0A="
replaced = "github.com/crypto-org-chain/go-ethereum"
[mod."github.com/fatih/color"]
version = "v1.16.0"
Expand Down
6 changes: 4 additions & 2 deletions indexer/kv_indexer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func NewKVIndexer(db dbm.DB, logger log.Logger, clientCtx client.Context) *KVInd
// - Iterates over all the messages of the Tx
// - Builds and stores a indexer.TxResult based on parsed events for every message
func (kv *KVIndexer) IndexBlock(block *tmtypes.Block, txResults []*abci.ExecTxResult) error {
height := block.Header.Height
height := block.Height

batch := kv.db.NewBatch()
defer batch.Close()
Expand Down Expand Up @@ -94,7 +94,7 @@ func (kv *KVIndexer) IndexBlock(block *tmtypes.Block, txResults []*abci.ExecTxRe
var cumulativeGasUsed uint64
for msgIndex, msg := range tx.GetMsgs() {
ethMsg := msg.(*evmtypes.MsgEthereumTx)
txHash := common.HexToHash(ethMsg.Hash)
var txHash common.Hash

txResult := ethermint.TxResult{
Height: height,
Expand All @@ -107,6 +107,7 @@ func (kv *KVIndexer) IndexBlock(block *tmtypes.Block, txResults []*abci.ExecTxRe
// some old versions don't emit any events, so workaround here directly.
txResult.GasUsed = ethMsg.GetGas()
txResult.Failed = true
txHash = ethMsg.Hash()
} else {
parsedTx := txs.GetTxByMsgIndex(msgIndex)
if parsedTx == nil {
Expand All @@ -118,6 +119,7 @@ func (kv *KVIndexer) IndexBlock(block *tmtypes.Block, txResults []*abci.ExecTxRe
}
txResult.GasUsed = parsedTx.GasUsed
txResult.Failed = parsedTx.Failed
txHash = parsedTx.Hash
}

cumulativeGasUsed += txResult.GasUsed
Expand Down
1 change: 1 addition & 0 deletions indexer/kv_indexer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func TestKVIndexer(t *testing.T) {
{Key: "txIndex", Value: "0"},
}},
{Type: types.EventTypeEthereumTx, Attributes: []abci.EventAttribute{
{Key: "ethereumTxHash", Value: txHash.Hex()},
{Key: "amount", Value: "1000"},
{Key: "txGasUsed", Value: "21000"},
{Key: "txHash", Value: "14A84ED06282645EFBF080E0B7ED80D8D8D6A36337668A12B5F229F81CDD3F57"},
Expand Down
Loading
Loading