Merge pull request #2257 from OffchainLabs/arbos-30-cleanup-escrow

Clean up retryable escrow accounts in ArbOS 30 [NIT-2453]

arbos/arbosState/arbosstate.go

@@ -324,9 +324,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",

arbos/tx_processor.go

@@ -144,6 +144,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, uint256.MustFromBig(value))
 		return true, 0, nil, nil
 	case *types.ArbitrumInternalTx:

arbos/util/transfer.go

@@ -34,6 +34,10 @@ func TransferBalance(
 			return fmt.Errorf("%w: addr %v have %v want %v", vm.ErrInsufficientBalance, *from, balance, amount)
 		}
 		evm.StateDB.SubBalance(*from, uint256.MustFromBig(amount))
+		if evm.Context.ArbOSVersion >= 30 {
+			// ensure the from account is "touched" for EIP-161
+			evm.StateDB.AddBalance(*from, &uint256.Int{})
+		}
 	}
 	if to != nil {
 		evm.StateDB.AddBalance(*to, uint256.MustFromBig(amount))

system_tests/common_test.go

@@ -188,6 +188,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 {

system_tests/retryable_test.go

@@ -32,7 +32,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,
@@ -41,6 +41,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")
@@ -205,9 +208,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")
@@ -278,14 +283,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)