Skip to content

feat: deterministic deployment scripts #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 48 commits into from

Conversation

Thegaram
Copy link
Contributor

@Thegaram Thegaram commented Jul 9, 2024

Migrated from scroll-tech/scroll#1303.

Problem: The current deployment scripts are slow and error-prone. We need to predict the deployment addresses of some contracts (e.g. for preallocating funds to L2ScrollMessenger in L2 genesis), but currently the addresses depend on the deployer private key and the order of deployment. If one transaction fails, subsequent transactions will lead to different addresses.

Solution:

  • Use https://github.com/Arachnid/deterministic-deployment-proxy to make contract addresses much more predictable.
  • Use a single Foundry script for deployment and initialization on L1 and L2 (basically merge the previous 5 scripts).
  • Move genesis generation into the Foundry script. Previously this was a complex combination of Javascript, Rush, and bash scripts.
  • Provide convenient Docker images. This makes deployment much more robust (we enforce a fixed Foundry version known to be working), and fast (compilation done in the build phase, no need to recompile when running the container).
  • Use a TOML configuration file.

Caveats:

  • Some of the deployed addresses still depend on the deployer account. The reason is that some contracts take the owner as a constructor argument, and the initial owner is the deployer account, so changing this also changes the create2 deterministic address. But the addresses do not depend on deployment order anymore.

@Thegaram Thegaram requested a review from yiweichi July 9, 2024 13:55
@yiweichi yiweichi marked this pull request as ready for review July 10, 2024 13:58
@Thegaram Thegaram marked this pull request as draft July 10, 2024 14:56
@zimpha zimpha self-requested a review July 17, 2024 03:40
@yiweichi yiweichi force-pushed the feat-deterministic-deployment branch from 187563f to 65c5b4e Compare August 20, 2024 17:27
@yiweichi yiweichi requested a review from zimpha September 26, 2024 08:23
@yiweichi
Copy link
Member

moved it here: #63

@yiweichi yiweichi closed this Nov 20, 2024
@zimpha zimpha deleted the feat-deterministic-deployment branch November 20, 2024 08:23
shu-unifra added a commit to unifralabs/scroll-contracts that referenced this pull request Apr 28, 2025
comment some unused contract address in verify.sh
dasdsadasca pushed a commit to dasdsadasca/scroll-contracts that referenced this pull request May 27, 2025
I've completed a meticulous verification of 8 of your specified vulnerability claims.

**Key Confirmed/Partially Confirmed Vulnerabilities:**

1.  **scroll-tech#7 Cross-Chain Message Replay via State Inconsistency (CONFIRMED - High/Critical):**
    *   **Issue:** In `L2ScrollMessenger._executeMessage` (and symmetrically in `L1ScrollMessenger.relayMessageWithProof`), the execution flag (`isL1MessageExecuted` / `isL2MessageExecuted`) is set *after* the external call.
    *   **Impact:** If the external call succeeds but the subsequent transaction fails (e.g., out-of-gas before setting the flag), and assuming the target contract's state changes from the successful call persist (as per your detailed exploit premise), the message can be replayed, leading to double execution (e.g., double minting/transfers).
    *   **Recommendation:** Set execution flags *before* the external call.

2.  **scroll-tech#8 Gas Price Oracle Manipulation (PARTIALLY CONFIRMED - High):**
    *   **Issue:** If a malicious L2 Sequencer can feed a significantly inflated L1 basefee to the `L1GasPriceOracle` contract on L2.
    *   **Impact:** Users on L2 would be overcharged for initiating L2->L1 messages, potentially making withdrawals/L2->L1 interactions economically unviable.
    *   **Mitigation:** Relies on trusted Sequencer operation and/or L2 node validation of L1 data.

3.  **scroll-tech#3 Enforced Batch Mode Bypass (PARTIALLY CONFIRMED - Medium/Low impact on direct censorship):**
    *   **Issue:** Sequencers/Provers can prevent the "finalization staleness" trigger for enforced mode by regularly finalizing batches, even if these batches exclude specific L2-originated transactions.
    *   **Impact:** If L1->L2 messages are not also stuck, enforced mode may not activate, allowing censorship of specific L2-native transactions. This is a limitation of the trigger's scope for L2 tx censorship resistance.
    *   **Mitigation:** Users facing L2 tx censorship would need to use L1->L2 messages.

**Refuted Critical/High Vulnerabilities (as per your specific claims):**
*   **scroll-tech#1 ScrollChain Batch Finalization Race Condition:** REFUTED. Critical state updates occur after proof verification.
*   **scroll-tech#2 L1ScrollMessenger Withdrawal Proof Bypass:** REFUTED. Comprehensive hashing and replay protection are effective.
*   **scroll-tech#4 Gateway Router Reentrancy Attack:** REFUTED. Layered defenses (context locks, nonReentrant guards) protect against claimed exploits.
*   **scroll-tech#5 Batch Bridge Hash Collision Attack:** REFUTED. Hashing mechanism is sound against collisions for fixed-size inputs.
*   **scroll-tech#6 Lido Bridge Rebasing Token Manipulation:** REFUTED. Bridge is designed for non-rebasing wstETH.

This information includes all prior documentation and detailed vulnerability analysis reports culminating in these verified findings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants