From 87465e98f9dcd33a07595b65857d50a1551681a6 Mon Sep 17 00:00:00 2001 From: zhiqiangxu <652732310@qq.com> Date: Wed, 30 Oct 2024 22:22:10 +0800 Subject: [PATCH 1/2] beacon/light: remove unused CommitteeChain.signerThreshold (#30484) This field is a duplicate of UpdateScore.SignerCount and never referenced. --- beacon/light/committee_chain.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/beacon/light/committee_chain.go b/beacon/light/committee_chain.go index a8d032bb65c5..778cd3028f76 100644 --- a/beacon/light/committee_chain.go +++ b/beacon/light/committee_chain.go @@ -77,7 +77,6 @@ type CommitteeChain struct { sigVerifier committeeSigVerifier // BLS sig verifier (dummy verifier in tests) config *types.ChainConfig - signerThreshold int minimumUpdateScore types.UpdateScore enforceTime bool // enforceTime specifies whether the age of a signed header should be checked } @@ -96,14 +95,13 @@ func NewTestCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, si // clock source and signature verification for testing purposes. func newCommitteeChain(db ethdb.KeyValueStore, config *types.ChainConfig, signerThreshold int, enforceTime bool, sigVerifier committeeSigVerifier, clock mclock.Clock, unixNano func() int64) *CommitteeChain { s := &CommitteeChain{ - committeeCache: lru.NewCache[uint64, syncCommittee](10), - db: db, - sigVerifier: sigVerifier, - clock: clock, - unixNano: unixNano, - config: config, - signerThreshold: signerThreshold, - enforceTime: enforceTime, + committeeCache: lru.NewCache[uint64, syncCommittee](10), + db: db, + sigVerifier: sigVerifier, + clock: clock, + unixNano: unixNano, + config: config, + enforceTime: enforceTime, minimumUpdateScore: types.UpdateScore{ SignerCount: uint32(signerThreshold), SubPeriodIndex: params.SyncPeriodLength / 16, From 25bc07749ce21376e1023a6e16ec173fa3fc4e43 Mon Sep 17 00:00:00 2001 From: Martin HS Date: Wed, 30 Oct 2024 18:01:47 +0100 Subject: [PATCH 2/2] core/vm: speed up push and interpreter loop (#30662) Looking at the cpu profile of a burntpix benchmark, I noticed that a lot of time was spent in gas-used, in the interpreter loop. It's an actual call (not inlined), which explicitly wants to be ignored by tracing ("tracing.GasChangeIgnored"), so it can be safely and simply inlined. The other change is in `pushX`. These also do a call to `common.RightPadBytes`. I replaced that by a doing a corresponding `Lsh` on the `u256` if needed. Note: it's needed only to make the stack output look right, for fuzzers. It technically doesn't matter what we put there: if code ends on a pushdata immediate, nothing will consume the stack element. We could just as well just ignore it, if we didn't care about fuzzers (which I do). Seems quite a lot faster on burntpix, according to my runs. This PR: ``` EVM gas used: 5642735088 execution time: 34.84609475s allocations: 915683 allocated bytes: 175334088 ``` ``` EVM gas used: 5642735088 execution time: 36.671958278s allocations: 915701 allocated bytes: 175340528 ``` Master ``` EVM gas used: 5642735088 execution time: 49.349209526s allocations: 915684 allocated bytes: 175333368 ``` ``` EVM gas used: 5642735088 execution time: 46.581006598s allocations: 915681 allocated bytes: 175330728 ``` --------- Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com> Co-authored-by: Felix Lange --- core/vm/instructions.go | 12 +++--- core/vm/instructions_test.go | 72 +++++++++++++++++++++++++++++++++ core/vm/interpreter.go | 10 ++++- core/vm/runtime/runtime_test.go | 12 ++++++ 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 0e2fd52b14cf..427eb3bab5c1 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -984,13 +984,13 @@ func makePush(size uint64, pushByteSize int) executionFunc { start = min(codeLen, int(*pc+1)) end = min(codeLen, start+pushByteSize) ) - scope.Stack.push(new(uint256.Int).SetBytes( - common.RightPadBytes( - scope.Contract.Code[start:end], - pushByteSize, - )), - ) + a := new(uint256.Int).SetBytes(scope.Contract.Code[start:end]) + // Missing bytes: pushByteSize - len(pushData) + if missing := pushByteSize - (end - start); missing > 0 { + a.Lsh(a, uint(8*missing)) + } + scope.Stack.push(a) *pc += size return nil, nil } diff --git a/core/vm/instructions_test.go b/core/vm/instructions_test.go index a3f9ee81d16e..b8e62e1de5de 100644 --- a/core/vm/instructions_test.go +++ b/core/vm/instructions_test.go @@ -927,3 +927,75 @@ func TestOpMCopy(t *testing.T) { } } } + +// TestPush sanity-checks how code with immediates are handled when the code size is +// smaller than the size of the immediate. +func TestPush(t *testing.T) { + code := common.FromHex("0011223344556677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a19181716151413121") + + push32 := makePush(32, 32) + + scope := &ScopeContext{ + Memory: nil, + Stack: newstack(), + Contract: &Contract{ + Code: code, + }, + } + for i, want := range []string{ + "0x11223344556677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1", + "0x223344556677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1", + "0x3344556677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1", + "0x44556677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1", + "0x556677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1", + "0x6677889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a1", + "0x77889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a191", + "0x889900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a19181", + "0x9900aabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a1918171", + "0xaabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a191817161", + "0xaabbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a19181716151", + "0xbbccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a1918171615141", + "0xccddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a191817161514131", + "0xddeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a19181716151413121", + "0xeeff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a1918171615141312100", + "0xff0102030405060708090a0b0c0d0e0ff1e1d1c1b1a191817161514131210000", + "0x102030405060708090a0b0c0d0e0ff1e1d1c1b1a19181716151413121000000", + "0x2030405060708090a0b0c0d0e0ff1e1d1c1b1a1918171615141312100000000", + "0x30405060708090a0b0c0d0e0ff1e1d1c1b1a191817161514131210000000000", + "0x405060708090a0b0c0d0e0ff1e1d1c1b1a19181716151413121000000000000", + "0x5060708090a0b0c0d0e0ff1e1d1c1b1a1918171615141312100000000000000", + "0x60708090a0b0c0d0e0ff1e1d1c1b1a191817161514131210000000000000000", + "0x708090a0b0c0d0e0ff1e1d1c1b1a19181716151413121000000000000000000", + "0x8090a0b0c0d0e0ff1e1d1c1b1a1918171615141312100000000000000000000", + "0x90a0b0c0d0e0ff1e1d1c1b1a191817161514131210000000000000000000000", + "0xa0b0c0d0e0ff1e1d1c1b1a19181716151413121000000000000000000000000", + "0xb0c0d0e0ff1e1d1c1b1a1918171615141312100000000000000000000000000", + "0xc0d0e0ff1e1d1c1b1a191817161514131210000000000000000000000000000", + "0xd0e0ff1e1d1c1b1a19181716151413121000000000000000000000000000000", + "0xe0ff1e1d1c1b1a1918171615141312100000000000000000000000000000000", + "0xff1e1d1c1b1a191817161514131210000000000000000000000000000000000", + "0xf1e1d1c1b1a19181716151413121000000000000000000000000000000000000", + "0xe1d1c1b1a1918171615141312100000000000000000000000000000000000000", + "0xd1c1b1a191817161514131210000000000000000000000000000000000000000", + "0xc1b1a19181716151413121000000000000000000000000000000000000000000", + "0xb1a1918171615141312100000000000000000000000000000000000000000000", + "0xa191817161514131210000000000000000000000000000000000000000000000", + "0x9181716151413121000000000000000000000000000000000000000000000000", + "0x8171615141312100000000000000000000000000000000000000000000000000", + "0x7161514131210000000000000000000000000000000000000000000000000000", + "0x6151413121000000000000000000000000000000000000000000000000000000", + "0x5141312100000000000000000000000000000000000000000000000000000000", + "0x4131210000000000000000000000000000000000000000000000000000000000", + "0x3121000000000000000000000000000000000000000000000000000000000000", + "0x2100000000000000000000000000000000000000000000000000000000000000", + "0x0", + } { + pc := new(uint64) + *pc = uint64(i) + push32(pc, nil, scope) + res := scope.Stack.pop() + if have := res.Hex(); have != want { + t.Fatalf("case %d, have %v want %v", i, have, want) + } + } +} diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 793f398367a7..c40899440174 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -249,8 +249,11 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( } else if sLen > operation.maxStack { return nil, &ErrStackOverflow{stackLen: sLen, limit: operation.maxStack} } - if !contract.UseGas(cost, in.evm.Config.Tracer, tracing.GasChangeIgnored) { + // for tracing: this gas consumption event is emitted below in the debug section. + if contract.Gas < cost { return nil, ErrOutOfGas + } else { + contract.Gas -= cost } if operation.dynamicGas != nil { @@ -279,8 +282,11 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( if err != nil { return nil, fmt.Errorf("%w: %v", ErrOutOfGas, err) } - if !contract.UseGas(dynamicCost, in.evm.Config.Tracer, tracing.GasChangeIgnored) { + // for tracing: this gas consumption event is emitted below in the debug section. + if contract.Gas < dynamicCost { return nil, ErrOutOfGas + } else { + contract.Gas -= dynamicCost } // Do tracing before memory expansion diff --git a/core/vm/runtime/runtime_test.go b/core/vm/runtime/runtime_test.go index 1aefc810bdb6..97234368ee0c 100644 --- a/core/vm/runtime/runtime_test.go +++ b/core/vm/runtime/runtime_test.go @@ -514,6 +514,17 @@ func BenchmarkSimpleLoop(b *testing.B) { byte(vm.JUMP), } + loopingCode2 := []byte{ + byte(vm.JUMPDEST), // [ count ] + // push args for the call + byte(vm.PUSH4), 1, 2, 3, 4, + byte(vm.PUSH5), 1, 2, 3, 4, 5, + + byte(vm.POP), byte(vm.POP), + byte(vm.PUSH6), 0, 0, 0, 0, 0, 0, // jumpdestination + byte(vm.JUMP), + } + callRevertingContractWithInput := []byte{ byte(vm.JUMPDEST), // // push args for the call @@ -540,6 +551,7 @@ func BenchmarkSimpleLoop(b *testing.B) { benchmarkNonModifyingCode(100000000, staticCallIdentity, "staticcall-identity-100M", "", b) benchmarkNonModifyingCode(100000000, callIdentity, "call-identity-100M", "", b) benchmarkNonModifyingCode(100000000, loopingCode, "loop-100M", "", b) + benchmarkNonModifyingCode(100000000, loopingCode2, "loop2-100M", "", b) benchmarkNonModifyingCode(100000000, callInexistant, "call-nonexist-100M", "", b) benchmarkNonModifyingCode(100000000, callEOA, "call-EOA-100M", "", b) benchmarkNonModifyingCode(100000000, callRevertingContractWithInput, "call-reverting-100M", "", b)