Conversation
…s into kl/17-10-25/merge-draft
Kl/17 10 25/merge draft
…contracts into kl/14-10-25/remove-timeframe-assumption
Signed-off-by: Danil <deniallugo@gmail.com> # Conflicts: # AllContractsHashes.json # l1-contracts/deploy-scripts/DeployCTM.s.sol # l1-contracts/deploy-scripts/RegisterZKChain.s.sol # l1-contracts/deploy-scripts/upgrade/DefaultEcosystemUpgrade.s.sol # l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade_v26_1.s.sol # l1-contracts/deploy-scripts/upgrade/EcosystemUpgrade_v28.s.sol
Signed-off-by: Danil <deniallugo@gmail.com> # Conflicts: # l1-contracts/deploy-scripts/DeployL2Contracts.sol # l1-contracts/deploy-scripts/gateway/GatewayCTMFromL1.s.sol # l1-contracts/deploy-scripts/gateway/GatewayGovernanceUtils.s.sol
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
Signed-off-by: Danil <deniallugo@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1ce9d4c35
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// @dev FIXME: for the demo purposes, remove onlyCallFromBootloader modifier | ||
| function addInteropRoot(uint256 chainId, uint256 blockOrBatchNumber, bytes32[] calldata sides) external { | ||
| // In the current code sides should only contain the Interop Root itself, as mentioned above. |
There was a problem hiding this comment.
Restrict interop root writes to trusted caller
Because addInteropRoot is declared external with no access control, any address can set the interop root for an arbitrary (chainId, blockOrBatchNumber) once. L2MessageVerification later treats L2_INTEROP_ROOT_STORAGE.interopRoots(...) as the source of truth for message inclusion, so an attacker who pre-fills a root can craft proofs that verify forged bundles/messages on L2. This is exploitable whenever a malicious party can submit addInteropRoot before the legitimate publisher for that pair.
Useful? React with 👍 / 👎.
| // FIXME: does not burn, just sends to a a bad address. | ||
| // slither-disable-next-line arbitrary-send-eth | ||
| L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue{value: _totalBurnedCallsValue}(); | ||
| address(0xdeadbeef).call{value: _totalBurnedCallsValue}(""); |
There was a problem hiding this comment.
Burn msg.value instead of transferring to 0xdeadbeef
In _ensureCorrectTotalValue, the burn path for same-base-token interop bundles now transfers _totalBurnedCallsValue to 0xdeadbeef instead of calling L2_BASE_TOKEN_SYSTEM_CONTRACT.burnMsgValue. That does not update the base-token system contract’s accounting, so bundles that are supposed to burn value will leave the L2 supply/state out of sync with L1 accounting. This only affects bundles where destinationChainBaseTokenAssetId == thisChainBaseTokenAssetId and _totalBurnedCallsValue > 0, but in that case reconciliation/withdrawals can break because no actual burn is recorded.
Useful? React with 👍 / 👎.
What ❔
Why ❔
Checklist