From 40e012f809f4e0ce950c8a5e06a696125499af6d Mon Sep 17 00:00:00 2001 From: syntrust Date: Wed, 15 Jan 2025 14:42:43 +0800 Subject: [PATCH 01/14] add l1fee --- ethstorage/miner/l1_mining_api.go | 102 ++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 20 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 0e5cb625..2f8c8769 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -26,7 +26,10 @@ const ( ) var ( - mineSig = crypto.Keccak256Hash([]byte(`mine(uint256,uint256,address,uint256,bytes32[],uint256[],bytes,bytes[],bytes[])`)) + mineSig = crypto.Keccak256Hash([]byte(`mine(uint256,uint256,address,uint256,bytes32[],uint256[],bytes,bytes[],bytes[])`)) + ether = new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil) + txFeeCapL2 = new(big.Int).Mul(big.NewInt(1000), ether) + gasPriceOracle = common.HexToAddress("0x420000000000000000000000000000000000000F") ) func NewL1MiningAPI(l1 *eth.PollingClient, rc *eth.RandaoClient, lg log.Logger) *l1MiningAPI { @@ -112,12 +115,64 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add } m.lg.Info("Estimated gas done", "gas", estimatedGas) + nonce, err := m.NonceAt(ctx, cfg.SignerAddr, big.NewInt(rpc.LatestBlockNumber.Int64())) + if err != nil { + m.lg.Error("Query nonce failed", "error", err.Error()) + return common.Hash{}, err + } + m.lg.Debug("Query nonce done", "nonce", nonce) + gas := uint64(float64(estimatedGas) * gasBufferRatio) + + // check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail) + txFeeCap := ether + if m.rc != nil { + txFeeCap = txFeeCapL2 + } + estimatedTxFee := new(big.Int).Mul(new(big.Int).SetUint64(gas), gasFeeCap) + if estimatedTxFee.Cmp(txFeeCap) == 1 { + m.lg.Error(fmt.Sprintf("Mining tx dropped: tx fee (%d) exceeds the configured cap (%d)", + estimatedTxFee, txFeeCap)) + return common.Hash{}, errDropped + } + + rawTx := &types.DynamicFeeTx{ + ChainID: m.NetworkID, + Nonce: nonce, + GasTipCap: tip, + GasFeeCap: gasFeeCap, + Gas: gas, + To: &contract, + Value: common.Big0, + Data: calldata, + } + + unsignedTx := types.NewTx(rawTx) + extraCost := new(big.Int) + // Add L1 data fee as tx cost when es-node is deployed as an L3 + if m.rc != nil { + unsignedBin, err := unsignedTx.MarshalBinary() + if err != nil { + //TODO do not abort tx only because estimation failure + m.lg.Error("Failed to marshal unsigned tx", "error", err) + return common.Hash{}, err + } + l1fee, err := m.getL1Fee(ctx, unsignedBin) + if err != nil { + m.lg.Error("Failed to get L1 fee", "error", err) + return common.Hash{}, fmt.Errorf("getL1Fee failed: %w", err) + } + m.lg.Info("Get L1 fee done", "l1Fee", l1fee) + extraCost = l1fee + } + reward, err := m.GetMiningReward(rst.startShardId, rst.blockNumber.Int64()) if err != nil { m.lg.Warn("Query mining reward failed", "error", err.Error()) } if reward != nil { - profitableGasFeeCap := new(big.Int).Div(new(big.Int).Sub(reward, cfg.MinimumProfit), new(big.Int).SetUint64(estimatedGas)) + costCap := new(big.Int).Sub(reward, cfg.MinimumProfit) + costCap = new(big.Int).Add(costCap, extraCost) + profitableGasFeeCap := new(big.Int).Div(costCap, new(big.Int).SetUint64(estimatedGas)) m.lg.Info("Minimum profitable gas fee cap", "gasFeeCap", profitableGasFeeCap) if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) @@ -137,24 +192,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add } sign := cfg.SignerFnFactory(m.NetworkID) - nonce, err := m.NonceAt(ctx, cfg.SignerAddr, big.NewInt(rpc.LatestBlockNumber.Int64())) - if err != nil { - m.lg.Error("Query nonce failed", "error", err.Error()) - return common.Hash{}, err - } - m.lg.Debug("Query nonce done", "nonce", nonce) - gas := uint64(float64(estimatedGas) * gasBufferRatio) - rawTx := &types.DynamicFeeTx{ - ChainID: m.NetworkID, - Nonce: nonce, - GasTipCap: tip, - GasFeeCap: gasFeeCap, - Gas: gas, - To: &contract, - Value: common.Big0, - Data: calldata, - } - signedTx, err := sign(ctx, cfg.SignerAddr, types.NewTx(rawTx)) + signedTx, err := sign(ctx, cfg.SignerAddr, unsignedTx) if err != nil { m.lg.Error("Sign tx error", "error", err) return common.Hash{}, err @@ -169,6 +207,30 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add return signedTx.Hash(), nil } +func (m *l1MiningAPI) getL1Fee(ctx context.Context, _data []byte) (*big.Int, error) { + bytesType, _ := abi.NewType("bytes", "", nil) + uint256Type, _ := abi.NewType("uint256", "", nil) + + dataField, _ := abi.Arguments{{Type: bytesType}}.Pack(_data) + h := crypto.Keccak256Hash([]byte(`getL1Fee(bytes)`)) + calldata := append(h[0:4], dataField...) + msg := ethereum.CallMsg{ + To: &gasPriceOracle, + Data: calldata, + } + bs, err := m.CallContract(ctx, msg, nil) + if err != nil { + return nil, err + } + res, err := abi.Arguments{ + {Type: uint256Type}, + }.UnpackValues(bs) + if err != nil { + return nil, err + } + return res[0].(*big.Int), nil +} + func (m *l1MiningAPI) getRandaoProof(ctx context.Context, blockNumber *big.Int) ([]byte, error) { var caller interface { HeaderByNumber(context.Context, *big.Int) (*types.Header, error) From 049d09874e56efaaa3b81eef324007dc107e2abe Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 16 Jan 2025 10:19:26 +0800 Subject: [PATCH 02/14] fix test --- integration_tests/run_tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration_tests/run_tests.sh b/integration_tests/run_tests.sh index 0b5950fa..fef71557 100755 --- a/integration_tests/run_tests.sh +++ b/integration_tests/run_tests.sh @@ -26,7 +26,7 @@ if [ -z "$ES_NODE_STORAGE_L1CONTRACT_CLEF" ]; then fi # A newly deployed contract is required for each run for miner test, with zkp verifier of mode 2 if [ -z "$ES_NODE_STORAGE_L1CONTRACT" ]; then - export ES_NODE_STORAGE_L1CONTRACT=0xe8F0898cbA701E677970DB33404A817Ff42D4499 + export ES_NODE_STORAGE_L1CONTRACT=0x517ad0ba959f3556930c9Bc483B454584F7e11df fi # A contract with zkp verifier of mode 1 (one proof per sample) if [ -z "$ES_NODE_STORAGE_L1CONTRACT_ZKP1" ]; then @@ -34,7 +34,7 @@ if [ -z "$ES_NODE_STORAGE_L1CONTRACT_ZKP1" ]; then fi # The commonly used l1 eth rpc endpoint if [ -z "$ES_NODE_L1_ETH_RPC" ]; then - export ES_NODE_L1_ETH_RPC="http://65.109.20.29:8545" # L2 + export ES_NODE_L1_ETH_RPC="http://5.9.87.214:8545" # L2 fi # The clef endpoint that the miner will use to sign the transaction if [ -z "$ES_NODE_CLEF_RPC" ]; then From 1a7d0a9a6a4df689d0c2d0450ad4a3421b2e6b22 Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 16 Jan 2025 15:11:30 +0800 Subject: [PATCH 03/14] refactor --- ethstorage/miner/l1_mining_api.go | 66 ++++++++++++++----------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 2f8c8769..f7d08ccd 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -26,10 +26,10 @@ const ( ) var ( - mineSig = crypto.Keccak256Hash([]byte(`mine(uint256,uint256,address,uint256,bytes32[],uint256[],bytes,bytes[],bytes[])`)) - ether = new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil) - txFeeCapL2 = new(big.Int).Mul(big.NewInt(1000), ether) - gasPriceOracle = common.HexToAddress("0x420000000000000000000000000000000000000F") + mineSig = crypto.Keccak256Hash([]byte(`mine(uint256,uint256,address,uint256,bytes32[],uint256[],bytes,bytes[],bytes[])`)) + ether = new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil) + txFeeCapDefaultL2 = new(big.Int).Mul(big.NewInt(1000), ether) + gasPriceOracleL2 = common.HexToAddress("0x420000000000000000000000000000000000000F") ) func NewL1MiningAPI(l1 *eth.PollingClient, rc *eth.RandaoClient, lg log.Logger) *l1MiningAPI { @@ -121,17 +121,17 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add return common.Hash{}, err } m.lg.Debug("Query nonce done", "nonce", nonce) - gas := uint64(float64(estimatedGas) * gasBufferRatio) + safeGas := uint64(float64(estimatedGas) * gasBufferRatio) - // check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail) - txFeeCap := ether + // Check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail) + txFeeCapDefault := ether if m.rc != nil { - txFeeCap = txFeeCapL2 + txFeeCapDefault = txFeeCapDefaultL2 } - estimatedTxFee := new(big.Int).Mul(new(big.Int).SetUint64(gas), gasFeeCap) - if estimatedTxFee.Cmp(txFeeCap) == 1 { - m.lg.Error(fmt.Sprintf("Mining tx dropped: tx fee (%d) exceeds the configured cap (%d)", - estimatedTxFee, txFeeCap)) + txFee := new(big.Int).Mul(new(big.Int).SetUint64(safeGas), gasFeeCap) + m.lg.Debug("Estimated tx fee on the safe side", "txFee", fmtEth(txFee)) + if txFee.Cmp(txFeeCapDefault) == 1 { + m.lg.Error("Mining tx dropped: tx fee exceeds the configured cap", "txFee", fmtEth(txFee), "cap", fmtEth(txFeeCapDefault)) return common.Hash{}, errDropped } @@ -140,7 +140,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add Nonce: nonce, GasTipCap: tip, GasFeeCap: gasFeeCap, - Gas: gas, + Gas: safeGas, To: &contract, Value: common.Big0, Data: calldata, @@ -150,19 +150,13 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add extraCost := new(big.Int) // Add L1 data fee as tx cost when es-node is deployed as an L3 if m.rc != nil { - unsignedBin, err := unsignedTx.MarshalBinary() + l1fee, err := m.getL1Fee(ctx, unsignedTx) if err != nil { - //TODO do not abort tx only because estimation failure - m.lg.Error("Failed to marshal unsigned tx", "error", err) - return common.Hash{}, err + m.lg.Warn("Failed to get L1 fee", "error", err) + } else { + m.lg.Info("Get L1 fee done", "l1Fee", l1fee) + extraCost = l1fee } - l1fee, err := m.getL1Fee(ctx, unsignedBin) - if err != nil { - m.lg.Error("Failed to get L1 fee", "error", err) - return common.Hash{}, fmt.Errorf("getL1Fee failed: %w", err) - } - m.lg.Info("Get L1 fee done", "l1Fee", l1fee) - extraCost = l1fee } reward, err := m.GetMiningReward(rst.startShardId, rst.blockNumber.Int64()) @@ -170,6 +164,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add m.lg.Warn("Query mining reward failed", "error", err.Error()) } if reward != nil { + m.lg.Info("Query mining reward done", "reward", fmtEth(reward)) costCap := new(big.Int).Sub(reward, cfg.MinimumProfit) costCap = new(big.Int).Add(costCap, extraCost) profitableGasFeeCap := new(big.Int).Div(costCap, new(big.Int).SetUint64(estimatedGas)) @@ -207,24 +202,23 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add return signedTx.Hash(), nil } -func (m *l1MiningAPI) getL1Fee(ctx context.Context, _data []byte) (*big.Int, error) { +func (m *l1MiningAPI) getL1Fee(ctx context.Context, unsignedTx *types.Transaction) (*big.Int, error) { + unsignedBin, err := unsignedTx.MarshalBinary() + if err != nil { + return nil, err + } bytesType, _ := abi.NewType("bytes", "", nil) uint256Type, _ := abi.NewType("uint256", "", nil) - - dataField, _ := abi.Arguments{{Type: bytesType}}.Pack(_data) + dataField, _ := abi.Arguments{{Type: bytesType}}.Pack(unsignedBin) h := crypto.Keccak256Hash([]byte(`getL1Fee(bytes)`)) - calldata := append(h[0:4], dataField...) - msg := ethereum.CallMsg{ - To: &gasPriceOracle, - Data: calldata, - } - bs, err := m.CallContract(ctx, msg, nil) + bs, err := m.CallContract(ctx, ethereum.CallMsg{ + To: &gasPriceOracleL2, + Data: append(h[0:4], dataField...), + }, nil) if err != nil { return nil, err } - res, err := abi.Arguments{ - {Type: uint256Type}, - }.UnpackValues(bs) + res, err := abi.Arguments{{Type: uint256Type}}.UnpackValues(bs) if err != nil { return nil, err } From e682b86c13929f5d75cd5abbdbd7ee06df7ca3c5 Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 16 Jan 2025 15:11:41 +0800 Subject: [PATCH 04/14] add todo --- ethstorage/miner/worker.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ethstorage/miner/worker.go b/ethstorage/miner/worker.go index 4d880a41..f15e1b6a 100644 --- a/ethstorage/miner/worker.go +++ b/ethstorage/miner/worker.go @@ -504,6 +504,7 @@ func (w *worker) checkTxStatus(txHash common.Hash, miner common.Address) { } } if reward != nil { + // TODO: the cost should include receipt.L1Fee for op-geth log.Info("Mining transaction accounting (in ether)", "reward", fmtEth(reward), "cost", fmtEth(cost), From 692521166cb643a25c401d3bd25e5f087e02427e Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 16 Jan 2025 15:40:09 +0800 Subject: [PATCH 05/14] minor --- ethstorage/miner/l1_mining_api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index f7d08ccd..3bcb8dfe 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -129,7 +129,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add txFeeCapDefault = txFeeCapDefaultL2 } txFee := new(big.Int).Mul(new(big.Int).SetUint64(safeGas), gasFeeCap) - m.lg.Debug("Estimated tx fee on the safe side", "txFee", fmtEth(txFee)) + m.lg.Debug("Estimated tx fee on the safe side", "safeGas", safeGas, "txFee", txFee) if txFee.Cmp(txFeeCapDefault) == 1 { m.lg.Error("Mining tx dropped: tx fee exceeds the configured cap", "txFee", fmtEth(txFee), "cap", fmtEth(txFeeCapDefault)) return common.Hash{}, errDropped @@ -164,7 +164,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add m.lg.Warn("Query mining reward failed", "error", err.Error()) } if reward != nil { - m.lg.Info("Query mining reward done", "reward", fmtEth(reward)) + m.lg.Info("Query mining reward done", "reward", reward) costCap := new(big.Int).Sub(reward, cfg.MinimumProfit) costCap = new(big.Int).Add(costCap, extraCost) profitableGasFeeCap := new(big.Int).Div(costCap, new(big.Int).SetUint64(estimatedGas)) From 446d27b6715694b47e94bd2fd17dbc2f51c9a2f0 Mon Sep 17 00:00:00 2001 From: syntrust Date: Mon, 20 Jan 2025 17:19:14 +0800 Subject: [PATCH 06/14] fix tx cost --- ethstorage/miner/l1_mining_api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 3bcb8dfe..00077a39 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -166,8 +166,8 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add if reward != nil { m.lg.Info("Query mining reward done", "reward", reward) costCap := new(big.Int).Sub(reward, cfg.MinimumProfit) - costCap = new(big.Int).Add(costCap, extraCost) - profitableGasFeeCap := new(big.Int).Div(costCap, new(big.Int).SetUint64(estimatedGas)) + txCost := new(big.Int).Sub(costCap, extraCost) + profitableGasFeeCap := new(big.Int).Div(txCost, new(big.Int).SetUint64(estimatedGas)) m.lg.Info("Minimum profitable gas fee cap", "gasFeeCap", profitableGasFeeCap) if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) From af863f3b60908a5bfc2e63a11a8b247a56e21e1c Mon Sep 17 00:00:00 2001 From: syntrust Date: Mon, 20 Jan 2025 18:30:45 +0800 Subject: [PATCH 07/14] refactor --- ethstorage/miner/l1_mining_api.go | 86 +++++++++++++++++++------------ 1 file changed, 54 insertions(+), 32 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 00077a39..2d9432ff 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -123,19 +123,11 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add m.lg.Debug("Query nonce done", "nonce", nonce) safeGas := uint64(float64(estimatedGas) * gasBufferRatio) - // Check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail) - txFeeCapDefault := ether - if m.rc != nil { - txFeeCapDefault = txFeeCapDefaultL2 - } - txFee := new(big.Int).Mul(new(big.Int).SetUint64(safeGas), gasFeeCap) - m.lg.Debug("Estimated tx fee on the safe side", "safeGas", safeGas, "txFee", txFee) - if txFee.Cmp(txFeeCapDefault) == 1 { - m.lg.Error("Mining tx dropped: tx fee exceeds the configured cap", "txFee", fmtEth(txFee), "cap", fmtEth(txFeeCapDefault)) - return common.Hash{}, errDropped + if err := m.txFeeCapCheck(safeGas, gasFeeCap); err != nil { + return common.Hash{}, err } - rawTx := &types.DynamicFeeTx{ + unsignedTx := types.NewTx(&types.DynamicFeeTx{ ChainID: m.NetworkID, Nonce: nonce, GasTipCap: tip, @@ -144,9 +136,52 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add To: &contract, Value: common.Big0, Data: calldata, + }) + gasFeeCapChecked, err := m.profitCheck(ctx, unsignedTx, rst, cfg, gasFeeCap, tip, estimatedGas, useConfig) + if err != nil { + return common.Hash{}, err } - unsignedTx := types.NewTx(rawTx) + sign := cfg.SignerFnFactory(m.NetworkID) + signedTx, err := sign(ctx, cfg.SignerAddr, unsignedTx) + if err != nil { + m.lg.Error("Sign tx error", "error", err) + return common.Hash{}, err + } + err = m.SendTransaction(ctx, signedTx) + if err != nil { + m.lg.Error("Send tx failed", "txNonce", nonce, "gasFeeCap", gasFeeCapChecked, "error", err) + return common.Hash{}, err + } + m.lg.Info("Submit mined result done", "shard", rst.startShardId, "block", rst.blockNumber, + "nonce", rst.nonce, "txSigner", cfg.SignerAddr.Hex(), "hash", signedTx.Hash().Hex()) + return signedTx.Hash(), nil +} + +// Check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail) +func (m *l1MiningAPI) txFeeCapCheck(safeGas uint64, gasFeeCap *big.Int) error { + txFeeCapDefault := ether + if m.rc != nil { + txFeeCapDefault = txFeeCapDefaultL2 + } + txFee := new(big.Int).Mul(new(big.Int).SetUint64(safeGas), gasFeeCap) + m.lg.Debug("Estimated tx fee on the safe side", "safeGas", safeGas, "txFee", txFee) + if txFee.Cmp(txFeeCapDefault) == 1 { + m.lg.Error("Mining tx dropped: tx fee exceeds the configured cap", "txFee", fmtEth(txFee), "cap", fmtEth(txFeeCapDefault)) + return errDropped + } + return nil +} + +func (m *l1MiningAPI) profitCheck( + ctx context.Context, + unsignedTx *types.Transaction, + rst result, + cfg Config, + gasFeeCap, tip *big.Int, + estimatedGas uint64, + useConfig bool, +) (*big.Int, error) { extraCost := new(big.Int) // Add L1 data fee as tx cost when es-node is deployed as an L3 if m.rc != nil { @@ -158,11 +193,12 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add extraCost = l1fee } } - reward, err := m.GetMiningReward(rst.startShardId, rst.blockNumber.Int64()) if err != nil { m.lg.Warn("Query mining reward failed", "error", err.Error()) + return gasFeeCap, nil } + gasFeeCapChecked := gasFeeCap if reward != nil { m.lg.Info("Query mining reward done", "reward", reward) costCap := new(big.Int).Sub(reward, cfg.MinimumProfit) @@ -172,34 +208,20 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) m.lg.Warn("Mining tx dropped: the profit will not meet expectation", "estimatedProfit", fmtEth(profit), "minimumProfit", fmtEth(cfg.MinimumProfit)) - return common.Hash{}, errDropped + return nil, errDropped } if !useConfig { - gasFeeCap = profitableGasFeeCap + gasFeeCapChecked = profitableGasFeeCap m.lg.Info("Using profitable gas fee cap", "gasFeeCap", gasFeeCap) } } else { if !useConfig { // (tip + 2*baseFee) to ensure the tx to be marketable for six consecutive 100% full blocks. - gasFeeCap = new(big.Int).Add(new(big.Int).Mul(new(big.Int).Sub(gasFeeCap, tip), big.NewInt(2)), tip) - m.lg.Info("Using marketable gas fee cap", "gasFeeCap", gasFeeCap) + gasFeeCapChecked = new(big.Int).Add(new(big.Int).Mul(new(big.Int).Sub(gasFeeCap, tip), big.NewInt(2)), tip) + m.lg.Info("Using marketable gas fee cap", "gasFeeCap", gasFeeCapChecked) } } - - sign := cfg.SignerFnFactory(m.NetworkID) - signedTx, err := sign(ctx, cfg.SignerAddr, unsignedTx) - if err != nil { - m.lg.Error("Sign tx error", "error", err) - return common.Hash{}, err - } - err = m.SendTransaction(ctx, signedTx) - if err != nil { - m.lg.Error("Send tx failed", "txNonce", nonce, "gasFeeCap", gasFeeCap, "error", err) - return common.Hash{}, err - } - m.lg.Info("Submit mined result done", "shard", rst.startShardId, "block", rst.blockNumber, - "nonce", rst.nonce, "txSigner", cfg.SignerAddr.Hex(), "hash", signedTx.Hash().Hex()) - return signedTx.Hash(), nil + return gasFeeCapChecked, nil } func (m *l1MiningAPI) getL1Fee(ctx context.Context, unsignedTx *types.Transaction) (*big.Int, error) { From b2bf0dd32312e3f8e7987163d8ba03655f80ac9f Mon Sep 17 00:00:00 2001 From: syntrust Date: Mon, 20 Jan 2025 19:09:21 +0800 Subject: [PATCH 08/14] refactor and fix --- ethstorage/miner/l1_mining_api.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 2d9432ff..e2f48f6a 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -137,7 +137,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add Value: common.Big0, Data: calldata, }) - gasFeeCapChecked, err := m.profitCheck(ctx, unsignedTx, rst, cfg, gasFeeCap, tip, estimatedGas, useConfig) + gasFeeCapChecked, err := m.profitCheck(ctx, unsignedTx, rst, cfg.MinimumProfit, gasFeeCap, tip, estimatedGas, useConfig) if err != nil { return common.Hash{}, err } @@ -177,8 +177,7 @@ func (m *l1MiningAPI) profitCheck( ctx context.Context, unsignedTx *types.Transaction, rst result, - cfg Config, - gasFeeCap, tip *big.Int, + minProfit, gasFeeCap, tip *big.Int, estimatedGas uint64, useConfig bool, ) (*big.Int, error) { @@ -201,13 +200,14 @@ func (m *l1MiningAPI) profitCheck( gasFeeCapChecked := gasFeeCap if reward != nil { m.lg.Info("Query mining reward done", "reward", reward) - costCap := new(big.Int).Sub(reward, cfg.MinimumProfit) + costCap := new(big.Int).Sub(reward, minProfit) txCost := new(big.Int).Sub(costCap, extraCost) profitableGasFeeCap := new(big.Int).Div(txCost, new(big.Int).SetUint64(estimatedGas)) m.lg.Info("Minimum profitable gas fee cap", "gasFeeCap", profitableGasFeeCap) if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) - m.lg.Warn("Mining tx dropped: the profit will not meet expectation", "estimatedProfit", fmtEth(profit), "minimumProfit", fmtEth(cfg.MinimumProfit)) + profit = new(big.Int).Sub(profit, extraCost) + m.lg.Warn("Mining tx dropped: the profit will not meet expectation", "estimatedProfit", fmtEth(profit), "minimumProfit", fmtEth(minProfit)) return nil, errDropped } if !useConfig { From eceb4dbbdb3c6032e0ea1086a33a64056fcdc75b Mon Sep 17 00:00:00 2001 From: syntrust Date: Wed, 22 Jan 2025 18:56:16 +0800 Subject: [PATCH 09/14] refactor and ut --- ethstorage/miner/l1_mining_api.go | 114 +++++++++++++------------ ethstorage/miner/l1_mining_api_test.go | 63 ++++++++++++++ go.mod | 5 +- go.sum | 5 ++ 4 files changed, 132 insertions(+), 55 deletions(-) create mode 100644 ethstorage/miner/l1_mining_api_test.go diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index e2f48f6a..232da360 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -137,7 +137,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add Value: common.Big0, Data: calldata, }) - gasFeeCapChecked, err := m.profitCheck(ctx, unsignedTx, rst, cfg.MinimumProfit, gasFeeCap, tip, estimatedGas, useConfig) + gasFeeCapChecked, err := checkProfit(ctx, m, unsignedTx, rst, cfg.MinimumProfit, gasFeeCap, tip, estimatedGas, m.rc != nil, useConfig, m.lg) if err != nil { return common.Hash{}, err } @@ -173,58 +173,7 @@ func (m *l1MiningAPI) txFeeCapCheck(safeGas uint64, gasFeeCap *big.Int) error { return nil } -func (m *l1MiningAPI) profitCheck( - ctx context.Context, - unsignedTx *types.Transaction, - rst result, - minProfit, gasFeeCap, tip *big.Int, - estimatedGas uint64, - useConfig bool, -) (*big.Int, error) { - extraCost := new(big.Int) - // Add L1 data fee as tx cost when es-node is deployed as an L3 - if m.rc != nil { - l1fee, err := m.getL1Fee(ctx, unsignedTx) - if err != nil { - m.lg.Warn("Failed to get L1 fee", "error", err) - } else { - m.lg.Info("Get L1 fee done", "l1Fee", l1fee) - extraCost = l1fee - } - } - reward, err := m.GetMiningReward(rst.startShardId, rst.blockNumber.Int64()) - if err != nil { - m.lg.Warn("Query mining reward failed", "error", err.Error()) - return gasFeeCap, nil - } - gasFeeCapChecked := gasFeeCap - if reward != nil { - m.lg.Info("Query mining reward done", "reward", reward) - costCap := new(big.Int).Sub(reward, minProfit) - txCost := new(big.Int).Sub(costCap, extraCost) - profitableGasFeeCap := new(big.Int).Div(txCost, new(big.Int).SetUint64(estimatedGas)) - m.lg.Info("Minimum profitable gas fee cap", "gasFeeCap", profitableGasFeeCap) - if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { - profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) - profit = new(big.Int).Sub(profit, extraCost) - m.lg.Warn("Mining tx dropped: the profit will not meet expectation", "estimatedProfit", fmtEth(profit), "minimumProfit", fmtEth(minProfit)) - return nil, errDropped - } - if !useConfig { - gasFeeCapChecked = profitableGasFeeCap - m.lg.Info("Using profitable gas fee cap", "gasFeeCap", gasFeeCap) - } - } else { - if !useConfig { - // (tip + 2*baseFee) to ensure the tx to be marketable for six consecutive 100% full blocks. - gasFeeCapChecked = new(big.Int).Add(new(big.Int).Mul(new(big.Int).Sub(gasFeeCap, tip), big.NewInt(2)), tip) - m.lg.Info("Using marketable gas fee cap", "gasFeeCap", gasFeeCapChecked) - } - } - return gasFeeCapChecked, nil -} - -func (m *l1MiningAPI) getL1Fee(ctx context.Context, unsignedTx *types.Transaction) (*big.Int, error) { +func (m *l1MiningAPI) GetL1Fee(ctx context.Context, unsignedTx *types.Transaction) (*big.Int, error) { unsignedBin, err := unsignedTx.MarshalBinary() if err != nil { return nil, err @@ -334,3 +283,62 @@ func (m *l1MiningAPI) suggestGasPrices(ctx context.Context, cfg Config) (*big.In } return tip, gasFeeCap, useConfig, nil } + +type ProfitChecker interface { + GetL1Fee(ctx context.Context, tx *types.Transaction) (*big.Int, error) + GetMiningReward(shardID uint64, blockNumber int64) (*big.Int, error) +} + +// Adjust the gas price based on the estimated rewards and costs. +func checkProfit( + ctx context.Context, + checker ProfitChecker, + unsignedTx *types.Transaction, + rst result, + minProfit, gasFeeCap, tip *big.Int, + estimatedGas uint64, + useL2, useConfig bool, + lg log.Logger, +) (*big.Int, error) { + extraCost := new(big.Int) + // Add L1 data fee as tx cost when es-node is deployed as an L3 + if useL2 { + l1fee, err := checker.GetL1Fee(ctx, unsignedTx) + if err != nil { + lg.Warn("Failed to get L1 fee", "error", err) + } else { + lg.Info("Get L1 fee done", "l1Fee", l1fee) + extraCost = l1fee + } + } + reward, err := checker.GetMiningReward(rst.startShardId, rst.blockNumber.Int64()) + if err != nil { + lg.Warn("Query mining reward failed", "error", err.Error()) + return gasFeeCap, nil + } + gasFeeCapChecked := gasFeeCap + if reward != nil { + lg.Info("Query mining reward done", "reward", reward) + costCap := new(big.Int).Sub(reward, minProfit) + txCost := new(big.Int).Sub(costCap, extraCost) + profitableGasFeeCap := new(big.Int).Div(txCost, new(big.Int).SetUint64(estimatedGas)) + lg.Info("Minimum profitable gas fee cap", "gasFeeCap", profitableGasFeeCap) + if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { + profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) + profit = new(big.Int).Sub(profit, extraCost) + lg.Warn("Mining tx dropped: the profit will not meet expectation", "estimatedProfit", fmtEth(profit), "minimumProfit", fmtEth(minProfit)) + return nil, errDropped + } + if !useConfig { + gasFeeCapChecked = profitableGasFeeCap + lg.Info("Using profitable gas fee cap", "gasFeeCap", gasFeeCapChecked) + } + } else { + if !useConfig { + // (tip + 2*baseFee) to ensure the tx to be marketable for six consecutive 100% full blocks. + gasFeeCapChecked = new(big.Int).Add(new(big.Int).Mul(new(big.Int).Sub(gasFeeCap, tip), big.NewInt(2)), tip) + lg.Info("Using marketable gas fee cap", "gasFeeCap", gasFeeCapChecked) + } + } + return gasFeeCapChecked, nil +} diff --git a/ethstorage/miner/l1_mining_api_test.go b/ethstorage/miner/l1_mining_api_test.go new file mode 100644 index 00000000..6b97e9f9 --- /dev/null +++ b/ethstorage/miner/l1_mining_api_test.go @@ -0,0 +1,63 @@ +// Copyright 2022-2023, EthStorage. +// For license information, see https://github.com/ethstorage/es-node/blob/main/LICENSE +package miner + +import ( + "context" + "math/big" + "os" + "testing" + + "github.com/ethereum/go-ethereum/core/types" + esLog "github.com/ethstorage/go-ethstorage/ethstorage/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "golang.org/x/term" +) + +var gwei = new(big.Int).Exp(big.NewInt(10), big.NewInt(9), nil) + +func Test_l1MiningAPI_checkProfit(t *testing.T) { + ctx := context.Background() + unsignedTx := &types.Transaction{} + rst := result{startShardId: 0, blockNumber: big.NewInt(100)} + minProfit := new(big.Int) + tip := gwei + gasFeeCap := new(big.Int).Mul(big.NewInt(5), gwei) + l1Fee := new(big.Int).Mul(big.NewInt(25), gwei) + // rewardL1 := new(big.Int).Mul(big.NewInt(5000000), gwei) // 0.05 eth + rewardL2 := new(big.Int).Mul(big.NewInt(25), ether) // 25 qkc + estimatedGas := uint64(500000) + useL2 := true + useConfig := false + lgr := esLog.NewLogger(esLog.CLIConfig{ + Level: "debug", + Format: "text", + Color: term.IsTerminal(int(os.Stdout.Fd())), + }) + mockAPI := &MockMiningAPI{l1MiningAPI: l1MiningAPI{lg: lgr}} + + // Test Case 1: L2 | Using a flexible gas price until only minimal profit remains + targetGasFeeCap := new(big.Int).SetInt64(49999999950000) + mockAPI.On("GetL1Fee", ctx, unsignedTx).Return(l1Fee, nil) + mockAPI.On("GetMiningReward", rst.startShardId, rst.blockNumber.Int64()).Return(rewardL2, nil) + resultGasFeeCap, err := checkProfit(ctx, mockAPI, unsignedTx, rst, minProfit, gasFeeCap, tip, estimatedGas, useL2, useConfig, mockAPI.lg) + assert.NoError(t, err) + assert.Equal(t, targetGasFeeCap, resultGasFeeCap) + +} + +type MockMiningAPI struct { + l1MiningAPI + mock.Mock +} + +func (m *MockMiningAPI) GetL1Fee(ctx context.Context, tx *types.Transaction) (*big.Int, error) { + args := m.Called(ctx, tx) + return args.Get(0).(*big.Int), args.Error(1) +} + +func (m *MockMiningAPI) GetMiningReward(shardId uint64, blockNumber int64) (*big.Int, error) { + args := m.Called(shardId, blockNumber) + return args.Get(0).(*big.Int), args.Error(1) +} diff --git a/go.mod b/go.mod index ef219c52..5e38a3ec 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/protolambda/go-kzg v0.0.0-20221224134646-c91cee5e954e github.com/spf13/cobra v1.5.0 github.com/status-im/keycard-go v0.2.0 + github.com/stretchr/testify v1.8.4 github.com/urfave/cli v1.22.9 golang.org/x/crypto v0.14.0 golang.org/x/sys v0.13.0 @@ -66,7 +67,7 @@ require ( github.com/rivo/uniseg v0.4.3 // indirect github.com/rs/cors v1.9.0 // indirect github.com/shirou/gopsutil v3.21.11+incompatible // indirect - github.com/stretchr/testify v1.8.4 // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/supranational/blst v0.3.11 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a // indirect github.com/tetratelabs/wazero v1.8.0 // indirect @@ -154,7 +155,7 @@ require ( github.com/onsi/ginkgo/v2 v2.13.0 // indirect github.com/opencontainers/runtime-spec v1.1.0 // indirect github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 // indirect - github.com/pkg/errors v0.9.1 // indirect + github.com/pkg/errors v0.9.1 github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.17.0 github.com/prometheus/client_model v0.4.1-0.20230718164431-9a2bf3000d16 // indirect diff --git a/go.sum b/go.sum index 25616595..78c6910b 100644 --- a/go.sum +++ b/go.sum @@ -634,13 +634,18 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM github.com/status-im/keycard-go v0.2.0 h1:QDLFswOQu1r5jsycloeQh3bVU8n/NatHHaZobtDnDzA= github.com/status-im/keycard-go v0.2.0/go.mod h1:wlp8ZLbsmrF6g6WjugPAx+IzoLrkdf9+mHxBEeo3Hbg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/supranational/blst v0.3.11 h1:LyU6FolezeWAhvQk0k6O/d49jqgO52MSDDfYgbeoEm4= From 60f8d26214afacbeb76ad9b5ee7914b23229c384 Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 23 Jan 2025 13:12:51 +0800 Subject: [PATCH 10/14] should check tx fee cap after profit check --- ethstorage/miner/l1_mining_api.go | 44 +++++++++++++------------------ 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 232da360..8300c291 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -123,10 +123,6 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add m.lg.Debug("Query nonce done", "nonce", nonce) safeGas := uint64(float64(estimatedGas) * gasBufferRatio) - if err := m.txFeeCapCheck(safeGas, gasFeeCap); err != nil { - return common.Hash{}, err - } - unsignedTx := types.NewTx(&types.DynamicFeeTx{ ChainID: m.NetworkID, Nonce: nonce, @@ -137,7 +133,7 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add Value: common.Big0, Data: calldata, }) - gasFeeCapChecked, err := checkProfit(ctx, m, unsignedTx, rst, cfg.MinimumProfit, gasFeeCap, tip, estimatedGas, m.rc != nil, useConfig, m.lg) + gasFeeCapChecked, err := checkGasPrice(ctx, m, unsignedTx, rst, cfg.MinimumProfit, gasFeeCap, tip, estimatedGas, safeGas, m.rc != nil, useConfig, m.lg) if err != nil { return common.Hash{}, err } @@ -158,21 +154,6 @@ func (m *l1MiningAPI) SubmitMinedResult(ctx context.Context, contract common.Add return signedTx.Hash(), nil } -// Check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail) -func (m *l1MiningAPI) txFeeCapCheck(safeGas uint64, gasFeeCap *big.Int) error { - txFeeCapDefault := ether - if m.rc != nil { - txFeeCapDefault = txFeeCapDefaultL2 - } - txFee := new(big.Int).Mul(new(big.Int).SetUint64(safeGas), gasFeeCap) - m.lg.Debug("Estimated tx fee on the safe side", "safeGas", safeGas, "txFee", txFee) - if txFee.Cmp(txFeeCapDefault) == 1 { - m.lg.Error("Mining tx dropped: tx fee exceeds the configured cap", "txFee", fmtEth(txFee), "cap", fmtEth(txFeeCapDefault)) - return errDropped - } - return nil -} - func (m *l1MiningAPI) GetL1Fee(ctx context.Context, unsignedTx *types.Transaction) (*big.Int, error) { unsignedBin, err := unsignedTx.MarshalBinary() if err != nil { @@ -284,19 +265,19 @@ func (m *l1MiningAPI) suggestGasPrices(ctx context.Context, cfg Config) (*big.In return tip, gasFeeCap, useConfig, nil } -type ProfitChecker interface { +type GasPriceChecker interface { GetL1Fee(ctx context.Context, tx *types.Transaction) (*big.Int, error) GetMiningReward(shardID uint64, blockNumber int64) (*big.Int, error) } -// Adjust the gas price based on the estimated rewards and costs. -func checkProfit( +// Adjust the gas price based on the estimated rewards, costs, and default tx fee cap. +func checkGasPrice( ctx context.Context, - checker ProfitChecker, + checker GasPriceChecker, unsignedTx *types.Transaction, rst result, minProfit, gasFeeCap, tip *big.Int, - estimatedGas uint64, + estimatedGas, safeGas uint64, useL2, useConfig bool, lg log.Logger, ) (*big.Int, error) { @@ -314,7 +295,6 @@ func checkProfit( reward, err := checker.GetMiningReward(rst.startShardId, rst.blockNumber.Int64()) if err != nil { lg.Warn("Query mining reward failed", "error", err.Error()) - return gasFeeCap, nil } gasFeeCapChecked := gasFeeCap if reward != nil { @@ -340,5 +320,17 @@ func checkProfit( lg.Info("Using marketable gas fee cap", "gasFeeCap", gasFeeCapChecked) } } + + // Check gas fee against tx fee cap (e.g., --rpc.txfeecap as in geth) early to avoid tx fail + txFeeCapDefault := ether + if useL2 { + txFeeCapDefault = txFeeCapDefaultL2 + } + txFee := new(big.Int).Mul(new(big.Int).SetUint64(safeGas), gasFeeCapChecked) + lg.Debug("Estimated tx fee on the safe side", "safeGas", safeGas, "txFee", txFee) + if txFee.Cmp(txFeeCapDefault) == 1 { + gasFeeCapChecked = new(big.Int).Div(txFeeCapDefault, new(big.Int).SetUint64(safeGas)) + lg.Warn("Tx fee exceeds the configured cap, lower the gasFeeCap", "txFee", fmtEth(txFee), "gasFeeCapUpdated", fmtEth(gasFeeCapChecked)) + } return gasFeeCapChecked, nil } From 10a6620edc9e118295ce334174463e271c83a768 Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 23 Jan 2025 13:13:46 +0800 Subject: [PATCH 11/14] refactor tests --- ethstorage/miner/l1_mining_api_test.go | 87 ++++++++++++++++++++------ 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/ethstorage/miner/l1_mining_api_test.go b/ethstorage/miner/l1_mining_api_test.go index 6b97e9f9..8298a5f0 100644 --- a/ethstorage/miner/l1_mining_api_test.go +++ b/ethstorage/miner/l1_mining_api_test.go @@ -18,33 +18,82 @@ import ( var gwei = new(big.Int).Exp(big.NewInt(10), big.NewInt(9), nil) func Test_l1MiningAPI_checkProfit(t *testing.T) { - ctx := context.Background() unsignedTx := &types.Transaction{} - rst := result{startShardId: 0, blockNumber: big.NewInt(100)} - minProfit := new(big.Int) - tip := gwei - gasFeeCap := new(big.Int).Mul(big.NewInt(5), gwei) - l1Fee := new(big.Int).Mul(big.NewInt(25), gwei) - // rewardL1 := new(big.Int).Mul(big.NewInt(5000000), gwei) // 0.05 eth - rewardL2 := new(big.Int).Mul(big.NewInt(25), ether) // 25 qkc - estimatedGas := uint64(500000) - useL2 := true - useConfig := false + mockResult := result{ + startShardId: 0, + blockNumber: big.NewInt(100), + } lgr := esLog.NewLogger(esLog.CLIConfig{ Level: "debug", Format: "text", Color: term.IsTerminal(int(os.Stdout.Fd())), }) - mockAPI := &MockMiningAPI{l1MiningAPI: l1MiningAPI{lg: lgr}} - // Test Case 1: L2 | Using a flexible gas price until only minimal profit remains - targetGasFeeCap := new(big.Int).SetInt64(49999999950000) - mockAPI.On("GetL1Fee", ctx, unsignedTx).Return(l1Fee, nil) - mockAPI.On("GetMiningReward", rst.startShardId, rst.blockNumber.Int64()).Return(rewardL2, nil) - resultGasFeeCap, err := checkProfit(ctx, mockAPI, unsignedTx, rst, minProfit, gasFeeCap, tip, estimatedGas, useL2, useConfig, mockAPI.lg) - assert.NoError(t, err) - assert.Equal(t, targetGasFeeCap, resultGasFeeCap) + testCases := []struct { + name string + l1Fee *big.Int + l1FeeErr error + reward *big.Int + rewardErr error + minProfit *big.Int + gasFeeCap *big.Int + tip *big.Int + estimatedGas uint64 + safeGas uint64 + useL2 bool + useConfig bool + wantError bool + wantGasFeeCap *big.Int + }{ + { + name: "Case1 - SWC Beta: relax the gas price until minimal profit remains", + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + estimatedGas: uint64(500000), + safeGas: uint64(600000), + useL2: true, + useConfig: false, + wantError: false, + wantGasFeeCap: new(big.Int).SetInt64(49999999950000), + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + mockAPI := &MockMiningAPI{l1MiningAPI: l1MiningAPI{lg: lgr}} + mockAPI.On("GetL1Fee", ctx, unsignedTx).Return(tc.l1Fee, tc.l1FeeErr) + mockAPI.On("GetMiningReward", mockResult.startShardId, mockResult.blockNumber.Int64()).Return(tc.reward, tc.rewardErr) + + gotGasFeeCap, gotErr := checkGasPrice( + ctx, + mockAPI, + unsignedTx, + mockResult, + tc.minProfit, + tc.gasFeeCap, + tc.tip, + tc.estimatedGas, + tc.safeGas, + tc.useL2, + tc.useConfig, + lgr, + ) + + if tc.wantError { + assert.Error(t, gotErr, "expected an error but did not get one") + assert.Nil(t, gotGasFeeCap, "gasFeeCap should be nil if error occurs") + } else { + assert.NoError(t, gotErr, "did not expect an error, but got one") + assert.Equal(t, tc.wantGasFeeCap, gotGasFeeCap, "unexpected final gasFeeCap") + } + }) + } } type MockMiningAPI struct { From 91d0623ba696df8fcd2903aadd4094e025f70891 Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 23 Jan 2025 13:18:22 +0800 Subject: [PATCH 12/14] rename --- ethstorage/miner/l1_mining_api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethstorage/miner/l1_mining_api_test.go b/ethstorage/miner/l1_mining_api_test.go index 8298a5f0..2b1fbf41 100644 --- a/ethstorage/miner/l1_mining_api_test.go +++ b/ethstorage/miner/l1_mining_api_test.go @@ -17,7 +17,7 @@ import ( var gwei = new(big.Int).Exp(big.NewInt(10), big.NewInt(9), nil) -func Test_l1MiningAPI_checkProfit(t *testing.T) { +func Test_l1MiningAPI_checkGasPrice(t *testing.T) { unsignedTx := &types.Transaction{} mockResult := result{ startShardId: 0, From 52d74384b9ad2d21fa71dcdd2efe70c5f15091e3 Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 23 Jan 2025 18:02:29 +0800 Subject: [PATCH 13/14] tests --- ethstorage/miner/l1_mining_api.go | 7 +- ethstorage/miner/l1_mining_api_test.go | 197 +++++++++++++++++++++---- 2 files changed, 175 insertions(+), 29 deletions(-) diff --git a/ethstorage/miner/l1_mining_api.go b/ethstorage/miner/l1_mining_api.go index 8300c291..292b28ba 100644 --- a/ethstorage/miner/l1_mining_api.go +++ b/ethstorage/miner/l1_mining_api.go @@ -300,8 +300,9 @@ func checkGasPrice( if reward != nil { lg.Info("Query mining reward done", "reward", reward) costCap := new(big.Int).Sub(reward, minProfit) - txCost := new(big.Int).Sub(costCap, extraCost) - profitableGasFeeCap := new(big.Int).Div(txCost, new(big.Int).SetUint64(estimatedGas)) + txCostCap := new(big.Int).Sub(costCap, extraCost) + lg.Debug("Tx cost cap", "txCostCap", txCostCap) + profitableGasFeeCap := new(big.Int).Div(txCostCap, new(big.Int).SetUint64(estimatedGas)) lg.Info("Minimum profitable gas fee cap", "gasFeeCap", profitableGasFeeCap) if gasFeeCap.Cmp(profitableGasFeeCap) == 1 { profit := new(big.Int).Sub(reward, new(big.Int).Mul(new(big.Int).SetUint64(estimatedGas), gasFeeCap)) @@ -330,7 +331,7 @@ func checkGasPrice( lg.Debug("Estimated tx fee on the safe side", "safeGas", safeGas, "txFee", txFee) if txFee.Cmp(txFeeCapDefault) == 1 { gasFeeCapChecked = new(big.Int).Div(txFeeCapDefault, new(big.Int).SetUint64(safeGas)) - lg.Warn("Tx fee exceeds the configured cap, lower the gasFeeCap", "txFee", fmtEth(txFee), "gasFeeCapUpdated", fmtEth(gasFeeCapChecked)) + lg.Warn("Tx fee exceeds the configured cap, lower the gasFeeCap", "txFee", fmtEth(txFee), "gasFeeCapUpdated", gasFeeCapChecked) } return gasFeeCapChecked, nil } diff --git a/ethstorage/miner/l1_mining_api_test.go b/ethstorage/miner/l1_mining_api_test.go index 2b1fbf41..e354a930 100644 --- a/ethstorage/miner/l1_mining_api_test.go +++ b/ethstorage/miner/l1_mining_api_test.go @@ -4,6 +4,7 @@ package miner import ( "context" + "fmt" "math/big" "os" "testing" @@ -29,37 +30,183 @@ func Test_l1MiningAPI_checkGasPrice(t *testing.T) { Color: term.IsTerminal(int(os.Stdout.Fd())), }) + estimatedGas := uint64(500000) + safeGas := uint64(600000) + testCases := []struct { - name string - l1Fee *big.Int - l1FeeErr error - reward *big.Int - rewardErr error - minProfit *big.Int - gasFeeCap *big.Int - tip *big.Int - estimatedGas uint64 - safeGas uint64 - useL2 bool - useConfig bool - wantError bool + name string + + // params + minProfit *big.Int + gasFeeCap *big.Int + tip *big.Int + useL2 bool + useConfig bool + + // mocked results + l1Fee *big.Int + l1FeeErr error + reward *big.Int + rewardErr error + + // test results + wantDropped bool wantGasFeeCap *big.Int }{ { - name: "Case1 - SWC Beta: relax the gas price until minimal profit remains", + name: "SWC Beta: relax the gas price until minimal profit remains", + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), l1FeeErr: nil, reward: new(big.Int).Mul(big.NewInt(25), ether), rewardErr: nil, + wantDropped: false, + wantGasFeeCap: new(big.Int).SetInt64(49999999950000), + }, + { + name: "SWC Beta: relax the gas price until minimal profit remains, but cut by tx fee cap", minProfit: big.NewInt(0), gasFeeCap: big.NewInt(1000251), tip: big.NewInt(1000000), - estimatedGas: uint64(500000), - safeGas: uint64(600000), useL2: true, useConfig: false, - wantError: false, - wantGasFeeCap: new(big.Int).SetInt64(49999999950000), + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(1000), ether), + rewardErr: nil, + wantDropped: false, + wantGasFeeCap: new(big.Int).SetInt64(1666666666666666), + }, + { + name: "SWC Beta: tx dropped due to low reward", + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(250), gwei), + rewardErr: nil, + wantDropped: true, + wantGasFeeCap: nil, + }, + { + name: "SWC Beta: tx dropped due to high minimum profit expected", + minProfit: new(big.Int).Mul(big.NewInt(25), ether), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + wantDropped: true, + wantGasFeeCap: nil, + }, + { + name: "SWC Beta: tx dropped due to high gas price", + minProfit: big.NewInt(0), + gasFeeCap: new(big.Int).Mul(big.NewInt(250000), gwei), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + wantDropped: true, + wantGasFeeCap: nil, + }, + { + name: "SWC Beta: tx dropped due to high l1 data fee", + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, + l1Fee: new(big.Int).Mul(big.NewInt(25), ether), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + wantDropped: true, + wantGasFeeCap: nil, + }, + { + name: "SWC Beta: failed to get l1 data fee", + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, + l1Fee: nil, + l1FeeErr: fmt.Errorf("l1fee error"), + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + wantDropped: false, + wantGasFeeCap: big.NewInt(50000000000000), + }, + + { + name: "SWC Beta: unable to get reward; use marketable gas price", + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(1000251), + tip: big.NewInt(1000000), + useL2: true, + useConfig: false, + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: nil, + rewardErr: fmt.Errorf("reward error"), + wantDropped: false, + wantGasFeeCap: big.NewInt(1000502), + }, + { + name: "SWC Beta: use configured gas price", + minProfit: big.NewInt(0), + gasFeeCap: big.NewInt(10002510), + tip: big.NewInt(1000000), + useL2: true, + useConfig: true, + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + wantDropped: false, + wantGasFeeCap: new(big.Int).SetInt64(10002510), + }, + { + name: "SWC Beta: use configured gas price, but dropped", + minProfit: big.NewInt(0), + gasFeeCap: new(big.Int).Mul(big.NewInt(1), ether), + tip: big.NewInt(1000000), + useL2: true, + useConfig: true, + l1Fee: new(big.Int).Mul(big.NewInt(25), gwei), + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(25), ether), + rewardErr: nil, + wantDropped: true, + wantGasFeeCap: nil, + }, + { + name: "Sepolia: relax the gas price until minimal profit remains", + minProfit: big.NewInt(0), + gasFeeCap: new(big.Int).Mul(big.NewInt(5), gwei), + tip: new(big.Int).Mul(big.NewInt(1), gwei), + useL2: false, + useConfig: false, + l1Fee: nil, + l1FeeErr: nil, + reward: new(big.Int).Exp(big.NewInt(10), big.NewInt(17), nil), // 0.1 eth + rewardErr: nil, + wantDropped: false, + wantGasFeeCap: new(big.Int).SetInt64(200000000000), }, } @@ -78,20 +225,18 @@ func Test_l1MiningAPI_checkGasPrice(t *testing.T) { tc.minProfit, tc.gasFeeCap, tc.tip, - tc.estimatedGas, - tc.safeGas, + estimatedGas, + safeGas, tc.useL2, tc.useConfig, lgr, ) - - if tc.wantError { - assert.Error(t, gotErr, "expected an error but did not get one") - assert.Nil(t, gotGasFeeCap, "gasFeeCap should be nil if error occurs") + if tc.wantDropped { + assert.ErrorIs(t, gotErr, errDropped, "expected an dropped error, but got none") } else { - assert.NoError(t, gotErr, "did not expect an error, but got one") - assert.Equal(t, tc.wantGasFeeCap, gotGasFeeCap, "unexpected final gasFeeCap") + assert.NoError(t, gotErr, "unexpected error") } + assert.Equal(t, tc.wantGasFeeCap, gotGasFeeCap, "unexpected final gasFeeCap") }) } } From cace59d4e9c22ceb3abacff66929e3a59ac18dbc Mon Sep 17 00:00:00 2001 From: syntrust Date: Thu, 23 Jan 2025 19:21:00 +0800 Subject: [PATCH 14/14] test --- ethstorage/miner/l1_mining_api_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/ethstorage/miner/l1_mining_api_test.go b/ethstorage/miner/l1_mining_api_test.go index e354a930..112fe13e 100644 --- a/ethstorage/miner/l1_mining_api_test.go +++ b/ethstorage/miner/l1_mining_api_test.go @@ -206,7 +206,21 @@ func Test_l1MiningAPI_checkGasPrice(t *testing.T) { reward: new(big.Int).Exp(big.NewInt(10), big.NewInt(17), nil), // 0.1 eth rewardErr: nil, wantDropped: false, - wantGasFeeCap: new(big.Int).SetInt64(200000000000), + wantGasFeeCap: new(big.Int).Mul(big.NewInt(200), gwei), + }, + { + name: "Sepolia: relax the gas price until minimal profit remains, but cut by tx fee cap", + minProfit: big.NewInt(0), + gasFeeCap: new(big.Int).Mul(big.NewInt(5), gwei), + tip: new(big.Int).Mul(big.NewInt(1), gwei), + useL2: false, + useConfig: false, + l1Fee: nil, + l1FeeErr: nil, + reward: new(big.Int).Mul(big.NewInt(1), ether), + rewardErr: nil, + wantDropped: false, + wantGasFeeCap: new(big.Int).SetInt64(1666666666666), }, }