Commit 2a2e1d2d authored by mergify[bot]'s avatar mergify[bot] Committed by GitHub

Merge branch 'develop' into fix/deposit-tx-gaslimit

parents 5dcb843b 2c892d5b
This diff is collapsed.
......@@ -100,11 +100,31 @@ func CheckBatch(cfg *rollup.Config, log log.Logger, l1Blocks []eth.L1BlockRef, l
return BatchDrop
}
// If we ran out of sequencer time drift, then we drop the batch and produce an empty batch instead,
// as the sequencer is not allowed to include anything past this point without moving to the next epoch.
// Check if we ran out of sequencer time drift
if max := batchOrigin.Time + cfg.MaxSequencerDrift; batch.Batch.Timestamp > max {
log.Warn("batch exceeded sequencer time drift, sequencer must adopt new L1 origin to include transactions again", "max_time", max)
return BatchDrop
if len(batch.Batch.Transactions) == 0 {
// If the sequencer is co-operating by producing an empty batch,
// then allow the batch if it was the right thing to do to maintain the L2 time >= L1 time invariant.
// We only check batches that do not advance the epoch, to ensure epoch advancement regardless of time drift is allowed.
if epoch.Number == batchOrigin.Number {
if len(l1Blocks) < 2 {
log.Info("without the next L1 origin we cannot determine yet if this empty batch that exceeds the time drift is still valid")
return BatchUndecided
}
nextOrigin := l1Blocks[1]
if batch.Batch.Timestamp >= nextOrigin.Time { // check if the next L1 origin could have been adopted
log.Info("batch exceeded sequencer time drift without adopting next origin, and next L1 origin would have been valid")
return BatchDrop
} else {
log.Info("continuing with empty batch before late L1 block to preserve L2 time invariant")
}
}
} else {
// If the sequencer is ignoring the time drift rule, then drop the batch and force an empty batch instead,
// as the sequencer is not allowed to include anything past this point without moving to the next epoch.
log.Warn("batch exceeded sequencer time drift, sequencer must adopt new L1 origin to include transactions again", "max_time", max)
return BatchDrop
}
}
// We can do this check earlier, but it's a more intensive one, so we do this last.
......
......@@ -151,6 +151,22 @@ func TestValidBatch(t *testing.T) {
SequenceNumber: 0,
}
l2A4 := eth.L2BlockRef{
Hash: testutils.RandomHash(rng),
Number: l2A3.Number + 1,
ParentHash: l2A3.Hash,
Time: l2A3.Time + conf.BlockTime, // 4*2 = 8, higher than seq time drift
L1Origin: l1A.ID(),
SequenceNumber: 4,
}
l1BLate := eth.L1BlockRef{
Hash: testutils.RandomHash(rng),
Number: l1A.Number + 1,
ParentHash: l1A.Hash,
Time: l2A4.Time + 1, // too late for l2A4 to adopt yet
}
testCases := []ValidBatchTestCase{
{
Name: "missing L1 info",
......@@ -249,16 +265,16 @@ func TestValidBatch(t *testing.T) {
Expected: BatchDrop,
},
{
Name: "epoch too old", // repeat of now outdated l2A3 data
Name: "epoch too old, but good parent hash and timestamp", // repeat of now outdated l2A3 data
L1Blocks: []eth.L1BlockRef{l1B, l1C, l1D},
L2SafeHead: l2B0, // we already moved on to B
Batch: BatchWithL1InclusionBlock{
L1InclusionBlock: l1C,
Batch: &BatchData{BatchV1{
ParentHash: l2A3.ParentHash,
ParentHash: l2B0.Hash, // build on top of safe head to continue
EpochNum: rollup.Epoch(l2A3.L1Origin.Number), // epoch A is no longer valid
EpochHash: l2A3.L1Origin.Hash,
Timestamp: l2A3.Time,
Timestamp: l2B0.Time + conf.BlockTime, // pass the timestamp check to get too epoch check
Transactions: nil,
}},
},
......@@ -313,23 +329,23 @@ func TestValidBatch(t *testing.T) {
Expected: BatchDrop,
},
{
Name: "sequencer time drift on same epoch",
Name: "sequencer time drift on same epoch with non-empty txs",
L1Blocks: []eth.L1BlockRef{l1A, l1B},
L2SafeHead: l2A3,
Batch: BatchWithL1InclusionBlock{
L1InclusionBlock: l1B,
Batch: &BatchData{BatchV1{ // we build l2A4, which has a timestamp of 2*4 = 8 higher than l2A0
ParentHash: l2A3.Hash,
EpochNum: rollup.Epoch(l2A3.L1Origin.Number),
EpochHash: l2A3.L1Origin.Hash,
Timestamp: l2A3.Time + conf.BlockTime,
Transactions: nil,
ParentHash: l2A4.ParentHash,
EpochNum: rollup.Epoch(l2A4.L1Origin.Number),
EpochHash: l2A4.L1Origin.Hash,
Timestamp: l2A4.Time,
Transactions: []hexutil.Bytes{[]byte("sequencer should not include this tx")},
}},
},
Expected: BatchDrop,
},
{
Name: "sequencer time drift on changing epoch",
Name: "sequencer time drift on changing epoch with non-empty txs",
L1Blocks: []eth.L1BlockRef{l1X, l1Y, l1Z},
L2SafeHead: l2X0,
Batch: BatchWithL1InclusionBlock{
......@@ -339,11 +355,75 @@ func TestValidBatch(t *testing.T) {
EpochNum: rollup.Epoch(l2Y0.L1Origin.Number),
EpochHash: l2Y0.L1Origin.Hash,
Timestamp: l2Y0.Time, // valid, but more than 6 ahead of l1Y.Time
Transactions: nil,
Transactions: []hexutil.Bytes{[]byte("sequencer should not include this tx")},
}},
},
Expected: BatchDrop,
},
{
Name: "sequencer time drift on same epoch with empty txs and late next epoch",
L1Blocks: []eth.L1BlockRef{l1A, l1BLate},
L2SafeHead: l2A3,
Batch: BatchWithL1InclusionBlock{
L1InclusionBlock: l1BLate,
Batch: &BatchData{BatchV1{ // l2A4 time < l1BLate time, so we cannot adopt origin B yet
ParentHash: l2A4.ParentHash,
EpochNum: rollup.Epoch(l2A4.L1Origin.Number),
EpochHash: l2A4.L1Origin.Hash,
Timestamp: l2A4.Time,
Transactions: nil,
}},
},
Expected: BatchAccept, // accepted because empty & preserving L2 time invariant
},
{
Name: "sequencer time drift on changing epoch with empty txs",
L1Blocks: []eth.L1BlockRef{l1X, l1Y, l1Z},
L2SafeHead: l2X0,
Batch: BatchWithL1InclusionBlock{
L1InclusionBlock: l1Z,
Batch: &BatchData{BatchV1{
ParentHash: l2Y0.ParentHash,
EpochNum: rollup.Epoch(l2Y0.L1Origin.Number),
EpochHash: l2Y0.L1Origin.Hash,
Timestamp: l2Y0.Time, // valid, but more than 6 ahead of l1Y.Time
Transactions: nil,
}},
},
Expected: BatchAccept, // accepted because empty & still advancing epoch
},
{
Name: "sequencer time drift on same epoch with empty txs and no next epoch in sight yet",
L1Blocks: []eth.L1BlockRef{l1A},
L2SafeHead: l2A3,
Batch: BatchWithL1InclusionBlock{
L1InclusionBlock: l1B,
Batch: &BatchData{BatchV1{ // we build l2A4, which has a timestamp of 2*4 = 8 higher than l2A0
ParentHash: l2A4.ParentHash,
EpochNum: rollup.Epoch(l2A4.L1Origin.Number),
EpochHash: l2A4.L1Origin.Hash,
Timestamp: l2A4.Time,
Transactions: nil,
}},
},
Expected: BatchUndecided, // we have to wait till the next epoch is in sight to check the time
},
{
Name: "sequencer time drift on same epoch with empty txs and but in-sight epoch that invalidates it",
L1Blocks: []eth.L1BlockRef{l1A, l1B, l1C},
L2SafeHead: l2A3,
Batch: BatchWithL1InclusionBlock{
L1InclusionBlock: l1C,
Batch: &BatchData{BatchV1{ // we build l2A4, which has a timestamp of 2*4 = 8 higher than l2A0
ParentHash: l2A4.ParentHash,
EpochNum: rollup.Epoch(l2A4.L1Origin.Number),
EpochHash: l2A4.L1Origin.Hash,
Timestamp: l2A4.Time,
Transactions: nil,
}},
},
Expected: BatchDrop, // dropped because it could have advanced the epoch to B
},
{
Name: "empty tx included",
L1Blocks: []eth.L1BlockRef{l1A, l1B},
......
......@@ -167,20 +167,21 @@ L2OutputOracleUpgradeable_Test:test_initValuesOnProxy_succeeds() (gas: 26093)
L2OutputOracleUpgradeable_Test:test_initializeImpl_alreadyInitialized_reverts() (gas: 15149)
L2OutputOracleUpgradeable_Test:test_initializeProxy_alreadyInitialized_reverts() (gas: 20131)
L2OutputOracleUpgradeable_Test:test_upgrading_succeeds() (gas: 180413)
L2StandardBridge_BridgeERC20To_Test:test_bridgeERC20To_succeeds() (gas: 387796)
L2StandardBridge_BridgeERC20To_Test:test_withdrawTo_withdrawingERC20_succeeds() (gas: 388044)
L2StandardBridge_BridgeERC20To_Test:test_bridgeERC20To_succeeds() (gas: 387797)
L2StandardBridge_BridgeERC20To_Test:test_withdrawTo_withdrawingERC20_succeeds() (gas: 388045)
L2StandardBridge_BridgeERC20_Test:test_bridgeERC20_succeeds() (gas: 383520)
L2StandardBridge_BridgeERC20_Test:test_withdraw_notEOA_reverts() (gas: 251687)
L2StandardBridge_BridgeERC20_Test:test_withdraw_withdrawingERC20_succeeds() (gas: 383722)
L2StandardBridge_BridgeERC20_Test:test_withdraw_withdrawingERC20_succeeds() (gas: 383723)
L2StandardBridge_Bridge_Test:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 23798)
L2StandardBridge_Bridge_Test:test_finalizeBridgeETH_sendToMessenger_reverts() (gas: 23960)
L2StandardBridge_Bridge_Test:test_finalizeBridgeETH_sendToSelf_reverts() (gas: 23848)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingERC20_succeeds() (gas: 91011)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingETH_succeeds() (gas: 89843)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingERC20_succeeds() (gas: 91013)
L2StandardBridge_Bridge_Test:test_finalizeDeposit_depositingETH_succeeds() (gas: 89845)
L2StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 43155)
L2StandardBridge_Test:test_initialize_succeeds() (gas: 24247)
L2StandardBridge_Test:test_receive_succeeds() (gas: 177167)
L2StandardBridge_Test:test_withdraw_insufficientValue_reverts() (gas: 19637)
L2StandardBridge_Test:test_receive_succeeds() (gas: 173990)
L2StandardBridge_Test:test_withdraw_ether_succeeds() (gas: 140500)
L2StandardBridge_Test:test_withdraw_insufficientValue_reverts() (gas: 16463)
L2ToL1MessagePasserTest:test_burn_succeeds() (gas: 112572)
L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract_succeeds() (gas: 70423)
L2ToL1MessagePasserTest:test_initiateWithdrawal_fromEOA_succeeds() (gas: 75874)
......@@ -393,7 +394,7 @@ SequencerFeeVault_Test:test_constructor_succeeds() (gas: 5504)
SequencerFeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 5420)
SequencerFeeVault_Test:test_receive_succeeds() (gas: 17336)
SequencerFeeVault_Test:test_withdraw_notEnough_reverts() (gas: 9309)
SequencerFeeVault_Test:test_withdraw_succeeds() (gas: 163169)
SequencerFeeVault_Test:test_withdraw_succeeds() (gas: 163184)
SystemConfig_Initialize_TestFail:test_initialize_lowGasLimit_reverts() (gas: 61966)
SystemConfig_Setters_TestFail:test_setBatcherHash_notOwner_reverts() (gas: 10545)
SystemConfig_Setters_TestFail:test_setGasConfig_notOwner_reverts() (gas: 10532)
......
......@@ -85,6 +85,8 @@ contract L2StandardBridge is StandardBridge, Semver {
/**
* @custom:legacy
* @notice Initiates a withdrawal from L2 to L1.
* This function only works with OptimismMintableERC20 tokens or ether. Use the
* `bridgeERC20` function to bridge native L2 tokens to L1.
*
* @param _l2Token Address of the L2 token to withdraw.
* @param _amount Amount of the L2 token to withdraw.
......@@ -107,6 +109,8 @@ contract L2StandardBridge is StandardBridge, Semver {
* be locked in the L1StandardBridge. ETH may be recoverable if the call can be
* successfully replayed by increasing the amount of gas supplied to the call. If the
* call will fail for any amount of gas, then the ETH will be locked permanently.
* This function only works with OptimismMintableERC20 tokens or ether. Use the
* `bridgeERC20To` function to bridge native L2 tokens to L1.
*
* @param _l2Token Address of the L2 token to withdraw.
* @param _to Recipient account on L1.
......@@ -126,7 +130,8 @@ contract L2StandardBridge is StandardBridge, Semver {
/**
* @custom:legacy
* @notice Finalizes a deposit from L1 to L2.
* @notice Finalizes a deposit from L1 to L2. To finalize a deposit of ether, use address(0)
* and the l1Token and the Legacy ERC20 ether predeploy address as the l2Token.
*
* @param _l1Token Address of the L1 token to deposit.
* @param _l2Token Address of the corresponding L2 token.
......@@ -179,10 +184,10 @@ contract L2StandardBridge is StandardBridge, Semver {
uint32 _minGasLimit,
bytes memory _extraData
) internal {
address l1Token = OptimismMintableERC20(_l2Token).l1Token();
if (_l2Token == Predeploys.LEGACY_ERC20_ETH) {
_initiateBridgeETH(_from, _to, _amount, _minGasLimit, _extraData);
} else {
address l1Token = OptimismMintableERC20(_l2Token).l1Token();
_initiateBridgeERC20(_l2Token, l1Token, _from, _to, _amount, _minGasLimit, _extraData);
}
}
......
......@@ -19,6 +19,9 @@ contract FeeVault_Test is Bridge_Initializer {
super.setUp();
vm.etch(Predeploys.BASE_FEE_VAULT, address(new BaseFeeVault(recipient)).code);
vm.etch(Predeploys.L1_FEE_VAULT, address(new L1FeeVault(recipient)).code);
vm.label(Predeploys.BASE_FEE_VAULT, "BaseFeeVault");
vm.label(Predeploys.L1_FEE_VAULT, "L1FeeVault");
}
function test_constructor_succeeds() external {
......
......@@ -118,6 +118,38 @@ contract L2StandardBridge_Test is Bridge_Initializer {
vm.prank(alice, alice);
L2Bridge.withdraw(address(Predeploys.LEGACY_ERC20_ETH), 100, 1000, hex"");
}
/**
* @notice Use the legacy `withdraw` interface on the L2StandardBridge to
* withdraw ether from L2 to L1.
*/
function test_withdraw_ether_succeeds() external {
assertTrue(alice.balance >= 100);
assertEq(Predeploys.L2_TO_L1_MESSAGE_PASSER.balance, 0);
vm.expectEmit(true, true, true, true, address(L2Bridge));
emit WithdrawalInitiated({
l1Token: address(0),
l2Token: Predeploys.LEGACY_ERC20_ETH,
from: alice,
to: alice,
amount: 100,
data: hex""
});
vm.expectEmit(true, true, true, true, address(L2Bridge));
emit ETHBridgeInitiated({ from: alice, to: alice, amount: 100, data: hex"" });
vm.prank(alice, alice);
L2Bridge.withdraw{ value: 100 }({
_l2Token: Predeploys.LEGACY_ERC20_ETH,
_amount: 100,
_minGasLimit: 1000,
_extraData: hex""
});
assertEq(Predeploys.L2_TO_L1_MESSAGE_PASSER.balance, 100);
}
}
contract PreBridgeERC20 is Bridge_Initializer {
......
......@@ -16,6 +16,7 @@ contract SequencerFeeVault_Test is Bridge_Initializer {
function setUp() public override {
super.setUp();
vm.etch(Predeploys.SEQUENCER_FEE_WALLET, address(new SequencerFeeVault(recipient)).code);
vm.label(Predeploys.SEQUENCER_FEE_WALLET, "SequencerFeeVault");
}
function test_minWithdrawalAmount_succeeds() external {
......@@ -52,7 +53,7 @@ contract SequencerFeeVault_Test is Bridge_Initializer {
// No ether has been withdrawn yet
assertEq(vault.totalProcessed(), 0);
vm.expectEmit(true, true, true, true);
vm.expectEmit(true, true, true, true, address(Predeploys.SEQUENCER_FEE_WALLET));
emit Withdrawal(address(vault).balance, vault.RECIPIENT(), address(this));
// The entire vault's balance is withdrawn
......
......@@ -146,6 +146,12 @@ The rationale is to maintain liveness in case of either a skipped slot on L1, or
which requires longer epochs.
Shorter epochs are then required to avoid L2 timestamps drifting further and further ahead of L1.
Note that `min_l2_timestamp + l2_block_time` ensures that a new L2 batch can always be processed, even if the
`max_sequencer_drift` is exceeded. However, when exceeding the `max_sequencer_drift`, progression to the next L1 origin
is enforced, with an exception to ensure the minimum timestamp bound (based on this next L1 origin) can be met in the
next L2 batch, and `len(batch.transactions) == 0` continues to be enforced while the `max_sequencer_drift` is exceeded.
See [Batch Queue] for more details.
## Eager Block Derivation
In practice, it is often not necessary to wait for a full sequencing window of L1 blocks in order to start deriving the
......@@ -598,10 +604,18 @@ Rules, in validation order:
- `batch.epoch_num > epoch.number+1` -> `drop`: i.e. the L1 origin cannot change by more than one L1 block per L2 block.
- `batch.epoch_hash != batch_origin.hash` -> `drop`: i.e. a batch must reference a canonical L1 origin,
to prevent batches from being replayed onto unexpected L1 chains.
- `batch.timestamp > batch_origin.time + max_sequencer_drift` -> `drop`: i.e. a batch that does not adopt the next L1
within time will be dropped, in favor of an empty batch that can advance the L1 origin. This enforces the max L2
timestamp rule.
- `batch.timestamp < batch_origin.time` -> `drop`: enforce the min L2 timestamp rule.
- `batch.timestamp > batch_origin.time + max_sequencer_drift`: enforce the L2 timestamp drift rule,
but with exceptions to preserve above min L2 timestamp invariant:
- `len(batch.transactions) == 0`:
- `epoch.number == batch.epoch_num`:
this implies the batch does not already advance the L1 origin, and must thus be checked against `next_epoch`.
- If `next_epoch` is not known -> `undecided`:
without the next L1 origin we cannot yet determine if time invariant could have been kept.
- If `batch.timestamp >= next_epoch.time` -> `drop`:
the batch could have adopted the next L1 origin without breaking the `L2 time >= L1 time` invariant.
- `len(batch.transactions) > 0`: -> `drop`:
when exceeding the sequencer time drift, never allow the sequencer to include transactions.
- `batch.transactions`: `drop` if the `batch.transactions` list contains a transaction
that is invalid or derived by other means exclusively:
- any transaction that is empty (zero length byte string)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment