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.

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

core: implement EIP-7623 increase calldata cost #30946

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Dec 20, 2024

This PR builds on #29040 and updates it to the new version of the spec. I filled the EEST tests and they pass.

Link to spec: https://eips.ethereum.org/EIPS/eip-7623

@s1na s1na added the prague label Dec 20, 2024
@MariusVanDerWijden MariusVanDerWijden changed the title Core: implement EIP-7623 increase calldata cost core: implement EIP-7623 increase calldata cost Jan 3, 2025
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I was a bit stumped by the changes to the test cases in internal/ethapi/testdata, but I figured out that these tests use the MergedTestChainConfig which already enables Prague, so the gas costs do not match anymore after this, which is expected.

2dvorak added a commit to 2dvorak/kaia that referenced this pull request Jan 7, 2025
Refer to ethereum/go-ethereum#30946.
That PR is ongoing so we may need further updates.
@lightclient lightclient force-pushed the eip-7623-rebased branch 3 times, most recently from 75b4157 to 49d2543 Compare January 10, 2025 21:53
@lightclient
Copy link
Member

I rebased this into a single commit.

@lightclient
Copy link
Member

Thoughts on something like this?

diff --git a/core/state_transition.go b/core/state_transition.go
index c65739ac30..60fb000b05 100644
--- a/core/state_transition.go
+++ b/core/state_transition.go
@@ -66,14 +66,20 @@ func (result *ExecutionResult) Revert() []byte {
        return common.CopyBytes(result.ReturnData)
 }

+func ZeroBytes(data []byte) (z uint64) {
+       for _, byt := range data {
+               if byt == 0 {
+                       z++
+               }
+       }
+       return
+}
+
 // IntrinsicGas computes the 'intrinsic gas' and the number of tokens for EIP-7623
 // for a message with the given data.
