From 66a2454120629897be518d8ccb223e33ed0eb348 Mon Sep 17 00:00:00 2001 From: Lee Bousfield Date: Wed, 24 Apr 2024 15:58:17 -0500 Subject: [PATCH] Clean up retryable escrow accounts in ArbOS 30 --- arbos/arbosState/arbosstate.go | 13 +++++++++++++ arbos/tx_processor.go | 2 ++ arbos/util/transfer.go | 4 ++++ system_tests/common_test.go | 7 +++++++ system_tests/retryable_test.go | 27 +++++++++++++++++++-------- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/arbos/arbosState/arbosstate.go b/arbos/arbosState/arbosstate.go index 9e3b90532e..b753797822 100644 --- a/arbos/arbosState/arbosstate.go +++ b/arbos/arbosState/arbosstate.go @@ -320,9 +320,22 @@ func (state *ArbosState) UpgradeArbosVersion( case 20: // Update Brotli compression level for fast compression from 0 to 1 ensure(state.SetBrotliCompressionLevel(1)) + // ArbOS versions 21 through 29 are left to Orbit chains for custom upgrades. + case 30: + if !chainConfig.DebugMode() { + // This upgrade isn't finalized so we only want to support it for testing + return fmt.Errorf( + "the chain is upgrading to unsupported ArbOS version %v, %w", + nextArbosVersion, + ErrFatalNodeOutOfDate, + ) + } + // no state changes needed default: if nextArbosVersion >= 12 && nextArbosVersion <= 19 { // ArbOS versions 12 through 19 are left to Orbit chains for custom upgrades. + } else if nextArbosVersion >= 21 && nextArbosVersion <= 29 { + // ArbOS versions 21 through 29 are left to Orbit chains for custom upgrades. } else { return fmt.Errorf( "the chain is upgrading to unsupported ArbOS version %v, %w", diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index 569edb7c63..06ea51bcb9 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -143,6 +143,8 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r // We intentionally use the variant here that doesn't do tracing, // because this transfer is represented as the outer eth transaction. // This transfer is necessary because we don't actually invoke the EVM. + // Since MintBalance already called AddBalance on `from`, + // we don't have EIP-161 concerns around not touching `from`. core.Transfer(evm.StateDB, from, *to, value) return true, 0, nil, nil case *types.ArbitrumInternalTx: diff --git a/arbos/util/transfer.go b/arbos/util/transfer.go index 3a81181200..dd6a0807db 100644 --- a/arbos/util/transfer.go +++ b/arbos/util/transfer.go @@ -33,6 +33,10 @@ func TransferBalance( return fmt.Errorf("%w: addr %v have %v want %v", vm.ErrInsufficientBalance, *from, balance, amount) } evm.StateDB.SubBalance(*from, amount) + if evm.Context.ArbOSVersion >= 30 { + // ensure the from account is "touched" for EIP-161 + evm.StateDB.AddBalance(*from, common.Big0) + } } if to != nil { evm.StateDB.AddBalance(*to, amount) diff --git a/system_tests/common_test.go b/system_tests/common_test.go index a0bed2785d..af9cac2801 100644 --- a/system_tests/common_test.go +++ b/system_tests/common_test.go @@ -186,6 +186,13 @@ func (b *NodeBuilder) DefaultConfig(t *testing.T, withL1 bool) *NodeBuilder { return b } +func (b *NodeBuilder) WithArbOSVersion(arbosVersion uint64) *NodeBuilder { + newChainConfig := *b.chainConfig + newChainConfig.ArbitrumChainParams.InitialArbOSVersion = arbosVersion + b.chainConfig = &newChainConfig + return b +} + func (b *NodeBuilder) Build(t *testing.T) func() { if b.execConfig.RPC.MaxRecreateStateDepth == arbitrum.UninitializedMaxRecreateStateDepth { if b.execConfig.Caching.Archive { diff --git a/system_tests/retryable_test.go b/system_tests/retryable_test.go index b0691db173..f7c7cec54c 100644 --- a/system_tests/retryable_test.go +++ b/system_tests/retryable_test.go @@ -31,7 +31,7 @@ import ( "github.com/offchainlabs/nitro/util/colors" ) -func retryableSetup(t *testing.T) ( +func retryableSetup(t *testing.T, modifyNodeConfig ...func(*NodeBuilder)) ( *NodeBuilder, *bridgegen.Inbox, func(*types.Receipt) *types.Transaction, @@ -40,6 +40,9 @@ func retryableSetup(t *testing.T) ( ) { ctx, cancel := context.WithCancel(context.Background()) builder := NewNodeBuilder(ctx).DefaultConfig(t, true) + for _, f := range modifyNodeConfig { + f(builder) + } builder.Build(t) builder.L2Info.GenerateAccount("User2") @@ -200,9 +203,11 @@ func TestSubmitRetryableImmediateSuccess(t *testing.T) { } } -func TestSubmitRetryableEmptyEscrow(t *testing.T) { +func testSubmitRetryableEmptyEscrow(t *testing.T, arbosVersion uint64) { t.Parallel() - builder, delayedInbox, lookupL2Tx, ctx, teardown := retryableSetup(t) + builder, delayedInbox, lookupL2Tx, ctx, teardown := retryableSetup(t, func(builder *NodeBuilder) { + builder.WithArbOSVersion(arbosVersion) + }) defer teardown() user2Address := builder.L2Info.GetAddress("User2") @@ -273,14 +278,20 @@ func TestSubmitRetryableEmptyEscrow(t *testing.T) { escrowAccount := retryables.RetryableEscrowAddress(l2Tx.Hash()) state, err := builder.L2.ExecNode.ArbInterface.BlockChain().State() Require(t, err) - escrowCodeHash := state.GetCodeHash(escrowAccount) - if escrowCodeHash == (common.Hash{}) { - Fatal(t, "Escrow account deleted (or not created)") - } else if escrowCodeHash != types.EmptyCodeHash { - Fatal(t, "Escrow account has unexpected code hash", escrowCodeHash) + escrowExists := state.Exist(escrowAccount) + if escrowExists != (arbosVersion < 30) { + Fatal(t, "Escrow account existance", escrowExists, "doesn't correspond to ArbOS version", arbosVersion) } } +func TestSubmitRetryableEmptyEscrowArbOS20(t *testing.T) { + testSubmitRetryableEmptyEscrow(t, 20) +} + +func TestSubmitRetryableEmptyEscrowArbOS30(t *testing.T) { + testSubmitRetryableEmptyEscrow(t, 30) +} + func TestSubmitRetryableFailThenRetry(t *testing.T) { t.Parallel() builder, delayedInbox, lookupL2Tx, ctx, teardown := retryableSetup(t)