Skip to content

more robust CREATE3_FACTORY deployments#1647

Open
0xDEnYO wants to merge 1 commit intomainfrom
robust-create3-deployments
Open

more robust CREATE3_FACTORY deployments#1647
0xDEnYO wants to merge 1 commit intomainfrom
robust-create3-deployments

Conversation

@0xDEnYO
Copy link
Copy Markdown
Contributor

@0xDEnYO 0xDEnYO commented Mar 11, 2026

Why did I implement it this way?

fixes Chain .... not supported error for chains that are not in alloy-chains

Checklist before requesting a review

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

@lifi-action-bot lifi-action-bot marked this pull request as draft March 11, 2026 02:25
@lifi-action-bot lifi-action-bot changed the title more robust CREATE3_FACTORY deployments more robust CREATE3_FACTORY deployments Mar 11, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 11, 2026

Walkthrough

The script script/deploy/deployAndStoreCREATE3Factory.sh is refactored to implement a two-stage deployment strategy: attempting Foundry forge deployment first, then falling back to cast-based deployment for unsupported chains, with improved error handling and address extraction.

Changes

Cohort / File(s) Summary
CREATE3 Factory Deployment Script Refactor
script/deploy/deployAndStoreCREATE3Factory.sh
Introduces dual deployment paths: primary Foundry forge-based deployment with address extraction from raw return data, and secondary fallback using cast send for chains not in Foundry's alloy-chains list. Adds improved error messaging, early failure handling on private key retrieval, and reduces nested conditional complexity while preserving gas-estimate-multiplier and simulation flags.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

AuditNotRequired

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. Required section 'Which Linear task belongs to this PR?' is missing entirely. Add the missing 'Which Linear task belongs to this PR?' section to identify the associated Linear task or ticket number.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: making CREATE3_FACTORY deployments more robust by fixing the alloy-chains compatibility issue.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch robust-create3-deployments
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
script/deploy/deployAndStoreCREATE3Factory.sh (1)

59-102: Fallback path for unsupported chains lacks bytecode verification for parity with forge path.

The cast-based fallback correctly handles chains not in Foundry's alloy-chains list. However, unlike the forge path which polls for bytecode availability via extractDeployedAddressFromRawReturnData (with its bytecode verification loop for blockchain sync delays), this path directly uses the contract address from the receipt without verification.

For consistency, consider adding a bytecode check after line 100:

♻️ Optional: Add bytecode verification for parity with forge path
       FACTORY_ADDRESS=$(echo "$RECEIPT_JSON" | jq -r '.contractAddress // empty' 2>/dev/null)
+      # Verify bytecode is available (parity with forge path)
+      if [[ -n "$FACTORY_ADDRESS" && "$FACTORY_ADDRESS" != "null" ]]; then
+        if [[ "$(doesAddressContainBytecode "$NETWORK" "$FACTORY_ADDRESS")" != "true" ]]; then
+          warning "Contract deployed but bytecode not yet visible; may need blockchain sync time"
+        fi
+      fi
     fi
   fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@script/deploy/deployAndStoreCREATE3Factory.sh` around lines 59 - 102, The
fallback branch that sets FACTORY_ADDRESS from the cast receipt needs the same
post-deploy bytecode verification loop as the forge path: after FACTORY_ADDRESS
is set, poll the chain via cast rpc (e.g., cast code or eth_getCode) until the
on-chain bytecode for FACTORY_ADDRESS is non-empty / matches the expected
runtime bytecode (use the artifact bytecode/runtime bytecode extracted from
ARTIFACT) or until a timeout/retries expire; mirror the logic used by
extractDeployedAddressFromRawReturnData (retry/backoff and error handling) and
error out if verification fails so the cast fallback has parity with the forge
deploy path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@script/deploy/deployAndStoreCREATE3Factory.sh`:
- Around line 59-102: The fallback branch that sets FACTORY_ADDRESS from the
cast receipt needs the same post-deploy bytecode verification loop as the forge
path: after FACTORY_ADDRESS is set, poll the chain via cast rpc (e.g., cast code
or eth_getCode) until the on-chain bytecode for FACTORY_ADDRESS is non-empty /
matches the expected runtime bytecode (use the artifact bytecode/runtime
bytecode extracted from ARTIFACT) or until a timeout/retries expire; mirror the
logic used by extractDeployedAddressFromRawReturnData (retry/backoff and error
handling) and error out if verification fails so the cast fallback has parity
with the forge deploy path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 77df83d3-5e74-4336-b37a-f8ed6a8c7343

📥 Commits

Reviewing files that changed from the base of the PR and between ecef43c and 0c822d0.

📒 Files selected for processing (1)
  • script/deploy/deployAndStoreCREATE3Factory.sh

@0xDEnYO 0xDEnYO marked this pull request as ready for review March 19, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants