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 28 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
14 changes: 13 additions & 1 deletion op-node/rollup/derive/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,21 @@ 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 afterForceIncludeTxs []hexutil.Bytes
// TODO I'm usure if this needs to be only done AFTER interop is enabled, but not on the same block.
// This is similar to what Proto mentioned about blocking the ExecutingMessages on the same block as the interop activation.
if ba.rollupCfg.IsInterop(nextL2Time) {
depositsCompleteTx, err := DepositsCompleteBytes(seqNumber, l1Info)
if err != nil {
return nil, NewCriticalError(fmt.Errorf("failed to create depositsCompleteTx: %w", err))
}
afterForceIncludeTxs = append(afterForceIncludeTxs, depositsCompleteTx)
}

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

var withdrawals *types.Withdrawals
Expand Down
23 changes: 20 additions & 3 deletions op-node/rollup/derive/deposit_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ type UserDepositSource struct {
}

const (
UserDepositSourceDomain = 0
L1InfoDepositSourceDomain = 1
UpgradeDepositSourceDomain = 2
UserDepositSourceDomain = 0
L1InfoDepositSourceDomain = 1
UpgradeDepositSourceDomain = 2
AfterForceIncludeSourceDomain = 3
)

func (dep *UserDepositSource) SourceHash() common.Hash {
Expand Down Expand Up @@ -63,3 +64,19 @@ func (dep *UpgradeDepositSource) SourceHash() common.Hash {
copy(domainInput[32:], intentHash[:])
return crypto.Keccak256Hash(domainInput[:])
}

// Used for DepositsComplete/ResetDeposits post-deposits transactions.
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
type AfterForceIncludeSource struct {
L1BlockHash common.Hash
}

func (dep *AfterForceIncludeSource) SourceHash() common.Hash {
var input [32 * 2]byte
copy(input[:32], dep.L1BlockHash[:])
depositIDHash := crypto.Keccak256Hash(input[:])

var domainInput [32 * 2]byte
binary.BigEndian.PutUint64(domainInput[32-8:32], AfterForceIncludeSourceDomain)
copy(domainInput[32:], depositIDHash[:])
return crypto.Keccak256Hash(domainInput[:])
}
102 changes: 93 additions & 9 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 = "setL1BlockValuesIsthmus()"
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,10 +164,24 @@ func (info *L1BlockInfo) unmarshalBinaryBedrock(data []byte) error {
// | 32 | BlockHash |
// | 32 | BatcherHash |
// +---------+--------------------------+

// Marshal Ecotone and Isthmus
func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) {
w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen))
if err := solabi.WriteSignature(w, L1InfoFuncEcotoneBytes4); err != nil {
out, err := marshalBinaryWithSignature(info, L1InfoFuncEcotoneBytes4)
if err != nil {
return nil, fmt.Errorf("failed to marshal Ecotone l1 block info: %w", err)
}
return out, nil
}
func (info *L1BlockInfo) marshalBinaryIsthmus() ([]byte, error) {
out, err := marshalBinaryWithSignature(info, L1InfoFuncIsthmusBytes4)
if err != nil {
return nil, fmt.Errorf("failed to marshal Isthmus l1 block info: %w", err)
}
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 marshalBinaryWithSignature(info *L1BlockInfo, signature []byte) ([]byte, error) {
w := bytes.NewBuffer(make([]byte, 0, L1InfoEcotoneLen)) // Ecotone and Isthmus have the same length
if err := solabi.WriteSignature(w, signature); err != nil {
return nil, err
}
if err := binary.Write(w, binary.BigEndian, info.BaseFeeScalar); err != nil {
Expand Down Expand Up @@ -200,14 +219,21 @@ func (info *L1BlockInfo) marshalBinaryEcotone() ([]byte, error) {
return w.Bytes(), nil
}

// Unmarshal Ecotone and Isthmus
func (info *L1BlockInfo) unmarshalBinaryEcotone(data []byte) error {
return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncEcotoneBytes4, data)
}
func (info *L1BlockInfo) unmarshalBinaryIsthmus(data []byte) error {
return unmarshalBinaryWithSignatureAndData(info, L1InfoFuncIsthmusBytes4, data)
}
func unmarshalBinaryWithSignatureAndData(info *L1BlockInfo, signature []byte, data []byte) error {
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 {
if _, err := solabi.ReadAndValidateSignature(r, signature); err != nil {
return err
}
if err := binary.Read(r, binary.BigEndian, &info.BaseFeeScalar); err != nil {
Expand Down Expand Up @@ -250,9 +276,24 @@ 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 {
// note from Proto:
// Since we use the pre-interop L1 tx one last time during the upgrade block,
// we must disallow the deposit-txs from using the CrossL2Inbox during this block.
// If the CrossL2Inbox does not exist yet, then it is safe,
// but we have to ensure that the spec and code puts any Interop upgrade-txs after the user deposits.
return rollupCfg.IsInterop(l2BlockTime) && !rollupCfg.IsInteropActivationBlock(l2BlockTime)
}

// 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
// Important, this should be ordered from most recent to oldest
if isInteropButNotFirstBlock(rollupCfg, l2BlockTime) {
skeletor-spaceman marked this conversation as resolved.
Show resolved Hide resolved
return &info, info.unmarshalBinaryIsthmus(data)
}
if isEcotoneButNotFirstBlock(rollupCfg, l2BlockTime) {
return &info, info.unmarshalBinaryEcotone(data)
}
Expand All @@ -271,6 +312,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 +325,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 +385,37 @@ func L1InfoDepositBytes(rollupCfg *rollup.Config, sysCfg eth.SystemConfig, seqNu
}
return opaqueL1Tx, nil
}

func DepositsCompleteDeposit(seqNumber uint64, block eth.BlockInfo) (*types.DepositTx, error) {
source := AfterForceIncludeSource{
L1BlockHash: block.Hash(),
}
out := &types.DepositTx{
SourceHash: source.SourceHash(),
From: L1InfoDepositerAddress,
To: &L1BlockAddress,
Mint: nil,
Value: big.NewInt(0),
// We set the gas limit to 15k to ensure that the DepositsComplete Transaction does not run out of gas.
// GasBenchMark_L1BlockIsthmus_DepositsComplete:test_depositsComplete_benchmark() (gas: 7768)
// GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5768)
// see `test_depositsComplete_benchmark` at: `/packages/contracts-bedrock/test/BenchmarkTest.t.sol`
Gas: 15_000,
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 @@ -467,6 +467,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/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
GasBenchMark_L1BlockIsthmus_DepositsComplete:test_depositsComplete_benchmark() (gas: 7768)
GasBenchMark_L1BlockIsthmus_DepositsComplete_Warm:test_depositsComplete_benchmark() (gas: 5768)
GasBenchMark_L1BlockIsthmus_SetValuesIsthmus:test_setL1BlockValuesIsthmus_benchmark() (gas: 174050)
GasBenchMark_L1BlockIsthmus_SetValuesIsthmus_Warm:test_setL1BlockValuesIsthmus_benchmark() (gas: 5185)
GasBenchMark_L1Block_SetValuesEcotone:test_setL1BlockValuesEcotone_benchmark() (gas: 159033)
GasBenchMark_L1Block_SetValuesEcotone_Warm:test_setL1BlockValuesEcotone_benchmark() (gas: 7539)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 369356)
GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2967496)
GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 564483)
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/scripts/L2Genesis.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ contract L2Genesis is Deployer {
/// @notice This predeploy is following the safety invariant #1.
function setL1Block() public {
if (cfg.useInterop()) {
string memory cname = "L1BlockInterop";
string memory cname = "L1BlockIsthmus";
address impl = Predeploys.predeployToCodeNamespace(Predeploys.L1_BLOCK_ATTRIBUTES);
console.log("Setting %s implementation at: %s", cname, impl);
vm.etch(impl, vm.getDeployedCode(string.concat(cname, ".sol:", cname)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ contract DrippieConfig is Script {
abi.decode(checkparams, (CheckSecrets.Params));
} else if (strcmp(dripcheck, "CheckTrue")) {
// No parameters to decode.
} else {
}
else {
Comment on lines +129 to +130
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same linting weird behavior

console.log("ERROR: unknown drip configuration %s", dripcheck);
revert UnknownDripCheck(dripcheck);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.15;

import { OptimismPortal2 } from "src/L1/OptimismPortal2.sol";
import { L1BlockInterop, ConfigType } from "src/L2/L1BlockInterop.sol";
import { L1BlockIsthmus, ConfigType } from "src/L2/L1BlockIsthmus.sol";
import { Predeploys } from "src/libraries/Predeploys.sol";
import { Constants } from "src/libraries/Constants.sol";

Expand Down Expand Up @@ -47,7 +47,7 @@ contract OptimismPortalInterop is OptimismPortal2 {
uint256(0), // value
uint64(SYSTEM_DEPOSIT_GAS_LIMIT), // gasLimit
false, // isCreation,
abi.encodeCall(L1BlockInterop.setConfig, (_type, _value))
abi.encodeCall(L1BlockIsthmus.setConfig, (_type, _value))
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { OptimismPortalInterop as OptimismPortal } from "src/L1/OptimismPortalIn
import { GasPayingToken } from "src/libraries/GasPayingToken.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { SystemConfig } from "src/L1/SystemConfig.sol";
import { ConfigType } from "src/L2/L1BlockInterop.sol";
import { ConfigType } from "src/L2/L1BlockIsthmus.sol";
import { StaticConfig } from "src/libraries/StaticConfig.sol";

/// @title SystemConfigInterop
Expand Down
18 changes: 16 additions & 2 deletions packages/contracts-bedrock/src/L2/CrossL2Inbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ interface IDependencySet {
function isInDependencySet(uint256 _chainId) external view returns (bool);
}

/// @title IL1Block
/// @notice Interface for L1Block with only `isDeposit()` method.
interface IL1Block {
/// @notice Returns whether the call was triggered from a a deposit or not.
/// @return True if the current call was triggered by a deposit transaction, and false otherwise.
function isDeposit() external view returns (bool);
}

/// @notice Thrown when a non-written transient storage slot is attempted to be read from.
error NotEntered();

Expand All @@ -29,6 +37,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 @@ -56,8 +67,8 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware {
bytes32 internal constant CHAINID_SLOT = 0x6e0446e8b5098b8c8193f964f1b567ec3a2bdaeba33d36acb85c1f1d3f92d313;

/// @notice Semantic version.
/// @custom:semver 1.0.0-beta.4
string public constant version = "1.0.0-beta.4";
/// @custom:semver 1.1.0-beta.4
string public constant version = "1.1.0-beta.4";

/// @notice Emitted when a cross chain message is being executed.
/// @param msgHash Hash of message payload being executed.
Expand Down Expand Up @@ -107,6 +118,9 @@ contract CrossL2Inbox is ICrossL2Inbox, ISemver, TransientReentrancyAware {
payable
reentrantAware
{
// We need to know if this is being called on a depositTx
if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isDeposit()) revert NoExecutingDeposits();

// Check the Identifier.
_checkIdentifier(_id);

Expand Down
Loading