Repository navigation
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
godzillaba
reviewed
Oct 21, 2024
gzeoneth
requested changes
Oct 25, 2024
gzeoneth
reviewed
Oct 31, 2024
Comment on lines
+49
to
+51
| /// @dev Due to how arb governance works, it is not possible to wipe out the content of these | ||
| /// 4 storage slots during the upgrade. So after deprecation values in these slots will | ||
| /// stay "dirty" with default values, but slots will not be used or accessible in any way. |
Member
There was a problem hiding this comment.
let's remove these, keeping them dirty is simply a design choice
gzeoneth
self-requested a review
October 31, 2024 13:21
yahgwai
removed their request for review
July 27, 2026 08:50
Conflict resolutions: - hardhat.config.ts, audit-ci.jsonc, slither.db.json: take chore/solc-0.8.30. Its 0.8.30/cancun config replaces this branch's 0.8.28 bump, and the audit allowlist follows the current lockfile. - contract-tests.yml: use the repo's install-foundry action; keep this branch's format check, solhint and coverage steps disabled. - AbsBridge.setOutbox: keep the new formatting with this branch's address(0) check. - ERC20Outbox.t.sol: keep the _happyExecTx helper and add this branch's zeroed-context asserts to it. - test/storage, test/signatures: regenerated.
Re-accepts three findings already triaged on develop or this branch, which reappeared under new line numbers: Bridge._executeLowLevelCall arbitrary-send-eth, and the AbsOutbox.executeTransactionImpl reentrancy and AbsBridge.activeOutbox write-after-write from the transient save/restore. Triaged with slither-analyzer 0.11.4, the version CI installs.
yahgwai
added
the
audit-before-merge
Bump solhint to ^5.0.5, the first release whose parser accepts `transient`, and rename the disabled custom-errors rule to its new name, gas-custom-errors. Format BridgeTester with Foundry 1.8.3, the version CI pins.
Re-accepts the AbsOutbox.executeTransactionImpl reentrancy and AbsBridge.activeOutbox write-after-write findings at their new line numbers. Triaged with slither-analyzer 0.11.4.
Recording the bridge's active outbox in setVersion broke the e2e retryable test, which calls it from an aliased L1 address rather than the bridge. Move the recording into setVersionAndRecordOutbox and use that in the Foundry bridge test.
Transient variables get slots by declaration order, so an upgrade that runs mid-withdrawal needs that order kept, just like storage. The snapshot is only appended for contracts that have transient variables, so today only the bridge and outbox snapshots change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #479.
__contextand__activeOutbox, so the storage layout doesn't change. They keep their current placeholder values, and nothing reads them.postUpgradeInitfrom the Outbox andIOutbox. The old slots are left as they are rather than wiped, so the upgrade needs no init call.setOutboxnow rejectsaddress(0)instead of the all-ffaddress, because0is whatactiveOutboxnow holds when no outbox is active.^5.0.5, the first release that can parsetransient.Part of BLK-430