-func IntrinsicGas(data []byte, accessList types.AccessList, authList []types.SetCodeAuthorization, isContractCreation, isHomestead, isEIP2028, isEIP3860 bool) (uint64, uint64, error) {
+func IntrinsicGas(data []byte, accessList types.AccessList, authList []types.SetCodeAuthorization, isContractCreation, isHomestead, isEIP2028, isEIP3860 bool) (uint64, error) {
        // Set the starting gas for the raw transaction
-       var (
-               gas    uint64
-               tokens uint64
-       )
+       var gas uint64
        if isContractCreation && isHomestead {
                gas = params.TxGasContractCreation
        } else {
@@ -83,14 +89,8 @@ func IntrinsicGas(data []byte, accessList types.AccessList, authList []types.Set
        // Bump the required gas by the amount of transactional data
        if dataLen > 0 {
                // Zero and non-zero bytes are priced differently
-               var nz uint64
-               for _, byt := range data {
-                       if byt != 0 {
-                               nz++
-                       }
-               }
-               z := dataLen - nz
-               tokens = nz*params.TokenPerNonZeroByte7623 + z
+               z := ZeroBytes(data)
+               nz := dataLen - z

                // Make sure we don't exceed uint64 for all data combinations
                nonZeroGas := params.TxDataNonZeroGasFrontier
@@ -98,19 +98,19 @@ func IntrinsicGas(data []byte, accessList types.AccessList, authList []types.Set
                        nonZeroGas = params.TxDataNonZeroGasEIP2028
                }
                if (math.MaxUint64-gas)/nonZeroGas < nz {
-                       return 0, tokens, ErrGasUintOverflow
+                       return 0, ErrGasUintOverflow
                }
                gas += nz * nonZeroGas

                if (math.MaxUint64-gas)/params.TxDataZeroGas < z {
-                       return 0, tokens, ErrGasUintOverflow
+                       return 0, ErrGasUintOverflow
                }
                gas += z * params.TxDataZeroGas

                if isContractCreation && isEIP3860 {
                        lenWords := toWordSize(dataLen)
                        if (math.MaxUint64-gas)/params.InitCodeWordGas < lenWords {
-                               return 0, tokens, ErrGasUintOverflow
+                               return 0, ErrGasUintOverflow
                        }
                        gas += lenWords * params.InitCodeWordGas
                }
@@ -122,7 +122,7 @@ func IntrinsicGas(data []byte, accessList types.AccessList, authList []types.Set
        if authList != nil {
                gas += uint64(len(authList)) * params.CallNewAccountGas
        }
-       return gas, tokens, nil
+       return gas, nil
 }

 // toWordSize returns the ceiled word size required for init code payment calculation.
@@ -423,22 +423,24 @@ func (st *stateTransition) execute() (*ExecutionResult, error) {
        )

        // Check clauses 4-5, subtract intrinsic gas if everything is correct
-       gas, dataTokens, err := IntrinsicGas(msg.Data, msg.AccessList, msg.SetCodeAuthorizations, contractCreation, rules.IsHomestead, rules.IsIstanbul, rules.IsShanghai)
+       gas, err := IntrinsicGas(msg.Data, msg.AccessList, msg.SetCodeAuthorizations, contractCreation, rules.IsHomestead, rules.IsIstanbul, rules.IsShanghai)
        if err != nil {
                return nil, err
        }
        if st.gasRemaining < gas {
                return nil, fmt.Errorf("%w: have %d, want %d", ErrIntrinsicGas, st.gasRemaining, gas)
        }
-       // Gas limit suffices for the floor data cost (EIP-7623)
+       // Ensure gas limit suffices for the floor data cost (EIP-7623)
+       var floorGas uint64
        if rules.IsPrague {
-               floorGas, err := FloorDataGas(dataTokens)
+               floor, err := DataGas(msg.Data)
                if err != nil {
                        return nil, err
                }
                if st.gasRemaining < floorGas {
                        return nil, fmt.Errorf("%w: have %d, want %d", ErrDataFloorGas, st.gasRemaining, floorGas)
                }
+               floorGas = floor
        }
        if t := st.evm.Config.Tracer; t != nil && t.OnGasChange != nil {
                t.OnGasChange(st.gasRemaining, st.gasRemaining-gas, tracing.GasChangeTxIntrinsicGas)
@@ -503,9 +505,7 @@ func (st *stateTransition) execute() (*ExecutionResult, error) {
                ret, st.gasRemaining, vmerr = st.evm.Call(sender, st.to(), msg.Data, st.gasRemaining, value)
        }
        if rules.IsPrague {
-               // After EIP-7623: Data-heavy transactions pay the floor gas.
-               // Overflow error has already been checked and can be ignored here.
-               floorGas, _ := FloorDataGas(dataTokens)
+               // Data-heavy transactions pay the floor data gas.
                if st.gasUsed() < floorGas {
                        st.gasRemaining = st.initialGas - floorGas
                }
@@ -648,9 +648,14 @@ func (st *stateTransition) blobGasUsed() uint64 {
        return uint64(len(st.msg.BlobHashes) * params.BlobTxBlobGasPerBlob)
 }

-// FloorDataGas calculates the minimum gas required for a transaction
+// DataGas calculates the minimum gas required for a transaction
 // based on its data tokens (EIP-7623).
-func FloorDataGas(tokens uint64) (uint64, error) {
+func DataGas(data []byte) (uint64, error) {
+       var (
+               z      = ZeroBytes(data)
+               nz     = uint64(len(data)) - z
+               tokens = nz*params.TokenPerNonZeroByte7623 + z
+       )
        // Check for overflow
        if (math.MaxUint64-params.TxGas)/params.CostFloorPerToken7623 < tokens {
                return 0, ErrGasUintOverflow

@fjl
Copy link
Contributor

fjl commented Jan 15, 2025

Just noting your ZeroBytes function is equivalent to bytes.Count(s, 0).

@s1na
Copy link
Contributor Author

s1na commented Jan 17, 2025

I updated the PR based on ethereum/EIPs#9227.

Co-authored-by: Marius van der Wijden <[email protected]>
Co-authored-by: Sina Mahmoodi <[email protected]>

return floor gas instead of tokens

apply floor after refunds

fix tests

fix gas return amount

fix tests
@s1na s1na force-pushed the eip-7623-rebased branch from 4a6dd6e to 81baa03 Compare January 17, 2025 10:58
@MariusVanDerWijden
Copy link
Member

Unrelated to this PR, but we should consider switching to bytes.Count() as its ~10 times faster:


func BenchmarkCountZ(b *testing.B) {
	slice := make([]byte, b.N)
	for i := 0; i < b.N/2; i++ {
		slice[i] = 0xff
	}
	b.ResetTimer()
	var z uint64
	for _, byt := range slice {
		if byt == 0 {
			z++
		}
	}
}

func BenchmarkCountZ2(b *testing.B) {
	slice := make([]byte, b.N)
	for i := 0; i < b.N/2; i++ {
		slice[i] = 0xff
	}
	b.ResetTimer()
	bytes.Count(slice, []byte{0})
}

BenchmarkCountZ-24    	1000000000	         0.4348 ns/op	       0 B/op	       0 allocs/op
BenchmarkCountZ2-24    	1000000000	         0.03267 ns/op	       0 B/op	       0 allocs/op

nz++
}
}
z := uint64(bytes.Count(data, []byte{0}))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprisingly, new approach is a lot faster!

/*
goos: darwin
goarch: arm64
pkg: github.com/ethereum/go-ethereum/core
cpu: Apple M1 Pro
BenchmarkCountZeros
BenchmarkCountZeros-8   	 3027921	       391.3 ns/op
*/
func BenchmarkCountZeros(b *testing.B) {
	var buffer [20000]byte
	rand.Read(buffer[:])

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		bytes.Count(buffer[:], []byte{0})
	}
}

func countZero(b []byte) int {
	var c int
	for i := 0; i < len(b); i++ {
		if b[i] != 0 {
			c++
		}
	}
	return c
}

/*
goos: darwin
goarch: arm64
pkg: github.com/ethereum/go-ethereum/core
cpu: Apple M1 Pro
BenchmarkCountZeros2
BenchmarkCountZeros2-8   	  190629	      6341 ns/op
*/
func BenchmarkCountZeros2(b *testing.B) {
	buf := make([]byte, 20000)
	rand.Read(buf[:])

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		countZero(buf[:])
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants