Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

draft: ban-deposits-interop first RFC draft #11362

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
3d3e767
feat: ban-deposits-interop first RFC draft
skeletor-spaceman Aug 5, 2024
0d78e5a
fix: missing marshalling of tx
skeletor-spaceman Aug 5, 2024
37f17de
feat: code cleanup and renaming
skeletor-spaceman Aug 6, 2024
4f9621a
feat: cleaner implementation, always set isDeposit on
skeletor-spaceman Aug 6, 2024
65b6804
feat: simplified deposits complete tx and added Isthmus L1Info tx
skeletor-spaceman Aug 6, 2024
70b7db0
feat: L1Block make ecotone a public function
skeletor-spaceman Aug 7, 2024
24deb1c
feat: re-organized shared functions
skeletor-spaceman Aug 7, 2024
2e322c9
Merge branch 'develop' of github.com:ethereum-optimism/optimism into …
skeletor-spaceman Aug 7, 2024
f463e8a
feat: revamp unmarshalBinaryIsthmusAndEcotone function
skeletor-spaceman Aug 7, 2024
e90db39
feat: ban deposits interop (#11396)
0xDiscotech Aug 8, 2024
771b319
Merge branch 'develop' into feat/ban-deposits-interop
skeletor-spaceman Aug 8, 2024
2838d6e
fix: removed duplicated function from bad merge
skeletor-spaceman Aug 8, 2024
e40d49f
fix: relevant comments added, optional bool removed
skeletor-spaceman Aug 8, 2024
c89ffa4
fix: sstore of isDeposit
skeletor-spaceman Aug 8, 2024
ebc225d
fix: sstore of isDeposit
skeletor-spaceman Aug 8, 2024
198d683
fix: is deposit relevant tests
skeletor-spaceman Aug 8, 2024
1434e89
fix: is deposit relevant tests
skeletor-spaceman Aug 8, 2024
8e93fcc
fix: reorder marshal funcs
skeletor-spaceman Aug 9, 2024
abe646b
feat: add DepositSource to avoid possible collitions
skeletor-spaceman Aug 9, 2024
13096e8
feat: renames and revert isDeposit on legacy L1 set
skeletor-spaceman Aug 12, 2024
057fc5b
feat: rename Deposit for AfterForceInclude
skeletor-spaceman Aug 12, 2024
5e0d64c
feat: ban deposits interop (#11451)
0xDiscotech Aug 12, 2024
2c370f7
fix: typo
skeletor-spaceman Aug 12, 2024
da494fd
feat: ban deposits interop (#11454)
0xDiscotech Aug 13, 2024
09c549c
feat: ban deposits interop (#11481)
0xDiscotech Aug 14, 2024
bc562de
chore: some renames
skeletor-spaceman Aug 14, 2024
a8fd430
chore: update gas limit of DepositsComplete
skeletor-spaceman Aug 15, 2024
0a803e5
chore: merged main
skeletor-spaceman Aug 15, 2024
78aa2c4
Merge branch 'develop' of github.com:ethereum-optimism/optimism into …
skeletor-spaceman Aug 16, 2024
5eaf39d
fix: deprecate Interop L1 block
skeletor-spaceman Aug 16, 2024
d5ba5d1
fix: missed interop filename changes
skeletor-spaceman Aug 16, 2024
f2d8fb8
feat: merged from upstream
skeletor-spaceman Aug 22, 2024
ccf505d
feat: ban deposits client tests (#11504)
skeletor-spaceman Aug 29, 2024
a17e357
feat: merged from develop
skeletor-spaceman Aug 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion op-node/rollup/derive/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,19 @@ func (ba *FetchingAttributesBuilder) PreparePayloadAttributes(ctx context.Contex
return nil, NewCriticalError(fmt.Errorf("failed to create l1InfoTx: %w", err))
}

skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
txs := make([]hexutil.Bytes, 0, 1+len(depositTxs)+len(upgradeTxs))
var postDeposits []hexutil.Bytes
if ba.rollupCfg.IsInterop(nextL2Time) {
depositsCompleteTx, err := DepositsCompleteBytes(ba.rollupCfg, sysConfig, seqNumber, l1Info, nextL2Time)
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err))
}
postDeposits = append(postDeposits, depositsCompleteTx)
}

txs := make([]hexutil.Bytes, 0, 1+len(depositTxs)+len(postDeposits)+len(upgradeTxs))
txs = append(txs, l1InfoTx)
txs = append(txs, depositTxs...)
txs = append(txs, postDeposits...)
txs = append(txs, upgradeTxs...)

var withdrawals *types.Withdrawals
Expand Down
81 changes: 81 additions & 0 deletions op-node/rollup/derive/l1_block_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,17 @@ import (
const (
L1InfoFuncBedrockSignature = "setL1BlockValues(uint64,uint64,uint256,bytes32,uint64,bytes32,uint256,uint256)"
L1InfoFuncEcotoneSignature = "setL1BlockValuesEcotone()"
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
DepositsCompleteSignature = "depositsComplete()"
L1InfoArguments = 8
L1InfoBedrockLen = 4 + 32*L1InfoArguments
L1InfoEcotoneLen = 4 + 32*5 // after Ecotone upgrade, args are packed into 5 32-byte slots
DepositsCompleteLen = 4 // only the selector
)

var (
L1InfoFuncBedrockBytes4 = crypto.Keccak256([]byte(L1InfoFuncBedrockSignature))[:4]
L1InfoFuncEcotoneBytes4 = crypto.Keccak256([]byte(L1InfoFuncEcotoneSignature))[:4]
DepositsCompleteBytes4 = crypto.Keccak256([]byte(DepositsCompleteSignature))[:4]
L1InfoDepositerAddress = common.HexToAddress("0xdeaddeaddeaddeaddeaddeaddeaddeaddead0001")
L1BlockAddress = predeploys.L1BlockAddr
ErrInvalidFormat = errors.New("invalid ecotone l1 block info format")
Expand Down Expand Up @@ -259,6 +262,34 @@ func L1BlockInfoFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []b
return &info, info.unmarshalBinaryBedrock(data)
}

func (info *L1BlockInfo) marshalDepositsComplete() ([]byte, error) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
w := bytes.NewBuffer(make([]byte, 0, DepositsCompleteLen))
if err := solabi.WriteSignature(w, []byte(DepositsCompleteBytes4)); err != nil {
return nil, err
}
return w.Bytes(), nil
}

func (info *L1BlockInfo) unmarshalDepositsComplete(data []byte) error {
if len(data) != DepositsCompleteLen {
return fmt.Errorf("data is unexpected length: %d", len(data))
}
r := bytes.NewReader(data)

if _, err := solabi.ReadAndValidateSignature(r, DepositsCompleteBytes4); err != nil {
return err
}
if !solabi.EmptyReader(r) {
return errors.New("too many bytes")
}
return nil
}

func DepositCompletedFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []byte) (*L1BlockInfo, error) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
var info L1BlockInfo
return &info, info.unmarshalDepositsComplete(data)
}

// L1InfoDeposit creates a L1 Info deposit transaction based on the L1 block,
// and the L2 block-height difference with the start of the epoch.
func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) {
Expand Down Expand Up @@ -335,3 +366,53 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu
}
return opaqueL1Tx, nil
}

func DepositsCompleteDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, block eth.BlockInfo, l2BlockTime uint64) (*types.DepositTx, error) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
l1BlockInfo := L1BlockInfo{
Number: block.NumberU64(),
Time: block.Time(),
BaseFee: block.BaseFee(),
BlockHash: block.Hash(),
SequenceNumber: seqNumber,
BatcherAddr: sysCfg.BatcherAddr,
}
data, err := l1BlockInfo.marshalDepositsComplete()
if err != nil {
return nil, fmt.Errorf("failed to marshal Bedrock l1 block info: %w", err)
}
source := L1InfoDepositSource{
L1BlockHash: block.Hash(),
SeqNumber: seqNumber,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of the "sourcehash" is that it helps uniquely identify deposit transactions, because they do not include a nonce.
While technically ok to reuse the same source-hash as the system L1Info deposit (since different tx contents are mixed in), I would prefer us to specify a new type of deposit-source, to really ensure the two deposits get a different tx-hash, and not surprise any users of the source-hash property.

// Set a normal gas limit with `IsSystemTransaction` to ensure
// that the DepositCompleted Transaction does not run out of gas.
out := &types.DepositTx{
SourceHash: source.SourceHash(),
From: L1InfoDepositerAddress,
To: &L1BlockAddress,
Mint: nil,
Value: big.NewInt(0),
Gas: 50_000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we specified/checked how much gas this TX needs conservatively to run?

IsSystemTransaction: false,
Data: data,
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
}
// With the regolith fork we disable the IsSystemTx functionality, and allocate real gas
if rollupCfg.IsRegolith(l2BlockTime) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
out.IsSystemTransaction = false
out.Gas = RegolithSystemTxGas
}
return out, nil
}

func DepositsCompleteBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber uint64, l1Info eth.BlockInfo, l2BlockTime uint64) ([]byte, error) {
dep, err := DepositsCompleteDeposit(rollupCfg, sysCfg, seqNumber, l1Info, l2BlockTime)
if err != nil {
return nil, fmt.Errorf("failed to create DepositsComplete tx: %w", err)
}
depositsCompleteTx := types.NewTx(dep)
opaqueDepositsCompleteTx, err := depositsCompleteTx.MarshalBinary()
if err != nil {
return nil, fmt.Errorf("failed to encode DepositsComplete tx: %w", err)
}
return opaqueDepositsCompleteTx, nil
}
1 change: 1 addition & 0 deletions op-node/rollup/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ func (c *Config) IsHoloceneActivationBlock(l2BlockTime uint64) bool {
!c.IsHolocene(l2BlockTime-c.BlockTime)
}

// TODO rename to IsIsthmusActivationBlock (this will require quite a bit of renaming)
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
func (c *Config) IsInteropActivationBlock(l2BlockTime uint64) bool {
return c.IsInterop(l2BlockTime) &&
l2BlockTime >= c.BlockTime &&
Expand Down
6 changes: 6 additions & 0 deletions packages/contracts-bedrock/src/L2/CrossL2Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ error InvalidChainId();
/// @notice Thrown when trying to execute a cross chain message and the target call fails.
error TargetCallFailed();

/// @notice Thrown when trying to execute a cross chain message on a deposit transaction.
error NoExecutingDeposits();

/// @custom:proxied
/// @custom:predeploy 0x4200000000000000000000000000000000000022
/// @title CrossL2Inbox
Expand Down Expand Up @@ -114,6 +117,9 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware {
payable
reentrantAware
{
// We need to know if this is being called on a depositTx
if (L1_BLOCK.isDeposit()) revert NoExecutingDeposits();

skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
if (_id.timestamp > block.timestamp) revert InvalidTimestamp();
if (!IDependencySet(Predeploys.L1_BLOCK_ATTRIBUTES).isInDependencySet(_id.chainId)) {
revert InvalidChainId();
Expand Down
26 changes: 25 additions & 1 deletion packages/contracts-bedrock/src/L2/L1Block.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ contract L1Block is ISemver, IGasToken {
/// @notice The latest L1 blob base fee.
uint256 public blobBaseFee;

/// @notice The isDeposit flag.
bool public isDeposit;
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved

/// @custom:semver 1.4.1-beta.1
function version() public pure virtual returns (string memory) {
return "1.4.1-beta.1";
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -87,6 +90,11 @@ contract L1Block is ISemver, IGasToken {
return token != Constants.ETHER;
}

function isDeposit() returns (bool _isDeposit) external view {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
require(msg.sender == CROSS_L2_INBOX, "L1Block: only the CrossL2Inbox can check if it is a deposit");
_isDeposit = isDeposit;
}

/// @custom:legacy
/// @notice Updates the L1 block values.
/// @param _number L1 blocknumber.
Expand All @@ -97,6 +105,7 @@ contract L1Block is ISemver, IGasToken {
/// @param _batcherHash Versioned hash to authenticate batcher by.
/// @param _l1FeeOverhead L1 fee overhead.
/// @param _l1FeeScalar L1 fee scalar.
/// @param _isDeposit isDeposit flag
function setL1BlockValues(
uint64 _number,
uint64 _timestamp,
Expand All @@ -105,7 +114,8 @@ contract L1Block is ISemver, IGasToken {
uint64 _sequenceNumber,
bytes32 _batcherHash,
uint256 _l1FeeOverhead,
uint256 _l1FeeScalar
uint256 _l1FeeScalar,
bool _isDeposit
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
)
external
{
Expand All @@ -119,6 +129,14 @@ contract L1Block is ISemver, IGasToken {
batcherHash = _batcherHash;
l1FeeOverhead = _l1FeeOverhead;
l1FeeScalar = _l1FeeScalar;
isDeposit = _isDeposit;
}

// TODO natspec
function setL1BlockValuesIsthmus() external {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
isDeposit = true;
setL1BlockValuesEcotone(calldata);
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
// TODO ^ this is an example... we might need to do an assembly low-level call here
}

/// @notice Updates the L1 block values for an Ecotone upgraded chain.
Expand Down Expand Up @@ -162,4 +180,10 @@ contract L1Block is ISemver, IGasToken {

emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol });
}

/// @notice Resets the isDeposit flag.
function depositsComplete() external {
if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor();
isDeposit = false;
}
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
}
Loading