From b7df2c700864e27f7cda2235d47c81760cdae9e9 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 3 Apr 2023 22:29:40 -0600 Subject: [PATCH 1/6] Update L1 pricing params to more accurate values in ArbOS version 11 --- arbos/arbosState/arbosstate.go | 15 +++++++++++++-- arbos/l1pricing/l1pricing.go | 9 +++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/arbos/arbosState/arbosstate.go b/arbos/arbosState/arbosstate.go index bb9456e139..57c334808b 100644 --- a/arbos/arbosState/arbosstate.go +++ b/arbos/arbosState/arbosstate.go @@ -296,7 +296,16 @@ func (state *ArbosState) UpgradeArbosVersion( ErrFatalNodeOutOfDate, ) } - // no state changes needed + // Update the PerBatchGasCost to a more accurate value compared to the old v6 default. + ensure(state.l1PricingState.SetPerBatchGasCost(l1pricing.InitialPerBatchGasCostV12)) + + // We had mistakenly initialized AmortizedCostCapBips to math.MaxUint64 in older versions, + // but the correct value to disable the amortization cap is 0. + oldAmortizationCap, err := state.l1PricingState.AmortizedCostCapBips() + ensure(err) + if oldAmortizationCap == math.MaxUint64 { + ensure(state.l1PricingState.SetAmortizedCostCapBips(0)) + } default: return fmt.Errorf( "the chain is upgrading to unsupported ArbOS version %v, %w", @@ -308,7 +317,9 @@ func (state *ArbosState) UpgradeArbosVersion( } if firstTime && upgradeTo >= 6 { - state.Restrict(state.l1PricingState.SetPerBatchGasCost(l1pricing.InitialPerBatchGasCostV6)) + if upgradeTo < 11 { + state.Restrict(state.l1PricingState.SetPerBatchGasCost(l1pricing.InitialPerBatchGasCostV6)) + } state.Restrict(state.l1PricingState.SetEquilibrationUnits(l1pricing.InitialEquilibrationUnitsV6)) state.Restrict(state.l2PricingState.SetSpeedLimitPerSecond(l2pricing.InitialSpeedLimitPerSecondV6)) state.Restrict(state.l2PricingState.SetMaxPerBlockGasLimit(l2pricing.InitialPerBlockGasLimitV6)) diff --git a/arbos/l1pricing/l1pricing.go b/arbos/l1pricing/l1pricing.go index fb33c2e93c..3538b40bac 100644 --- a/arbos/l1pricing/l1pricing.go +++ b/arbos/l1pricing/l1pricing.go @@ -72,10 +72,11 @@ const ( ) const ( - InitialInertia = 10 - InitialPerUnitReward = 10 - InitialPricePerUnitWei = 50 * params.GWei - InitialPerBatchGasCostV6 = 100000 + InitialInertia = 10 + InitialPerUnitReward = 10 + InitialPricePerUnitWei = 50 * params.GWei + InitialPerBatchGasCostV6 = 100_000 + InitialPerBatchGasCostV12 = 210_000 // overriden as part of the upgrade ) // one minute at 100000 bytes / sec From 2f3490a213a479a0791bb27f9f04cab104564e30 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Mon, 3 Apr 2023 22:54:41 -0600 Subject: [PATCH 2/6] Fix ArbOwner_test with new default amortization cost --- precompiles/ArbOwner_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/precompiles/ArbOwner_test.go b/precompiles/ArbOwner_test.go index 85a62a3ef3..191ea02552 100644 --- a/precompiles/ArbOwner_test.go +++ b/precompiles/ArbOwner_test.go @@ -4,11 +4,10 @@ package precompiles import ( - "github.com/offchainlabs/nitro/arbos/l1pricing" "math/big" "testing" - "github.com/ethereum/go-ethereum/common/math" + "github.com/offchainlabs/nitro/arbos/l1pricing" "github.com/offchainlabs/nitro/arbos/arbosState" "github.com/offchainlabs/nitro/arbos/burn" @@ -94,7 +93,7 @@ func TestArbOwner(t *testing.T) { costCap, err := gasInfo.GetAmortizedCostCapBips(callCtx, evm) Require(t, err) - if costCap != math.MaxUint64 { + if costCap != 0 { Fail(t, costCap) } newCostCap := uint64(77734) From b306582ded6a200b36d61dc43a516c469057c16c Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Tue, 1 Aug 2023 17:01:39 -0500 Subject: [PATCH 3/6] add disable L1 charging to eth_call --- arbos/tx_processor.go | 5 ++++- go-ethereum | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index d0f999d0de..ae3b98009d 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -385,7 +385,10 @@ func (p *TxProcessor) GasChargingHook(gasRemaining *uint64) (common.Address, err poster = p.evm.Context.Coinbase } - if basefee.Sign() > 0 { + if p.msg.TxRunMode != core.MessageEthcallMode { + p.msg.SkipL1Charging = false + } + if basefee.Sign() > 0 && !p.msg.SkipL1Charging { // Since tips go to the network, and not to the poster, we use the basefee. // Note, this only determines the amount of gas bought, not the price per gas. diff --git a/go-ethereum b/go-ethereum index 3725b60e04..491fee52fa 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 3725b60e0494df145672ab67dd3ec18a85a2b5d1 +Subproject commit 491fee52faf1e7dfcf5431f91a68e95ba779ea66 From c095eb7133eb0ef55a79c669f5530f0b71760853 Mon Sep 17 00:00:00 2001 From: ganeshvanahalli Date: Wed, 2 Aug 2023 10:57:21 -0500 Subject: [PATCH 4/6] add tests --- arbos/tx_processor.go | 2 +- system_tests/estimation_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index ae3b98009d..2bb02276ee 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -385,7 +385,7 @@ func (p *TxProcessor) GasChargingHook(gasRemaining *uint64) (common.Address, err poster = p.evm.Context.Coinbase } - if p.msg.TxRunMode != core.MessageEthcallMode { + if p.msg.TxRunMode == core.MessageCommitMode { p.msg.SkipL1Charging = false } if basefee.Sign() > 0 && !p.msg.SkipL1Charging { diff --git a/system_tests/estimation_test.go b/system_tests/estimation_test.go index 2a416ad179..26b5a78145 100644 --- a/system_tests/estimation_test.go +++ b/system_tests/estimation_test.go @@ -218,3 +218,36 @@ func TestComponentEstimate(t *testing.T) { Fatal(t, l2Estimate, l2Used) } } + +func TestDisableL1Charging(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + _, node, client := CreateTestL2(t, ctx) + defer node.StopAndWait() + addr := common.HexToAddress("0x12345678") + + gasWithL1Charging, err := client.EstimateGas(ctx, ethereum.CallMsg{To: &addr}) + Require(t, err) + + gasWithoutL1Charging, err := client.EstimateGas(ctx, ethereum.CallMsg{To: &addr, SkipL1Charging: true}) + Require(t, err) + + if gasWithL1Charging <= gasWithoutL1Charging { + Fatal(t, "SkipL1Charging didn't disable L1 charging") + } + if gasWithoutL1Charging != params.TxGas { + Fatal(t, "Incorrect gas estimate with disabled L1 charging") + } + + _, err = client.CallContract(ctx, ethereum.CallMsg{To: &addr, Gas: gasWithL1Charging}, nil) + Require(t, err) + + _, err = client.CallContract(ctx, ethereum.CallMsg{To: &addr, Gas: gasWithoutL1Charging}, nil) + if err == nil { + Fatal(t, "CallContract passed with insufficient gas") + } + + _, err = client.CallContract(ctx, ethereum.CallMsg{To: &addr, Gas: gasWithoutL1Charging, SkipL1Charging: true}, nil) + Require(t, err) +} From 8aee7527365bc625eb19a664903be71dc756eb13 Mon Sep 17 00:00:00 2001 From: Tsahi Zidenberg Date: Wed, 2 Aug 2023 15:45:22 -0600 Subject: [PATCH 5/6] makefile: fix soft-float build dependency --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 96f15c20ce..b828b6c19c 100644 --- a/Makefile +++ b/Makefile @@ -346,10 +346,9 @@ contracts/test/prover/proofs/%.json: $(arbitrator_cases)/%.wasm $(arbitrator_pro test -f target/lib-wasm/libbrotlidec-static.a || ./scripts/build-brotli.sh -w -d @touch $@ -.make/wasm-lib: $(DEP_PREDICATE) $(ORDER_ONLY_PREDICATE) .make +.make/wasm-lib: $(DEP_PREDICATE) arbitrator/wasm-libraries/soft-float/SoftFloat/build/Wasm-Clang/softfloat.a $(ORDER_ONLY_PREDICATE) .make test -f arbitrator/wasm-libraries/soft-float/bindings32.o || ./scripts/build-brotli.sh -f -d -t . test -f arbitrator/wasm-libraries/soft-float/bindings64.o || ./scripts/build-brotli.sh -f -d -t . - test -f arbitrator/wasm-libraries/soft-float/SoftFloat/build/Wasm-Clang/softfloat.a || ./scripts/build-brotli.sh -f -d -t . @touch $@ .make: From 978eaeb608315c060cf6717fce51a8b4f7e2f489 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Thu, 3 Aug 2023 09:48:34 -0600 Subject: [PATCH 6/6] Fix two batch posters being enabled in staker tests --- go.mod | 1 - go.sum | 2 -- system_tests/staker_test.go | 44 +++++++++++++++---------------------- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/go.mod b/go.mod index 37ab04ff30..509dec9a5b 100644 --- a/go.mod +++ b/go.mod @@ -113,7 +113,6 @@ require ( github.com/hannahhoward/go-pubsub v0.0.0-20200423002714-8d62886cc36e // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/hashicorp/go-multierror v1.1.1 // indirect - github.com/holiman/big v0.0.0-20221017200358-a027dc42d04e // indirect github.com/ipfs/bbloom v0.0.4 // indirect github.com/ipfs/go-bitfield v1.1.0 // indirect github.com/ipfs/go-block-format v0.1.1 // indirect diff --git a/go.sum b/go.sum index ca552ef60a..304f7cc4aa 100644 --- a/go.sum +++ b/go.sum @@ -600,8 +600,6 @@ github.com/hashicorp/vault/api v1.0.4/go.mod h1:gDcqh3WGcR1cpF5AJz/B1UFheUEneMoI github.com/hashicorp/vault/sdk v0.1.13/go.mod h1:B+hVj7TpuQY1Y/GPbCpffmgd+tSEwvhkWnjtSYCaS2M= github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= -github.com/holiman/big v0.0.0-20221017200358-a027dc42d04e h1:pIYdhNkDh+YENVNi3gto8n9hAmRxKxoar0iE6BLucjw= -github.com/holiman/big v0.0.0-20221017200358-a027dc42d04e/go.mod h1:j9cQbcqHQujT0oKJ38PylVfqohClLr3CvDC+Qcg+lhU= github.com/holiman/bloomfilter/v2 v2.0.3 h1:73e0e/V0tCydx14a0SCYS/EWCxgwLZ18CZcZKVu0fao= github.com/holiman/bloomfilter/v2 v2.0.3/go.mod h1:zpoh+gs7qcpqrHr3dB55AMiJwo0iURXE7ZOP9L9hSkA= github.com/holiman/uint256 v1.2.0 h1:gpSYcPLWGv4sG43I2mVLiDZCNDh/EpGjSk8tmtxitHM= diff --git a/system_tests/staker_test.go b/system_tests/staker_test.go index 8a8ef29bf3..2aae1fc794 100644 --- a/system_tests/staker_test.go +++ b/system_tests/staker_test.go @@ -35,7 +35,7 @@ import ( "github.com/offchainlabs/nitro/validator/valnode" ) -func makeBackgroundTxs(ctx context.Context, l2info *BlockchainTestInfo, l2clientA arbutil.L1Interface, l2clientB arbutil.L1Interface, faultyStaker bool) error { +func makeBackgroundTxs(ctx context.Context, l2info *BlockchainTestInfo, l2clientA arbutil.L1Interface) error { for i := uint64(0); ctx.Err() == nil; i++ { l2info.Accounts["BackgroundUser"].Nonce = i tx := l2info.PrepareTx("BackgroundUser", "BackgroundUser", l2info.TransferGas, common.Big0, nil) @@ -47,19 +47,6 @@ func makeBackgroundTxs(ctx context.Context, l2info *BlockchainTestInfo, l2client if err != nil { return err } - if faultyStaker { - // Create a different transaction for the second node - l2info.Accounts["BackgroundUser"].Nonce = i - tx = l2info.PrepareTx("BackgroundUser", "BackgroundUser", l2info.TransferGas, common.Big1, nil) - err = l2clientB.SendTransaction(ctx, tx) - if err != nil { - return err - } - _, err = EnsureTxSucceeded(ctx, l2clientB, tx) - if err != nil { - return err - } - } } return nil } @@ -82,7 +69,11 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) if faultyStaker { l2info.GenerateGenesisAccount("FaultyAddr", common.Big1) } - l2clientB, l2nodeB := Create2ndNodeWithConfig(t, ctx, l2nodeA, l1stack, l1info, &l2info.ArbInitData, arbnode.ConfigDefaultL1Test(), nil) + config := arbnode.ConfigDefaultL1Test() + config.Sequencer.Enable = false + config.DelayedSequencer.Enable = false + config.BatchPoster.Enable = false + _, l2nodeB := Create2ndNodeWithConfig(t, ctx, l2nodeA, l1stack, l1info, &l2info.ArbInitData, config, nil) defer l2nodeB.StopAndWait() nodeAGenesis := l2nodeA.Execution.Backend.APIBackend().CurrentHeader().Hash() @@ -132,6 +123,9 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) _, err = EnsureTxSucceeded(ctx, l1client, tx) Require(t, err) + validatorUtils, err := rollupgen.NewValidatorUtils(l2nodeA.DeployInfo.ValidatorUtils, l1client) + Require(t, err) + valConfig := staker.L1ValidatorConfig{} valWalletA, err := staker.NewContractValidatorWallet(nil, l2nodeA.DeployInfo.ValidatorWalletCreator, l2nodeA.DeployInfo.Rollup, l2nodeA.L1Reader, &l1authA, 0, func(common.Address) {}) @@ -243,12 +237,6 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) Require(t, err) _, err = EnsureTxSucceeded(ctx, l2clientA, tx) Require(t, err) - if faultyStaker { - err = l2clientB.SendTransaction(ctx, tx) - Require(t, err) - _, err = EnsureTxSucceeded(ctx, l2clientB, tx) - Require(t, err) - } // Continually make L2 transactions in a background thread backgroundTxsCtx, cancelBackgroundTxs := context.WithCancel(ctx) @@ -259,7 +247,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) })() go (func() { defer close(backgroundTxsShutdownChan) - err := makeBackgroundTxs(backgroundTxsCtx, l2info, l2clientA, l2clientB, faultyStaker) + err := makeBackgroundTxs(backgroundTxsCtx, l2info, l2clientA) if !errors.Is(err, context.Canceled) { log.Warn("error making background txs", "err", err) } @@ -302,8 +290,11 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) if !challengeMangerTimedOut { // Upgrade the ChallengeManager contract to an implementation which says challenges are always timed out - mockImpl, _, _, err := mocksgen.DeployTimedOutChallengeManager(&deployAuth, l1client) + mockImpl, tx, _, err := mocksgen.DeployTimedOutChallengeManager(&deployAuth, l1client) Require(t, err) + _, err = EnsureTxSucceeded(ctx, l1client, tx) + Require(t, err) + managerAddr := valWalletA.ChallengeManagerAddress() // 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103 proxyAdminSlot := common.BigToHash(arbmath.BigSub(crypto.Keccak256Hash([]byte("eip1967.proxy.admin")).Big(), common.Big1)) @@ -316,7 +307,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) proxyAdmin, err := mocksgen.NewProxyAdminForBinding(proxyAdminAddr, l1client) Require(t, err) - tx, err := proxyAdmin.Upgrade(&deployAuth, managerAddr, mockImpl) + tx, err = proxyAdmin.Upgrade(&deployAuth, managerAddr, mockImpl) Require(t, err) _, err = EnsureTxSucceeded(ctx, l1client, tx) Require(t, err) @@ -342,9 +333,9 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) Require(t, err, "EnsureTxSucceeded failed for staker", stakerName, "tx") } if faultyStaker { - challengeAddr, err := rollup.CurrentChallenge(&bind.CallOpts{}, valWalletAddrA) + conflictInfo, err := validatorUtils.FindStakerConflict(&bind.CallOpts{}, l2nodeA.DeployInfo.Rollup, l1authA.From, l1authB.From, big.NewInt(1024)) Require(t, err) - if challengeAddr != 0 { + if staker.ConflictType(conflictInfo.Ty) == staker.CONFLICT_TYPE_FOUND { cancelBackgroundTxs() } } @@ -357,6 +348,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool) if isHonestZombie { Fatal(t, "staker A became a zombie") } + fmt.Printf("watchtower staker acting:\n") watchTx, err := stakerC.Act(ctx) if err != nil && !strings.Contains(err.Error(), "catch up") { Require(t, err, "watchtower staker failed to act")