x/evm/keeper: invoke tx.To() once and reuse that value instead of incurring a 24B allocation for a [20]byte

[odeke-em]

Noticed while auditing the EVM module that we've got this code pattern https://github.com/tharsis/ethermint/blob/54fa882ab8e56e3de65a1a730b02e73884c26bed/x/evm/keeper/msg_server.go#L47-L48

and it looks innocent until we actually examine what it does, which is https://github.com/ethereum/go-ethereum/blob/7a0c19f813e285516f4b525305fd73b625d2dec8/core/types/transaction.go#L286-L288

of which the code for copyAddressPtr is https://github.com/ethereum/go-ethereum/blob/7a0c19f813e285516f4b525305fd73b625d2dec8/core/types/transaction.go#L631-L637

which for an array which uses a reflect.incurs each time this expense:

• 20 bytes copied
• Round up to of len(array's slice) rounded up to character size class and pointer aligned so 24 bytes on 64-bit machines

Remedy

We need to adopt the elegant Go idiom of in-condition variables with the fix being

-       if tx.To() != nil {
-               attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyRecipie
nt, tx.To().Hex()))
+       if to := tx.To(); to != nil {
+               attrs = append(attrs, sdk.NewAttribute(types.AttributeKeyRecipie
nt, to.Hex()))
        }

Exhibits

Here is a demo of how we can see how much is incurred in allocations https://go.dev/play/p/9d_F2SyixjK with results of the benchmarks before and after the change

$ benchstat before.txt after.txt 
name        old time/op    new time/op    delta
CopyAddr-8    38.4ns ± 3%    19.3ns ± 3%  -49.66%  (p=0.000 n=10+10)

name        old alloc/op   new alloc/op   delta
CopyAddr-8     48.0B ± 0%     24.0B ± 0%  -50.00%  (p=0.000 n=10+10)

name        old allocs/op  new allocs/op  delta
CopyAddr-8      2.00 ± 0%      1.00 ± 0%  -50.00%  (p=0.000 n=10+10)

then an exhibit of the benchmarks,

type Addr [20]byte

var exampleAddr = &Addr{'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o'}

func copyAddr(a *Addr) *Addr {
        if a == nil {
                return nil
        }
        c := *a
        return &c
}

func after() *Addr {
        return copyAddr(exampleAddr)
}

func before() *Addr {
        a := copyAddr(exampleAddr)
        if a == nil {
                return a
        }
        return copyAddr(exampleAddr)
}

var sink interface{}

func BenchmarkCopyAddrBefore(b *testing.B) {
        for i := 0; i < b.N; i++ {
                sink = before()
        }
        if sink == nil {
                b.Fatal("Benchmark did not run")
        }
        sink = (interface{})(nil)
}

func BenchmarkCopyAddrAfter(b *testing.B) {
        for i := 0; i < b.N; i++ {
                sink = after()
        }
        if sink == nil {
                b.Fatal("Benchmark did not run")
        }
        sink = (interface{})(nil)
}