diff --git a/arbos/arbosState/arbosstate.go b/arbos/arbosState/arbosstate.go index 0bc9b85232..d727d5d942 100644 --- a/arbos/arbosState/arbosstate.go +++ b/arbos/arbosState/arbosstate.go @@ -203,11 +203,8 @@ func InitializeArbosState(stateDB vm.StateDB, burner burn.Burner, chainConfig *p merkleAccumulator.InitializeMerkleAccumulator(sto.OpenSubStorage(sendMerkleSubspace)) blockhash.InitializeBlockhashes(sto.OpenSubStorage(blockhashesSubspace)) - // by default, the remapped zero address is the initial chain owner - initialChainOwner := util.RemapL1Address(common.Address{}) - if chainConfig.ArbitrumChainParams.InitialChainOwner != (common.Address{}) { - initialChainOwner = chainConfig.ArbitrumChainParams.InitialChainOwner - } + // may be the zero address + initialChainOwner := chainConfig.ArbitrumChainParams.InitialChainOwner ownersStorage := sto.OpenSubStorage(chainOwnerSubspace) _ = addressSet.Initialize(ownersStorage) _ = addressSet.OpenAddressSet(ownersStorage).Add(initialChainOwner) diff --git a/arbos/incomingmessage.go b/arbos/incomingmessage.go index 0edd5035ef..d2f90104ff 100644 --- a/arbos/incomingmessage.go +++ b/arbos/incomingmessage.go @@ -203,7 +203,7 @@ func (msg *L1IncomingMessage) ParseL2Transactions(chainId *big.Int, batchFetcher ChainId: chainId, L1RequestId: depositRequestId, // Matches the From of parseUnsignedTx - To: util.RemapL1Address(msg.Header.Poster), + To: msg.Header.Poster, Value: tx.Value(), }) return types.Transactions{deposit, tx}, nil @@ -380,7 +380,7 @@ func parseUnsignedTx(rd io.Reader, poster common.Address, requestId *common.Hash case L2MessageKind_UnsignedUserTx: inner = &types.ArbitrumUnsignedTx{ ChainId: chainId, - From: util.RemapL1Address(poster), + From: poster, Nonce: nonce, GasFeeCap: maxFeePerGas.Big(), Gas: gasLimit.Big().Uint64(), @@ -395,7 +395,7 @@ func parseUnsignedTx(rd io.Reader, poster common.Address, requestId *common.Hash inner = &types.ArbitrumContractTx{ ChainId: chainId, RequestId: *requestId, - From: util.RemapL1Address(poster), + From: poster, GasFeeCap: maxFeePerGas.Big(), Gas: gasLimit.Big().Uint64(), To: destination, @@ -410,6 +410,10 @@ func parseUnsignedTx(rd io.Reader, poster common.Address, requestId *common.Hash } func parseEthDepositMessage(rd io.Reader, header *L1IncomingMessageHeader, chainId *big.Int) (*types.Transaction, error) { + to, err := util.AddressFromReader(rd) + if err != nil { + return nil, err + } balance, err := util.HashFromReader(rd) if err != nil { return nil, err @@ -420,7 +424,8 @@ func parseEthDepositMessage(rd io.Reader, header *L1IncomingMessageHeader, chain tx := &types.ArbitrumDepositTx{ ChainId: chainId, L1RequestId: *header.RequestId, - To: util.RemapL1Address(header.Poster), + From: header.Poster, + To: to, Value: balance.Big(), } return types.NewTx(tx), nil @@ -491,7 +496,7 @@ func parseSubmitRetryableMessage(rd io.Reader, header *L1IncomingMessageHeader, tx := &types.ArbitrumSubmitRetryableTx{ ChainId: chainId, RequestId: *header.RequestId, - From: util.RemapL1Address(header.Poster), + From: header.Poster, L1BaseFee: header.L1BaseFee, DepositValue: depositValue.Big(), GasFeeCap: maxFeePerGas.Big(), diff --git a/arbos/tx_processor.go b/arbos/tx_processor.go index 8655894549..6c65ed77f4 100644 --- a/arbos/tx_processor.go +++ b/arbos/tx_processor.go @@ -105,9 +105,6 @@ func (p *TxProcessor) StartTxHook() (endTxNow bool, gasUsed uint64, err error, r switch tx := underlyingTx.GetInner().(type) { case *types.ArbitrumDepositTx: defer (startTracer())() - if p.msg.From() != arbosAddress { - return false, 0, errors.New("deposit not from arbAddress"), nil - } util.MintBalance(p.msg.To(), p.msg.Value(), evm, util.TracingDuringEVM) return true, 0, nil, nil case *types.ArbitrumInternalTx: diff --git a/arbstate/geth_test.go b/arbstate/geth_test.go index 74b71b7e06..075516a8b0 100644 --- a/arbstate/geth_test.go +++ b/arbstate/geth_test.go @@ -73,6 +73,9 @@ func TestEthDepositMessage(t *testing.T) { L1BaseFee: big.NewInt(10000000000000), } msgBuf := bytes.Buffer{} + if err := util.AddressToWriter(addr, &msgBuf); err != nil { + t.Error(err) + } if err := util.HashToWriter(balance, &msgBuf); err != nil { t.Error(err) } @@ -88,7 +91,11 @@ func TestEthDepositMessage(t *testing.T) { secondRequestId := common.BigToHash(big.NewInt(4)) header.RequestId = &secondRequestId + header.Poster = util.RemapL1Address(addr) msgBuf2 := bytes.Buffer{} + if err := util.AddressToWriter(addr, &msgBuf2); err != nil { + t.Error(err) + } if err := util.HashToWriter(balance2, &msgBuf2); err != nil { t.Error(err) } @@ -103,7 +110,7 @@ func TestEthDepositMessage(t *testing.T) { RunMessagesThroughAPI(t, [][]byte{serialized, serialized2}, statedb) - balanceAfter := statedb.GetBalance(util.RemapL1Address(addr)) + balanceAfter := statedb.GetBalance(addr) if balanceAfter.Cmp(new(big.Int).Add(balance.Big(), balance2.Big())) != 0 { Fail(t) } diff --git a/contracts/src/bridge/Inbox.sol b/contracts/src/bridge/Inbox.sol index 5ddfb86d75..3f1e35463e 100644 --- a/contracts/src/bridge/Inbox.sol +++ b/contracts/src/bridge/Inbox.sol @@ -217,24 +217,23 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox { /// Look into retryable tickets if you are interested in this functionality. /// @dev this function should not be called inside contract constructors function depositEth() public payable override whenNotPaused returns (uint256) { - address sender = msg.sender; + address dest = msg.sender; // solhint-disable-next-line avoid-tx-origin - if (!AddressUpgradeable.isContract(sender) && tx.origin == msg.sender) { + if (AddressUpgradeable.isContract(msg.sender) || tx.origin != msg.sender) { // isContract check fails if this function is called during a contract's constructor. // We don't adjust the address for calls coming from L1 contracts since their addresses get remapped // If the caller is an EOA, we adjust the address. // This is needed because unsigned messages to the L2 (such as retryables) // have the L1 sender address mapped. - // Here we preemptively reverse the mapping for EOAs so deposits work as expected - sender = AddressAliasHelper.undoL1ToL2Alias(sender); + dest = AddressAliasHelper.applyL1ToL2Alias(msg.sender); } return _deliverMessage( L1MessageType_ethDeposit, - sender, // arb-os will add the alias to this value - abi.encodePacked(msg.value) + msg.sender, + abi.encodePacked(dest, msg.value) ); } @@ -405,7 +404,11 @@ contract Inbox is DelegateCallAware, PausableUpgradeable, IInbox { ) internal returns (uint256) { if (_messageData.length > MAX_DATA_SIZE) revert DataTooLarge(_messageData.length, MAX_DATA_SIZE); - uint256 msgNum = deliverToBridge(_kind, _sender, keccak256(_messageData)); + uint256 msgNum = deliverToBridge( + _kind, + AddressAliasHelper.applyL1ToL2Alias(_sender), + keccak256(_messageData) + ); emit InboxMessageDelivered(msgNum, _messageData); return msgNum; } diff --git a/contracts/test/contract/sequencerInboxForceInclude.spec.ts b/contracts/test/contract/sequencerInboxForceInclude.spec.ts index 727a8185a1..49c93c2fc4 100644 --- a/contracts/test/contract/sequencerInboxForceInclude.spec.ts +++ b/contracts/test/contract/sequencerInboxForceInclude.spec.ts @@ -30,7 +30,7 @@ import { SequencerInbox__factory, TransparentUpgradeableProxy__factory, } from '../../build/types' -import { initializeAccounts } from './utils' +import { applyAlias, initializeAccounts } from './utils' import { Event } from '@ethersproject/contracts' import { Interface } from '@ethersproject/abi' import { @@ -93,7 +93,7 @@ describe('SequencerInboxForceInclude', async () => { const countAfter = (await bridge.functions.delayedMessageCount())[0].toNumber() expect(countAfter, 'Unexpected inbox count').to.eq(countBefore + 1) - const senderAddr = await sender.getAddress() + const senderAddr = applyAlias(await sender.getAddress()) const messageDeliveredEvent = getMessageDeliveredEvents( sendUnsignedTxReceipt, diff --git a/contracts/test/contract/utils.ts b/contracts/test/contract/utils.ts index 43548c6c19..6636685b95 100644 --- a/contracts/test/contract/utils.ts +++ b/contracts/test/contract/utils.ts @@ -1,5 +1,21 @@ import { ethers } from 'hardhat' import { Signer } from '@ethersproject/abstract-signer' +import { getAddress } from '@ethersproject/address' + +const ADDRESS_ALIAS_OFFSET = BigInt("0x1111000000000000000000000000000000001111"); +const ADDRESS_BIT_LENGTH = 160; +const ADDRESS_NIBBLE_LENGTH = ADDRESS_BIT_LENGTH / 4; + +export const applyAlias = (addr: string) => { + // we use BigInts in here to allow for proper overflow behaviour + // BigInt.asUintN calculates the correct positive modulus + return getAddress( + "0x" + + BigInt.asUintN(ADDRESS_BIT_LENGTH, BigInt(addr) + ADDRESS_ALIAS_OFFSET) + .toString(16) + .padStart(ADDRESS_NIBBLE_LENGTH, "0") + ); +}; export async function initializeAccounts(): Promise { const [account0] = await ethers.getSigners() diff --git a/go-ethereum b/go-ethereum index 8096aa9f82..07e017aa73 160000 --- a/go-ethereum +++ b/go-ethereum @@ -1 +1 @@ -Subproject commit 8096aa9f821207e93cf9994858cadeb767e6dc46 +Subproject commit 07e017aa73e32be92aadb52fa327c552e1b7b118 diff --git a/precompiles/ArbOwner_test.go b/precompiles/ArbOwner_test.go index eaf1eb72be..994e3b7955 100644 --- a/precompiles/ArbOwner_test.go +++ b/precompiles/ArbOwner_test.go @@ -32,8 +32,7 @@ func TestAddressSet(t *testing.T) { callCtx := testContext(caller, evm) // the zero address is an owner by default - ZeroAddressL2 := util.RemapL1Address(common.Address{}) - Require(t, prec.RemoveChainOwner(callCtx, evm, ZeroAddressL2)) + Require(t, prec.RemoveChainOwner(callCtx, evm, common.Address{})) Require(t, prec.AddChainOwner(callCtx, evm, addr1)) Require(t, prec.AddChainOwner(callCtx, evm, addr2))