Skip to content

N
nebula
Commit 251af5e4 authored by Mark Tyneway's avatar Mark Tyneway Committed by GitHub
Browse files
Options

contracts-bedrock: deprecate `finalizeDeposit` (#10459) 

The [finalizeDeposit](https://github.com/ethereum-optimism/optimism/blob/b9eb669aa5dfc36204ce2167da2e5ab8bbde61de/packages/contracts-bedrock/src/L2/L2StandardBridge.sol#L147)
function is left over from the legacy OVM style standard bridge.
It is impossible to be called as the bridge has moved to the modern interface which is based on `finalizeBridgeETH` or `finalizeBridgeERC20`.

Methods actually used in bridge:

- [finalizeBridgeETH](https://github.com/ethereum-optimism/optimism/blob/b9eb669aa5dfc36204ce2167da2e5ab8bbde61de/packages/contracts-bedrock/src/universal/StandardBridge.sol#L333)
- [finalizeBridgeERC20](https://github.com/ethereum-optimism/optimism/blob/b9eb669aa5dfc36204ce2167da2e5ab8bbde61de/packages/contracts-bedrock/src/universal/StandardBridge.sol#L379)

Proof that finalizeDeposit is not used anywhere. Its defined in the L2StandardBridge and otherwise only in tests.

```
git grep -rin finalizeDeposit
snapshots/abi/L2StandardBridge.json:272:    "name": "finalizeDeposit",
src/L2/L2StandardBridge.sol:147:    function finalizeDeposit(
test/L2/L2StandardBridge.t.sol:523:    /// @dev Tests that `finalizeDeposit` succeeds. It should:
test/L2/L2StandardBridge.t.sol:527:    function test_finalizeDeposit_depositingERC20_succeeds() external {
test/L2/L2StandardBridge.t.sol:544:        l2StandardBridge.finalizeDeposit(address(L1Token), address(L2Token), alice, alice, 100, hex"");
test/L2/L2StandardBridge.t.sol:547:    /// @dev Tests that `finalizeDeposit` succeeds when depositing ERC20 with custom gas token.
test/L2/L2StandardBridge.t.sol:548:    function test_finalizeDeposit_depositingERC20_customGasToken_reverts() external {
test/L2/L2StandardBridge.t.sol:559:        l2StandardBridge.finalizeDeposit(address(L1Token), address(L2Token), alice, alice, 100, hex"");
test/L2/L2StandardBridge.t.sol:562:    /// @dev Tests that `finalizeDeposit` succeeds when depositing ETH.
test/L2/L2StandardBridge.t.sol:563:    function test_finalizeDeposit_depositingETH_succeeds() external {
test/L2/L2StandardBridge.t.sol:579:        l2StandardBridge.finalizeDeposit{ value: 100 }(
test/L2/L2StandardBridge.t.sol:584:    /// @dev Tests that `finalizeDeposit` reverts when depositing ETH with custom gas token.
test/L2/L2StandardBridge.t.sol:585:    function test_finalizeDeposit_depositingETH_customGasToken_reverts() external {
test/L2/L2StandardBridge.t.sol:594:        l2StandardBridge.finalizeDeposit(address(0), Predeploys.LEGACY_ERC20_ETH, alice, alice, 100, hex"");
test/L2/L2StandardBridge.t.sol:597:    /// @dev Tests that `finalizeDeposit` reverts if the amounts do not match.
test/L2/L2StandardBridge.t.sol:610:    /// @dev Tests that `finalizeDeposit` reverts if the receipient is the other bridge.
test/L2/L2StandardBridge.t.sol:623:    /// @dev Tests that `finalizeDeposit` reverts if the receipient is the messenger.
```

There cannot be third party integrations since it is `onlyBridge`,
meaning only the `L1StandardBridge` can call it.

This commit removes the tests and the function from the
`L2StandardBridge`. This is part of refactoring as we go,
ensuring that the code stays clean. Tend the garden.
parent 93e9d153
Hide whitespace changes
Inline Side-by-side
Showing with 6 additions and 161 deletions
packages/contracts-bedrock/semver-lock.json
View file @ 251af5e4
......@@ -84,8 +84,8 @@
"sourceCodeHash": "0xa7646a588275046f92525ef121e5a0fe149e7752ea51fe62f7e0686a21153542"
},
"src/L2/L2StandardBridge.sol": {
"initCodeHash": "0x4d7e23702aa5627f78842b2b06e0ee0eec1ea63cf6552b652f4c8bdb56e1c8f4",
"sourceCodeHash": "0x55a865c85406b3e1414ab4e9f03a902855300272321b4eebcdfdc284e51fb2df"
"initCodeHash": "0xdfe717975f7c76e80201f715a0d68f3fef02beca6a9e0f378a115e175c6a5944",
"sourceCodeHash": "0xb4a9f333f04008f610eb55fa6ff7e610bab340d53156cb50ec65a575c9576b0e"
},
"src/L2/L2ToL1MessagePasser.sol": {
"initCodeHash": "0x08bbede75cd6dfd076903b8f04d24f82fa7881576c135825098778632e37eebc",
......
packages/contracts-bedrock/snapshots/abi/L2StandardBridge.json
View file @ 251af5e4
......@@ -236,44 +236,6 @@
"stateMutability": "payable",
"type": "function"
},
{
"inputs": [
{
"internalType": "address",
"name": "_l1Token",
"type": "address"
},
{
"internalType": "address",
"name": "_l2Token",
"type": "address"
},
{
"internalType": "address",
"name": "_from",
"type": "address"
},
{
"internalType": "address",
"name": "_to",
"type": "address"
},
{
"internalType": "uint256",
"name": "_amount",
"type": "uint256"
},
{
"internalType": "bytes",
"name": "_extraData",
"type": "bytes"
}
],
"name": "finalizeDeposit",
"outputs": [],
"stateMutability": "payable",
"type": "function"
},
{
"inputs": [
{
......
packages/contracts-bedrock/src/L2/L2StandardBridge.sol
View file @ 251af5e4
......@@ -52,8 +52,8 @@ contract L2StandardBridge is StandardBridge, ISemver {
bytes extraData
);
/// @custom:semver 1.9.0
string public constant version = "1.9.0";
/// @custom:semver 1.10.0
string public constant version = "1.10.0";
/// @notice Constructs the L2StandardBridge contract.
constructor() StandardBridge() {
......@@ -134,36 +134,6 @@ contract L2StandardBridge is StandardBridge, ISemver {
_initiateWithdrawal(_l2Token, msg.sender, _to, _amount, _minGasLimit, _extraData);
}
/// @custom:legacy
/// @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.
/// Subject to be deprecated in the future.
/// @param _l1Token Address of the L1 token to deposit.
/// @param _l2Token Address of the corresponding L2 token.
/// @param _from Address of the depositor.
/// @param _to Address of the recipient.
/// @param _amount Amount of the tokens being deposited.
/// @param _extraData Extra data attached to the deposit.
function finalizeDeposit(
address _l1Token,
address _l2Token,
address _from,
address _to,
uint256 _amount,
bytes calldata _extraData
)
external
payable
virtual
{
require(isCustomGasToken() == false, "L2StandardBridge: not supported with custom gas token");
if (_l1Token == address(0) && _l2Token == Predeploys.LEGACY_ERC20_ETH) {
finalizeBridgeETH(_from, _to, _amount, _extraData);
} else {
finalizeBridgeERC20(_l2Token, _l1Token, _from, _to, _amount, _extraData);
}
}
/// @custom:legacy
/// @notice Retrieves the access of the corresponding L1 bridge contract.
/// @return Address of the corresponding L1 bridge contract.
......
packages/contracts-bedrock/test/L2/L2StandardBridge.t.sol
View file @ 251af5e4
......@@ -520,94 +520,7 @@ contract L2StandardBridge_BridgeERC20To_Test is PreBridgeERC20To {
}
contract L2StandardBridge_Bridge_Test is Bridge_Initializer {
/// @dev Tests that `finalizeDeposit` succeeds. It should:
/// - only be callable by the l1TokenBridge
/// - emit `DepositFinalized` if the token pair is supported
/// - call `Withdrawer.initiateWithdrawal` if the token pair is not supported
function test_finalizeDeposit_depositingERC20_succeeds() external {
vm.mockCall(
address(l2StandardBridge.messenger()),
abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
abi.encode(address(l2StandardBridge.OTHER_BRIDGE()))
);
vm.expectCall(address(L2Token), abi.encodeWithSelector(OptimismMintableERC20.mint.selector, alice, 100));
// Should emit both the bedrock and legacy events
vm.expectEmit(address(l2StandardBridge));
emit DepositFinalized(address(L1Token), address(L2Token), alice, alice, 100, hex"");
vm.expectEmit(address(l2StandardBridge));
emit ERC20BridgeFinalized(address(L2Token), address(L1Token), alice, alice, 100, hex"");
vm.prank(address(l2CrossDomainMessenger));
l2StandardBridge.finalizeDeposit(address(L1Token), address(L2Token), alice, alice, 100, hex"");
}
/// @dev Tests that `finalizeDeposit` succeeds when depositing ERC20 with custom gas token.
function test_finalizeDeposit_depositingERC20_customGasToken_reverts() external {
vm.mockCall(address(l1Block), abi.encodeWithSignature("gasPayingToken()"), abi.encode(address(1), uint8(18)));
vm.mockCall(
address(l2StandardBridge.messenger()),
abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
abi.encode(address(l2StandardBridge.OTHER_BRIDGE()))
);
vm.expectRevert("L2StandardBridge: not supported with custom gas token");
vm.prank(address(l2CrossDomainMessenger));
l2StandardBridge.finalizeDeposit(address(L1Token), address(L2Token), alice, alice, 100, hex"");
}
/// @dev Tests that `finalizeDeposit` succeeds when depositing ETH.
function test_finalizeDeposit_depositingETH_succeeds() external {
vm.mockCall(
address(l2StandardBridge.messenger()),
abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
abi.encode(address(l2StandardBridge.OTHER_BRIDGE()))
);
// Should emit both the bedrock and legacy events
vm.expectEmit(address(l2StandardBridge));
emit DepositFinalized(address(0), Predeploys.LEGACY_ERC20_ETH, alice, alice, 100, hex"");
vm.expectEmit(address(l2StandardBridge));
emit ETHBridgeFinalized(alice, alice, 100, hex"");
vm.deal(address(l2CrossDomainMessenger), 100);
vm.prank(address(l2CrossDomainMessenger));
l2StandardBridge.finalizeDeposit{ value: 100 }(
address(0), Predeploys.LEGACY_ERC20_ETH, alice, alice, 100, hex""
);
}
/// @dev Tests that `finalizeDeposit` reverts when depositing ETH with custom gas token.
function test_finalizeDeposit_depositingETH_customGasToken_reverts() external {
vm.mockCall(address(l1Block), abi.encodeWithSignature("gasPayingToken()"), abi.encode(address(1), uint8(2)));
vm.mockCall(
address(l2StandardBridge.messenger()),
abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
abi.encode(address(l2StandardBridge.OTHER_BRIDGE()))
);
vm.prank(address(l2CrossDomainMessenger));
vm.expectRevert("L2StandardBridge: not supported with custom gas token");
l2StandardBridge.finalizeDeposit(address(0), Predeploys.LEGACY_ERC20_ETH, alice, alice, 100, hex"");
}
/// @dev Tests that `finalizeDeposit` reverts if the amounts do not match.
function test_finalizeBridgeETH_incorrectValue_reverts() external {
vm.mockCall(
address(l2StandardBridge.messenger()),
abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
abi.encode(address(l2StandardBridge.OTHER_BRIDGE()))
);
vm.deal(address(l2CrossDomainMessenger), 100);
vm.prank(address(l2CrossDomainMessenger));
vm.expectRevert("StandardBridge: amount sent does not match amount required");
l2StandardBridge.finalizeBridgeETH{ value: 50 }(alice, alice, 100, hex"");
}
/// @dev Tests that `finalizeDeposit` reverts if the receipient is the other bridge.
/// @dev Tests that `finalizeBridgeETH` reverts if the receipient is the other bridge.
function test_finalizeBridgeETH_sendToSelf_reverts() external {
vm.mockCall(
address(l2StandardBridge.messenger()),
......@@ -620,7 +533,7 @@ contract L2StandardBridge_Bridge_Test is Bridge_Initializer {
l2StandardBridge.finalizeBridgeETH{ value: 100 }(alice, address(l2StandardBridge), 100, hex"");
}
/// @dev Tests that `finalizeDeposit` reverts if the receipient is the messenger.
/// @dev Tests that `finalizeBridgeETH` reverts if the receipient is the messenger.
function test_finalizeBridgeETH_sendToMessenger_reverts() external {
vm.mockCall(
address(l2StandardBridge.messenger()),
......
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