imp(ante): MonoAnteHandler for EVM transactions #2028

fedekunze · 2023-11-10T10:27:47Z

Description

Closes #XXX

app/ante/evm/12_emit_event.go

…nto fedekunze/mono-antehandler

app/ante/evm/mono.go

app/ante/evm/12_emit_event.go

+			return ctx, errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
+		}
+
+		txIdx := uint64(i) // nosec: G701


codecov · 2023-11-10T13:37:24Z

Codecov Report

Merging #2028 (0b57976) into main (84ca6b3) will decrease coverage by 0.25%.
The diff coverage is 67.14%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2028      +/-   ##
==========================================
- Coverage   70.42%   70.17%   -0.25%     
==========================================
  Files         344      353       +9     
  Lines       25655    25948     +293     
==========================================
+ Hits        18067    18210     +143     
- Misses       6654     6792     +138     
- Partials      934      946      +12

Files	Coverage Δ
app/ante/cosmos.go	`100.00% <100.00%> (ø)`
app/ante/evm/01_setup_ctx.go	`100.00% <100.00%> (ø)`
app/ante/evm/03_global_fee.go	`100.00% <100.00%> (ø)`
app/ante/evm/05_signature_verification.go	`100.00% <100.00%> (ø)`
app/ante/evm/06_account_verification.go	`100.00% <100.00%> (ø)`
app/ante/evm/fee_checker.go	`100.00% <100.00%> (ø)`
app/ante/handler_options.go	`91.42% <ø> (+37.14%)`	⬆️
app/ante/ante.go	`40.62% <0.00%> (ø)`
app/ante/evm/10_increment_sequence.go	`92.10% <92.10%> (ø)`
app/ante/evm/08_vesting.go	`89.02% <83.33%> (ø)`
... and 9 more

app/ante/evm/mono.go

+		}
+
+		// 12. emit events
+		txIdx := uint64(i) // nosec: G701


app/ante/evm.go

facs95 · 2023-11-22T08:35:01Z

Added benchmarking tests to the PR which could also be done through another PR but the results where very good actually.

The relvant results to look at are the ones for EVM which are the ones impacted by this PR. Its a 15% increase in time/operation and ~30% increase on memory allocation efficiency with the current changes.

To tests locally run:
go test -v ./app/ante -bench=AnteHandler -benchtime=1000x -run='^$' -benchmem -count 10

goos: darwin
goarch: arm64
pkg: github.com/evmos/evmos/v15/app/ante
                                         │ /tmp/old.txt │            /tmp/new.txt             │
                                         │    sec/op    │   sec/op     vs base                │
AnteHandler/tx_type_evm_transfer_sim-12    176.5µ ±  3%   158.9µ ± 3%   -9.95% (p=0.000 n=10)
AnteHandler/tx_type_evm_transfer-12        180.8µ ±  2%   160.9µ ± 4%  -11.01% (p=0.000 n=10)
AnteHandler/tx_type_bank_msg_send_sim-12   63.85µ ± 10%   62.96µ ± 8%        ~ (p=0.353 n=10)
AnteHandler/tx_type_bank_msg_send-12       121.0µ ±  3%   120.2µ ± 2%        ~ (p=0.280 n=10)
geomean                                    125.3µ         117.9µ        -5.88%

                                         │ /tmp/old.txt │             /tmp/new.txt             │
                                         │     B/op     │     B/op      vs base                │
AnteHandler/tx_type_evm_transfer_sim-12    60.67Ki ± 2%   47.17Ki ± 3%  -22.25% (p=0.000 n=10)
AnteHandler/tx_type_evm_transfer-12        60.64Ki ± 2%   47.13Ki ± 3%  -22.28% (p=0.000 n=10)
AnteHandler/tx_type_bank_msg_send_sim-12   20.67Ki ± 8%   20.65Ki ± 8%        ~ (p=0.796 n=10)
AnteHandler/tx_type_bank_msg_send-12       27.67Ki ± 5%   27.55Ki ± 5%        ~ (p=0.971 n=10)
geomean                                    38.09Ki        33.54Ki       -11.94%

                                         │ /tmp/old.txt │            /tmp/new.txt            │
                                         │  allocs/op   │ allocs/op   vs base                │
AnteHandler/tx_type_evm_transfer_sim-12     1345.0 ± 0%   993.0 ± 0%  -26.17% (p=0.000 n=10)
AnteHandler/tx_type_evm_transfer-12         1345.0 ± 0%   993.0 ± 0%  -26.17% (p=0.000 n=10)
AnteHandler/tx_type_bank_msg_send_sim-12     460.0 ± 0%   460.0 ± 0%        ~ (p=1.000 n=10)
AnteHandler/tx_type_bank_msg_send-12         595.0 ± 0%   594.5 ± 0%        ~ (p=0.650 n=10)
geomean                                      838.8        720.6       -14.09%

testutil/integration/evmos/factory/factory.go

app/ante/ante.go

x/evm/types/utils.go

facs95 · 2023-11-22T14:26:36Z

Some tests are failing when using the mono ante handler.. still not ready for review. Will try to fix those early tomorrow

app/ante/evm.go

app/ante/evm/04_validate.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

Vvaradinov

Didn't get to fully review before it was merged but LGTM only a few nits.

Vvaradinov · 2023-11-27T08:59:13Z

app/ante/ante.go

@@ -27,7 +26,7 @@ func NewAnteHandler(options HandlerOptions) sdk.AnteHandler {
 				switch typeURL := opts[0].GetTypeUrl(); typeURL {
 				case "/ethermint.evm.v1.ExtensionOptionsEthereumTx":


nit: define these strings as constants on top of the file?

Vvaradinov · 2023-11-27T09:04:45Z

app/ante/evm/03_global_fee.go

+	return next(ctx, tx, simulate)
+}
+
+// For dynamic transactions, GetFee() uses the GasFeeCap value, which


Suggested change

// For dynamic transactions, GetFee() uses the GasFeeCap value, which

// CheckGlobalFee - For dynamic transactions, GetFee() uses the GasFeeCap value, which

Vvaradinov · 2023-11-27T09:18:35Z

app/ante/evm/09_gas_consume.go

+	return gasWanted, minPriority, nil
+}
+
+// deductFee checks if the fee payer has enough funds to pay for the fees and deducts them.


Suggested change

// deductFee checks if the fee payer has enough funds to pay for the fees and deducts them.

// DeductFee checks if the fee payer has enough funds to pay for the fees and deducts them.

fedekunze added 2 commits November 10, 2023 11:27

imp(ante): MonoAnteHandler for EVM transactions

0bceacd

cont refactor

978b5de

github-advanced-security bot found potential problems Nov 10, 2023

View reviewed changes

app/ante/evm/12_emit_event.go Fixed Show fixed Hide fixed

fedekunze added 5 commits November 10, 2023 13:02

can transfer

8e7e72e

cont refactor

ffc945e

Merge branch 'main' into fedekunze/mono-antehandler

a532871

fix from

4ebfec9

Merge branch 'fedekunze/mono-antehandler' of github.com:evmos/evmos i…

8a627e2

…nto fedekunze/mono-antehandler

github-advanced-security bot found potential problems Nov 10, 2023

View reviewed changes

consume gas

edcfccd

github-advanced-security bot found potential problems Nov 10, 2023

View reviewed changes

app/ante/evm/mono.go Fixed Show fixed Hide fixed

fedekunze added 3 commits November 10, 2023 17:20

fixes

72910dd

update setup

bc05e7a

use struct

afaaafa

github-advanced-security bot found potential problems Nov 10, 2023

View reviewed changes

app/ante/evm/mono.go

}

// 12. emit events

txIdx := uint64(i) // nosec: G701

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion Error

Potential integer overflow by integer type conversion

facs95 reviewed Nov 21, 2023

View reviewed changes

app/ante/evm.go Show resolved Hide resolved

add ante handler benchmarking tests

37f4e5a

github-advanced-security bot found potential problems Nov 22, 2023

View reviewed changes

testutil/integration/evmos/factory/factory.go Fixed Show fixed Hide fixed

facs95 and others added 2 commits November 22, 2023 17:03

check error on factory

88fb8c1

run make format

abc6358

fedekunze commented Nov 22, 2023

View reviewed changes

app/ante/ante.go Show resolved Hide resolved

facs95 marked this pull request as ready for review November 22, 2023 14:23

facs95 requested a review from a team as a code owner November 22, 2023 14:23

facs95 requested review from facs95 and 0xstepit and removed request for a team November 22, 2023 14:23

facs95 marked this pull request as draft November 22, 2023 14:25

fedekunze commented Nov 22, 2023

View reviewed changes

x/evm/types/utils.go Show resolved Hide resolved

facs95 added 7 commits November 23, 2023 20:02

make effective fee compatible with old ante

f5b4eaf

add comment

02a1bee

fix benchmark tests

55a0c57

improve benchmarking

c921891

Merge branch 'main' into fedekunze/mono-antehandler

7f401b5

add comments

c9860cc

add licenses

c025be6

facs95 marked this pull request as ready for review November 23, 2023 15:36

Merge branch 'main' into fedekunze/mono-antehandler

23ed3e8

fedekunze commented Nov 24, 2023

View reviewed changes

app/ante/evm.go Outdated Show resolved Hide resolved

app/ante/evm/04_validate.go Outdated Show resolved Hide resolved

facs95 and others added 9 commits November 25, 2023 08:45

Update app/ante/evm.go

568fcd8

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

add comment

3fd9ac4

add changelog

55bba43

add changelog

b0c7fd1

fix lint

0645f53

fix licenses

b61df92

Merge branch 'main' into fedekunze/mono-antehandler

575d191

fix werc20 tests

addc39e

fix lint

0b57976

facs95 approved these changes Nov 27, 2023

View reviewed changes

fedekunze merged commit 8a46ed8 into main Nov 27, 2023
32 of 36 checks passed

fedekunze deleted the fedekunze/mono-antehandler branch November 27, 2023 10:11

Vvaradinov reviewed Nov 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

imp(ante): MonoAnteHandler for EVM transactions #2028

imp(ante): MonoAnteHandler for EVM transactions #2028

fedekunze commented Nov 10, 2023

codecov bot commented Nov 10, 2023 •

edited

facs95 commented Nov 22, 2023 •

edited

facs95 commented Nov 22, 2023

Vvaradinov left a comment

Vvaradinov Nov 27, 2023

Vvaradinov Nov 27, 2023

Vvaradinov Nov 27, 2023

		@@ -27,7 +26,7 @@ func NewAnteHandler(options HandlerOptions) sdk.AnteHandler {
		switch typeURL := opts[0].GetTypeUrl(); typeURL {
		case "/ethermint.evm.v1.ExtensionOptionsEthereumTx":

	// For dynamic transactions, GetFee() uses the GasFeeCap value, which
	// CheckGlobalFee - For dynamic transactions, GetFee() uses the GasFeeCap value, which

	// deductFee checks if the fee payer has enough funds to pay for the fees and deducts them.
	// DeductFee checks if the fee payer has enough funds to pay for the fees and deducts them.

imp(ante): MonoAnteHandler for EVM transactions #2028

imp(ante): MonoAnteHandler for EVM transactions #2028

Conversation

fedekunze commented Nov 10, 2023

Description

codecov bot commented Nov 10, 2023 • edited

Codecov Report

facs95 commented Nov 22, 2023 • edited

facs95 commented Nov 22, 2023

Vvaradinov left a comment

Choose a reason for hiding this comment

Vvaradinov Nov 27, 2023

Choose a reason for hiding this comment

Vvaradinov Nov 27, 2023

Choose a reason for hiding this comment

Vvaradinov Nov 27, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 10, 2023 •

edited

facs95 commented Nov 22, 2023 •

edited