From a5fe7353cff959d6fcfcdd9593de19056edb9bdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 21 Oct 2024 12:45:33 +0300 Subject: [PATCH] common: drop BigMin and BigMax, they pollute our dep graph (#30645) Way back we've added `common.math.BigMin` and `common.math.BigMax`. These were kind of cute helpers, but unfortunate ones, because package all over out codebase added dependencies to this package just to avoid having to write out 3 lines of code. Because of this, we've also started having package name clashes with the stdlib `math`, which got solves even more badly by moving some helpers over ***from*** the stdlib into our custom lib (e.g. MaxUint64). The latter ones were nuked out in a previous PR and this PR nukes out BigMin and BigMax, inlining them at all call sites. As we're transitioning to uint256, if need be, we can add a min and max to that. --- common/math/big.go | 16 --------------- common/math/big_test.go | 30 ----------------------------- consensus/ethash/consensus.go | 5 +++-- consensus/misc/eip1559/eip1559.go | 15 +++++++++------ core/state_transition.go | 11 ++++++++--- core/types/transaction.go | 11 ++++++++--- core/vm/contracts.go | 22 ++++++++++++++------- graphql/graphql.go | 13 ++++++++++--- internal/ethapi/transaction_args.go | 12 +++++++----- tests/state_test_util.go | 6 ++++-- 10 files changed, 64 insertions(+), 77 deletions(-) diff --git a/common/math/big.go b/common/math/big.go index 952a58c586..825f4baec9 100644 --- a/common/math/big.go +++ b/common/math/big.go @@ -143,22 +143,6 @@ func BigPow(a, b int64) *big.Int { return r.Exp(r, big.NewInt(b), nil) } -// BigMax returns the larger of x or y. -func BigMax(x, y *big.Int) *big.Int { - if x.Cmp(y) < 0 { - return y - } - return x -} - -// BigMin returns the smaller of x or y. -func BigMin(x, y *big.Int) *big.Int { - if x.Cmp(y) > 0 { - return y - } - return x -} - // PaddedBigBytes encodes a big integer as a big-endian byte slice. The length // of the slice is at least n bytes. func PaddedBigBytes(bigint *big.Int, n int) []byte { diff --git a/common/math/big_test.go b/common/math/big_test.go index 92a322e29d..6f701b621a 100644 --- a/common/math/big_test.go +++ b/common/math/big_test.go @@ -68,36 +68,6 @@ func TestMustParseBig256(t *testing.T) { MustParseBig256("ggg") } -func TestBigMax(t *testing.T) { - a := big.NewInt(10) - b := big.NewInt(5) - - max1 := BigMax(a, b) - if max1 != a { - t.Errorf("Expected %d got %d", a, max1) - } - - max2 := BigMax(b, a) - if max2 != a { - t.Errorf("Expected %d got %d", a, max2) - } -} - -func TestBigMin(t *testing.T) { - a := big.NewInt(10) - b := big.NewInt(5) - - min1 := BigMin(a, b) - if min1 != b { - t.Errorf("Expected %d got %d", b, min1) - } - - min2 := BigMin(b, a) - if min2 != b { - t.Errorf("Expected %d got %d", b, min2) - } -} - func TestPaddedBigBytes(t *testing.T) { tests := []struct { num *big.Int diff --git a/consensus/ethash/consensus.go b/consensus/ethash/consensus.go index 0bd1a56bce..3d3c9cf918 100644 --- a/consensus/ethash/consensus.go +++ b/consensus/ethash/consensus.go @@ -24,7 +24,6 @@ import ( mapset "github.com/deckarep/golang-set/v2" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/consensus" "github.com/ethereum/go-ethereum/consensus/misc" "github.com/ethereum/go-ethereum/consensus/misc/eip1559" @@ -480,7 +479,9 @@ func calcDifficultyFrontier(time uint64, parent *types.Header) *big.Int { expDiff := periodCount.Sub(periodCount, big2) expDiff.Exp(big2, expDiff, nil) diff.Add(diff, expDiff) - diff = math.BigMax(diff, params.MinimumDifficulty) + if diff.Cmp(params.MinimumDifficulty) < 0 { + diff = params.MinimumDifficulty + } } return diff } diff --git a/consensus/misc/eip1559/eip1559.go b/consensus/misc/eip1559/eip1559.go index 84b82c4c49..a90bd744b2 100644 --- a/consensus/misc/eip1559/eip1559.go +++ b/consensus/misc/eip1559/eip1559.go @@ -22,7 +22,6 @@ import ( "math/big" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/consensus/misc" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/params" @@ -78,9 +77,10 @@ func CalcBaseFee(config *params.ChainConfig, parent *types.Header) *big.Int { num.Mul(num, parent.BaseFee) num.Div(num, denom.SetUint64(parentGasTarget)) num.Div(num, denom.SetUint64(config.BaseFeeChangeDenominator())) - baseFeeDelta := math.BigMax(num, common.Big1) - - return num.Add(parent.BaseFee, baseFeeDelta) + if num.Cmp(common.Big1) < 0 { + return num.Add(parent.BaseFee, common.Big1) + } + return num.Add(parent.BaseFee, num) } else { // Otherwise if the parent block used less gas than its target, the baseFee should decrease. // max(0, parentBaseFee * gasUsedDelta / parentGasTarget / baseFeeChangeDenominator) @@ -88,8 +88,11 @@ func CalcBaseFee(config *params.ChainConfig, parent *types.Header) *big.Int { num.Mul(num, parent.BaseFee) num.Div(num, denom.SetUint64(parentGasTarget)) num.Div(num, denom.SetUint64(config.BaseFeeChangeDenominator())) - baseFee := num.Sub(parent.BaseFee, num) - return math.BigMax(baseFee, common.Big0) + baseFee := num.Sub(parent.BaseFee, num) + if baseFee.Cmp(common.Big0) < 0 { + baseFee = common.Big0 + } + return baseFee } } diff --git a/core/state_transition.go b/core/state_transition.go index d285d03fe2..4bd3c00167 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -22,7 +22,6 @@ import ( "math/big" "github.com/ethereum/go-ethereum/common" - cmath "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/tracing" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" @@ -170,7 +169,10 @@ func TransactionToMessage(tx *types.Transaction, s types.Signer, baseFee *big.In } // If baseFee provided, set gasPrice to effectiveGasPrice. if baseFee != nil { - msg.GasPrice = cmath.BigMin(msg.GasPrice.Add(msg.GasTipCap, baseFee), msg.GasFeeCap) + msg.GasPrice = msg.GasPrice.Add(msg.GasTipCap, baseFee) + if msg.GasPrice.Cmp(msg.GasFeeCap) > 0 { + msg.GasPrice = msg.GasFeeCap + } } var err error msg.From, err = types.Sender(s, tx) @@ -461,7 +463,10 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { } effectiveTip := msg.GasPrice if rules.IsLondon { - effectiveTip = cmath.BigMin(msg.GasTipCap, new(big.Int).Sub(msg.GasFeeCap, st.evm.Context.BaseFee)) + effectiveTip = new(big.Int).Sub(msg.GasFeeCap, st.evm.Context.BaseFee) + if effectiveTip.Cmp(msg.GasTipCap) > 0 { + effectiveTip = msg.GasTipCap + } } effectiveTipU256, _ := uint256.FromBig(effectiveTip) diff --git a/core/types/transaction.go b/core/types/transaction.go index 6c8759ee69..ea6f5ad6b9 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -26,7 +26,6 @@ import ( "time" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rlp" ) @@ -355,10 +354,16 @@ func (tx *Transaction) EffectiveGasTip(baseFee *big.Int) (*big.Int, error) { } var err error gasFeeCap := tx.GasFeeCap() - if gasFeeCap.Cmp(baseFee) == -1 { + if gasFeeCap.Cmp(baseFee) < 0 { err = ErrGasFeeCapTooLow } - return math.BigMin(tx.GasTipCap(), gasFeeCap.Sub(gasFeeCap, baseFee)), err + gasFeeCap = gasFeeCap.Sub(gasFeeCap, baseFee) + + gasTipCap := tx.GasTipCap() + if gasTipCap.Cmp(gasFeeCap) < 0 { + return gasTipCap, err + } + return gasFeeCap, err } // EffectiveGasTipValue is identical to EffectiveGasTip, but does not return an diff --git a/core/vm/contracts.go b/core/vm/contracts.go index c3ce6d2af0..f54d5ab86e 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -22,7 +22,7 @@ import ( "errors" "fmt" "maps" - gomath "math" + "math" "math/big" "github.com/consensys/gnark-crypto/ecc" @@ -30,7 +30,6 @@ import ( "github.com/consensys/gnark-crypto/ecc/bls12-381/fp" "github.com/consensys/gnark-crypto/ecc/bls12-381/fr" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/tracing" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto/blake2b" @@ -399,7 +398,12 @@ func (c *bigModExp) RequiredGas(input []byte) uint64 { } adjExpLen.Add(adjExpLen, big.NewInt(int64(msb))) // Calculate the gas cost of the operation - gas := new(big.Int).Set(math.BigMax(modLen, baseLen)) + gas := new(big.Int) + if modLen.Cmp(baseLen) < 0 { + gas.Set(baseLen) + } else { + gas.Set(modLen) + } if c.eip2565 { // EIP-2565 has three changes // 1. Different multComplexity (inlined here) @@ -413,11 +417,13 @@ func (c *bigModExp) RequiredGas(input []byte) uint64 { gas.Rsh(gas, 3) gas.Mul(gas, gas) - gas.Mul(gas, math.BigMax(adjExpLen, big1)) + if adjExpLen.Cmp(big1) > 0 { + gas.Mul(gas, adjExpLen) + } // 2. Different divisor (`GQUADDIVISOR`) (3) gas.Div(gas, big3) if gas.BitLen() > 64 { - return gomath.MaxUint64 + return math.MaxUint64 } // 3. Minimum price of 200 gas if gas.Uint64() < 200 { @@ -426,11 +432,13 @@ func (c *bigModExp) RequiredGas(input []byte) uint64 { return gas.Uint64() } gas = modexpMultComplexity(gas) - gas.Mul(gas, math.BigMax(adjExpLen, big1)) + if adjExpLen.Cmp(big1) > 0 { + gas.Mul(gas, adjExpLen) + } gas.Div(gas, big20) if gas.BitLen() > 64 { - return gomath.MaxUint64 + return math.MaxUint64 } return gas.Uint64() } diff --git a/graphql/graphql.go b/graphql/graphql.go index 90d70bf142..57dfd14ecd 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -30,7 +30,6 @@ import ( "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/consensus/misc/eip1559" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" @@ -277,7 +276,11 @@ func (t *Transaction) GasPrice(ctx context.Context) hexutil.Big { if block != nil { if baseFee, _ := block.BaseFeePerGas(ctx); baseFee != nil { // price = min(gasTipCap + baseFee, gasFeeCap) - return (hexutil.Big)(*math.BigMin(new(big.Int).Add(tx.GasTipCap(), baseFee.ToInt()), tx.GasFeeCap())) + gasFeeCap, effectivePrice := tx.GasFeeCap(), new(big.Int).Add(tx.GasTipCap(), baseFee.ToInt()) + if effectivePrice.Cmp(gasFeeCap) < 0 { + return (hexutil.Big)(*effectivePrice) + } + return (hexutil.Big)(*gasFeeCap) } } return hexutil.Big(*tx.GasPrice()) @@ -302,7 +305,11 @@ func (t *Transaction) EffectiveGasPrice(ctx context.Context) (*hexutil.Big, erro if header.BaseFee == nil { return (*hexutil.Big)(tx.GasPrice()), nil } - return (*hexutil.Big)(math.BigMin(new(big.Int).Add(tx.GasTipCap(), header.BaseFee), tx.GasFeeCap())), nil + gasFeeCap, effectivePrice := tx.GasFeeCap(), new(big.Int).Add(tx.GasTipCap(), header.BaseFee) + if effectivePrice.Cmp(gasFeeCap) < 0 { + return (*hexutil.Big)(effectivePrice), nil + } + return (*hexutil.Big)(gasFeeCap), nil } func (t *Transaction) MaxFeePerGas(ctx context.Context) *hexutil.Big { diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index e848f619d9..b6346e910d 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -22,12 +22,11 @@ import ( "crypto/sha256" "errors" "fmt" - gomath "math" + "math" "math/big" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/consensus/misc/eip4844" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" @@ -141,7 +140,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend, skipGas if skipGasEstimation { // Skip gas usage estimation if a precise gas limit is not critical, e.g., in non-transaction calls. gas := hexutil.Uint64(b.RPCGasCap()) if gas == 0 { - gas = hexutil.Uint64(gomath.MaxUint64 / 2) + gas = hexutil.Uint64(math.MaxUint64 / 2) } args.Gas = &gas } else { // Estimate the gas usage otherwise. @@ -379,7 +378,7 @@ func (args *TransactionArgs) CallDefaults(globalGasCap uint64, baseFee *big.Int, if args.Gas == nil { gas := globalGasCap if gas == 0 { - gas = uint64(gomath.MaxUint64 / 2) + gas = uint64(math.MaxUint64 / 2) } args.Gas = (*hexutil.Uint64)(&gas) } else { @@ -441,7 +440,10 @@ func (args *TransactionArgs) ToMessage(baseFee *big.Int, skipNonceCheck, skipEoA // Backfill the legacy gasPrice for EVM execution, unless we're all zeroes gasPrice = new(big.Int) if gasFeeCap.BitLen() > 0 || gasTipCap.BitLen() > 0 { - gasPrice = math.BigMin(new(big.Int).Add(gasTipCap, baseFee), gasFeeCap) + gasPrice = gasPrice.Add(gasTipCap, baseFee) + if gasPrice.Cmp(gasFeeCap) > 0 { + gasPrice = gasFeeCap + } } } } diff --git a/tests/state_test_util.go b/tests/state_test_util.go index cf0ce9777f..e9a58694ed 100644 --- a/tests/state_test_util.go +++ b/tests/state_test_util.go @@ -411,8 +411,10 @@ func (tx *stTransaction) toMessage(ps stPostState, baseFee *big.Int) (*core.Mess if tx.MaxPriorityFeePerGas == nil { tx.MaxPriorityFeePerGas = tx.MaxFeePerGas } - gasPrice = math.BigMin(new(big.Int).Add(tx.MaxPriorityFeePerGas, baseFee), - tx.MaxFeePerGas) + gasPrice = new(big.Int).Add(tx.MaxPriorityFeePerGas, baseFee) + if gasPrice.Cmp(tx.MaxFeePerGas) > 0 { + gasPrice = tx.MaxFeePerGas + } } if gasPrice == nil { return nil, errors.New("no gas price provided")