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 5 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(seqNumber, l1Info)
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
92 changes: 85 additions & 7 deletions op-node/rollup/derive/l1_block_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,19 @@ 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
L1InfoFuncIsthmusSignature = "setL2BlockValuesIsthmus()"
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]
L1InfoFuncIsthmusBytes4 = crypto.Keccak256([]byte(L1InfoFuncIsthmusSignature))[: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 @@ -144,7 +149,7 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error {
return nil
}

// Ecotone Binary Format
// Isthmus & Ecotone Binary Format
// +---------+--------------------------+
// | Bytes | Field |
// +---------+--------------------------+
Expand All @@ -159,7 +164,17 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error {
// | 32 | BlockHash |
// | 32 | BatcherHash |
// +---------+--------------------------+
func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) {
// Isthmus is the same as Ecotone
out, err := info.marshalBinaryEcotone()
if err != nil {
return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err)
}
// replace the first 4 bytes of the signature with L1InfoFuncIsthmusBytes4
copy(out[:4], L1InfoFuncIsthmusBytes4) // CHECK: is this correct? or is it too nasty?
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
return out, nil

}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no newline

Copy link
Collaborator Author

@skeletor-spaceman skeletor-spaceman Aug 12, 2024

Choose a reason for hiding this comment

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

@tynes not sure what you mean. do you want a new-line? the other set of 3 unmarshal... are all clumped together

func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) {
w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen))
if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil {
Expand Down Expand Up @@ -200,15 +215,29 @@ func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) {
return w.Bytes(), nil
}

func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error {
return info.unmarshalBinaryEcotoneAndIsthmus(data, true)
}
func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error {
return info.unmarshalBinaryEcotoneAndIsthmus(data)
}

// optional isIsthmus flag
func (info *L1BlockInfo) unmarshalBinaryEcotoneAndIsthmus(data []byte, isIsthmus ...bool) error {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
if len(data) != L1InfoEcotoneLen {
return fmt.Errorf("data is unexpected length: %d", len(data))
}
r := bytes.NewReader(data)

var err error
if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncEcotoneBytes4); err != nil {
return err
if isIsthmus != nil && isIsthmus[0] {
if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncIsthmusBytes4); err != nil {
return err
}
} else {
if _, err := solabi.ReadAndValidateSignature(r, L1InfoFuncEcotoneBytes4); err != nil {
return err
}
}
if err := binary.Read(r, binary.BigEndian, &info.BaseFeeScalar); err != nil {
return ErrInvalidFormat
Expand Down Expand Up @@ -250,12 +279,21 @@ func isEcotoneButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) boo
return rollupCfg.IsEcotone(l2BlockTime) && !rollupCfg.IsEcotoneActivationBlock(l2BlockTime)
}

// isInteropButNotFirstBlock returns whether the specified block is subject to the Isthmus upgrade,
// but is not the actiation block itself.
func isInteropButNotFirstBlock(rollupCfg *rollup.Config, l2BlockTime uint64) bool {
return rollupCfg.IsInterop(l2BlockTime) && !rollupCfg.IsInteropActivationBlock(l2BlockTime)
}
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved

// L1BlockInfoFromBytes is the inverse of L1InfoDeposit, to see where the L2 chain is derived from
func L1BlockInfoFromBytes(rollupCfg *rollup.Config, l2BlockTime uint64, data []byte) (*L1BlockInfo, error) {
var info L1BlockInfo
if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) {
return &info, info.unmarshalBinaryEcotone(data)
}
if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
return &info, info.unmarshalBinaryIsthmus(data)
}
return &info, info.unmarshalBinaryBedrock(data)
}

Expand All @@ -271,6 +309,7 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber
BatcherAddr: sysCfg.BatcherAddr,
}
var data []byte

skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) {
l1BlockInfo.BlobBaseFee = block.BlobBaseFee()
if l1BlockInfo.BlobBaseFee == nil {
Expand All @@ -283,11 +322,19 @@ func L1InfoDeposit(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNumber
}
l1BlockInfo.BlobBaseFeeScalar = scalars.BlobBaseFeeScalar
l1BlockInfo.BaseFeeScalar = scalars.BaseFeeScalar
out, err := l1BlockInfo.marshalBinaryEcotone()
if err != nil {
return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err)
if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
out, err := l1BlockInfo.marshalBinaryIsthmus()
if err != nil {
return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err)
}
data = out
} else {
out, err := l1BlockInfo.marshalBinaryEcotone()
if err != nil {
return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err)
}
data = out
}
data = out
} else {
l1BlockInfo.L1FeeOverhead = sysCfg.Overhead
l1BlockInfo.L1FeeScalar = sysCfg.Scalar
Expand Down Expand Up @@ -335,3 +382,34 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu
}
return opaqueL1Tx, nil
}

func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.DepositTx, error) {
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.

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: DepositsCompleteBytes4,
}
return out, nil
}

func DepositsCompleteBytes(seqNumber uint64, l1Info eth.BlockInfo) ([]byte, error) {
dep, err := DepositsCompleteDeposit(seqNumber, l1Info)
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