Set of fixes to the dev branch #311
Conversation
| @@ -38,7 +38,7 @@ address constant MULTIPLICATION_HIGH_ADDRESS = address((1 << 16) - 26); | |||
| address constant GET_EXTRA_ABI_DATA_ADDRESS = address((1 << 16) - 27); | |||
|
|
|||
| // All the offsets are in bits | |||
| uint256 constant META_GAS_PER_PUBDATA_BYTE_OFFSET = 0 * 8; | |||
| uint256 constant META_PUBDATA_PUBLISHED_OFFSET = 0 * 8; | |||
There was a problem hiding this comment.
Please ensure that this change is clearly indicated in the docs as a warning.
There was a problem hiding this comment.
The in-code documentation for a lot of functions is missing. It'll be better if we could address them.
| @@ -224,9 +224,9 @@ library SystemContractHelper { | |||
| /// that a single byte sent to L1 as pubdata costs. | |||
There was a problem hiding this comment.
Given the changes made in the return field of the natspec comments, the @notice field looks incorrect to me.
There was a problem hiding this comment.
Fixed in this PR: #351
| @@ -67,7 +67,7 @@ contract L2BaseToken is IBaseToken, ISystemContract { | |||
| emit Mint(_account, _amount); | |||
| } | |||
|
|
|||
| /// @notice Initiate the ETH withdrawal, funds will be available to claim on L1 `finalizeEthWithdrawal` method. | |||
| /// @notice Initiate the withdrawal of the base token, funds will be available to claim on L1 `finalizeEthWithdrawal` method. | |||
There was a problem hiding this comment.
The ETH here has been changed to base token and that makes sense. Should we also consider changing the name of finalizeEthWithdrawal since it's no longer necessarily ETH?
There was a problem hiding this comment.
Unfortunately no, since we need the old SDKs to be backward-compatible, this function is also only available for the Era hyperchain
What ❔
Set of fixes to the dev branch:
Why ❔
Checklist