Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3b500eb7d
ℹ️ 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".
| function initialize(uint256 _l1ChainId, bytes32 _baseTokenAssetId, address _ntv) external { | ||
| L1_CHAIN_ID = _l1ChainId; | ||
| BASE_TOKEN_ASSET_ID = _baseTokenAssetId; | ||
| _privateNtv = _ntv; | ||
| } |
There was a problem hiding this comment.
Prevent re-initializing the private asset tracker
initialize is fully unrestricted and can be called repeatedly, so any account can overwrite _privateNtv after deployment; because the inherited bridge-accounting entry points are gated by _nativeTokenVaultAddress(), an attacker can repoint the vault to themselves and then call privileged tracking functions (e.g. finalize/initiate bridging handlers), corrupting balances and migration state on any deployed private stack.
Useful? React with 👍 / 👎.
| L2_TO_L1_MESSENGER_SYSTEM_CONTRACT.sendToL1( | ||
| abi.encodePacked(PRIVATE_BUNDLE_IDENTIFIER, keccak256(_interopBundleBytes), _callCount) | ||
| ); | ||
| return bytes32(0); |
There was a problem hiding this comment.
Return the real L2->L1 message hash for private bundles
_sendBundleToL1 sends the message but always returns bytes32(0), so InteropBundleSent will emit a zero l2l1MsgHash for every private bundle; downstream tooling and tests that rely on this hash to correlate bundle events with messenger inclusion data cannot do so reliably.
Useful? React with 👍 / 👎.
| ) internal override { | ||
| // No base-token value collection for private interop. | ||
| } |
There was a problem hiding this comment.
Enforce msg.value accounting in private sendBundle flow
This override removes _ensureCorrectTotalValue entirely, so private sendBundle accepts arbitrary msg.value without validation or accounting; callers can accidentally lock ETH in the contract (no corresponding fee balance is credited), and existing contract balance can be unintentionally used to fund later indirect-call message values when users underpay.
Useful? React with 👍 / 👎.
…bs/era-contracts into kl/bzzk
|
Coverage after merging kl/bzzk into kl/anvil-testing-interop will be
Coverage Report |
What ❔
Why ❔
Checklist