Commit a61265ff authored by Maurelian's avatar Maurelian Committed by GitHub

Various improvements to documentation in the smart contracts and deposit spec (#3440)

* fix(ctb): M-03 Eth can be send to undeliverable recipient

* fix(ctb): M-07  CrossDomainMessenger allows sending messages to unrelayable addresses"

* fix(ctb): L-01 Improve deposit docs

* fix(ctb): L-09 Misleading inline documentation

* fix(ctb):  Standard Bridge does not support tokens with transfer fees

* fix(ctb): N-07 Unexplained literal values
parent fd15f789
......@@ -9,8 +9,13 @@ import { Semver } from "../universal/Semver.sol";
* @custom:proxied
* @title L1StandardBridge
* @notice The L1StandardBridge is responsible for transfering ETH and ERC20 tokens between L1 and
* L2. ERC20 tokens deposited into L2 are escrowed within this contract until withdrawal.
* L2. In the case that an ERC20 token is native to L1, it will be escrowed within this
* contract. If the ERC20 token is native to L2, it will be burnt.
* ETH is transferred to and escrowed within the OptimismPortal contract.
* Note that this contract is not intended to support all variations of ERC20 tokens.
* Examples of some token types that may not be properly supported by this contract include,
* but are not limited to: tokens with transfer fees, rebasing tokens, and
* tokens with blocklists.
*/
contract L1StandardBridge is StandardBridge, Semver {
/**
......
......@@ -306,7 +306,8 @@ contract L2OutputOracle is OwnableUpgradeable, Semver {
/**
* @notice Returns the L2 timestamp corresponding to a given L2 block number.
* Returns a null output proposal if none is found.
* If the L2 block number provided is between checkpoints, this function will return the
* timestamp of the previous checkpoint.
*
* @param _l2BlockNumber The L2 block number of the target block.
*/
......
......@@ -11,7 +11,8 @@ import { OptimismMintableERC20 } from "../universal/OptimismMintableERC20.sol";
* @custom:predeploy 0x4200000000000000000000000000000000000010
* @title L2StandardBridge
* @notice The L2StandardBridge is responsible for transfering ETH and ERC20 tokens between L1 and
* L2. ERC20 tokens sent to L1 are escrowed within this contract.
* L2. In the case that an ERC20 token is native to L2, it will be escrowed within this
* contract. If the ERC20 token is native to L1, it will be burnt.
* Note that this contract is not intended to support all variations of ERC20 tokens.
* Examples of some token types that may not be properly supported by this contract include,
* but are not limited to: tokens with transfer fees, rebasing tokens, and
......
......@@ -12,9 +12,8 @@ import { RLPWriter } from "./rlp/RLPWriter.sol";
library Encoding {
/**
* @notice RLP encodes the L2 transaction that would be generated when a given deposit is sent
* to the L2 system. Useful for searching for a deposit in the L2 system.
* This currently only supports user deposits and not system
* transactions.
* to the L2 system. Useful for searching for a deposit in the L2 system. The
* transaction is prefixed with 0x7e to identify its EIP-2718 type.
*
* @param _tx User deposit transaction to encode.
*
......
......@@ -107,9 +107,8 @@ abstract contract CrossDomainMessenger is
/**
* @notice Mapping of message hashes to boolean receipt values. Note that a message will only
* be present in this mapping if it failed to be relayed on this chain at least once.
* If a message is successfully relayed on the first attempt, then it will only be
* present within the successfulMessages mapping.
* be present in this mapping if it has successfully been relayed on this chain, and
* can therefore not be relayed again.
*/
mapping(bytes32 => bool) public successfulMessages;
......@@ -200,7 +199,10 @@ abstract contract CrossDomainMessenger is
}
/**
* @notice Sends a message to some target address on the other chain.
* @notice Sends a message to some target address on the other chain. Note that if the call
* always reverts, then the message will be unrelayable, and any ETH sent will be
* permanently locked. The same will occur if the target on the other chain is
* considered unsafe (see the _isUnsafeTarget() function).
*
* @param _target Target contract or wallet address.
* @param _message Message to trigger the target address with.
......
......@@ -240,6 +240,7 @@ contract ProxyAdmin is Owned {
Proxy(_proxy).upgradeTo(_implementation);
} else if (ptype == ProxyType.CHUGSPLASH) {
L1ChugSplashProxy(_proxy).setStorage(
// bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1)
0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc,
bytes32(uint256(uint160(_implementation)))
);
......
......@@ -204,7 +204,9 @@ abstract contract StandardBridge {
* smart contract and the call fails, the ETH will be temporarily locked in the
* StandardBridge on the other chain until the call is replayed. If the call cannot be
* replayed with any amount of gas (call always reverts), then the ETH will be
* permanently locked in the StandardBridge on the other chain.
* permanently locked in the StandardBridge on the other chain. ETH will also
* be locked if the receiver is the other bridge, because finalizeBridgeETH will revert
* in that case.
*
* @param _to Address of the receiver.
* @param _minGasLimit Minimum amount of gas that the bridge can be relayed with.
......
......@@ -151,6 +151,13 @@ account will be increased, but no other state transition will occur.
If `isSystemTransaction` in the deposit is set to `true`, the gas used by the deposit is unmetered.
It must not be subtracted from the gas pool and the `usedGas` field of the receipt must be set to 0.
Note for application developers: because `CALLER` and `ORIGIN` are set to `from`, the
semantics of using the `tx.origin == msg.sender` check will not work to determine whether
or not a caller is an EOA during a deposit transaction. Instead, the check could only be useful for
identifying the first call in the L2 deposit transaction. However this check does still satisfy
the common case in which developers are using this check to ensure that the `CALLER` is unable to
execute code before and after the call.
#### Nonce Handling
Despite the lack of signature validation, we still increment the nonce of the `from` account when a
......
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