Skip to content
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

Replace Create3 with ZeframLou/create3-factory #1387

Merged
merged 6 commits into from
Sep 18, 2023

Conversation

trajan0x
Copy link
Contributor

@trajan0x trajan0x commented Sep 18, 2023

Description

Addresses: #1381 (comment) by using https://github.com/ZeframLou/create3-factory

Summary by CodeRabbit

  • Refactor: Updated the submodule URL for packages/contracts-core/lib/create3-factory to a new repository.
  • New Feature: Added a new module create3 in the lib/create3-factory/src directory.
  • Refactor: Changed the contract name from DeployCREATE3 to DeployCREATE3Factory and updated its import statements. The contract file is now part of a common deployer-utils package.
  • Refactor: Updated the import path for the CREATE3Factory contract in DeployerUtils.sol, indicating a change in the contract's location.

@github-actions github-actions bot added M-contracts Module: Contracts 2-reviewers C-Protocol-Critical PRs that modify protocol-critical code. labels Sep 18, 2023
@trajan0x trajan0x mentioned this pull request Sep 18, 2023
4 tasks
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (a480961) 50.88921% compared to head (2545462) 50.88921%.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           feat/devnet-upper       #1387   +/-   ##
=====================================================
  Coverage           50.88921%   50.88921%           
=====================================================
  Files                    353         353           
  Lines                  24966       24966           
  Branches                 277         277           
=====================================================
  Hits                   12705       12705           
  Misses                 10981       10981           
  Partials                1280        1280           
Flag Coverage Δ
cctp-relayer 63.33333% <ø> (ø)
promexporter 41.02564% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aureliusbtc
Copy link
Contributor

LGTM - but think we should keep a deploy script that imports the create3 lib in our scripts folder, so that it creates our standardized deployment JSON using DeployerUtils vs fully forcing the deployer to use zefram/create3 to deploy

@trajan0x
Copy link
Contributor Author

Can do this. One thing to consider here is @ChiTimesChi is working on a deployer-utils folder soon and I don't know how well tested the Create3Factory is (I've actually never ran it, I got it from #1312).

Happy to re-include for now

@trajan0x trajan0x marked this pull request as ready for review September 18, 2023 15:49
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 18, 2023

Walkthrough

This pull request primarily involves changes related to the CREATE3Factory contract. The contract has been moved to a different location, and its import paths have been updated accordingly in various files. Additionally, a new submodule URL for the create3-factory module has been set.

Changes

File Summary
.gitmodules Submodule URL for packages/contracts-core/lib/create3-factory updated to https://github.com/zeframlou/create3-factory.
packages/contracts-core/lib/create3-factory Single commit with hash 06ec0ff36d41853dcd4399fbe2127aef801c4077 made.
packages/contracts-core/remappings.txt Addition of new module create3 in lib/create3-factory/src.
packages/contracts-core/script/.../DeployCREATE3Factory.sol Import statement for CREATE3Factory updated, contract name changed from DeployCREATE3 to DeployCREATE3Factory, and file is now part of deployer-utils package.
packages/contracts-core/script/utils/DeployerUtils.sol Import statement for CREATE3Factory contract updated to reflect new location.

🐇💻

In the land of code where the shadows lie,

A factory was moved under the digital sky.

Paths were changed, contracts renamed,

In the world of blockchain, nothing remained the same.

But fear not, for the rabbit's here,

Celebrating the changes with a cheer! 🎉🥕


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.

Copy link
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.

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between a480961 and 2545462 commits.
Files selected for processing (5)
  • .gitmodules (1 hunks)
  • packages/contracts-core/lib/create3-factory (1 hunks)
  • packages/contracts-core/remappings.txt (1 hunks)
  • packages/contracts-core/script/DeployCREATE3Factory.sol (1 hunks)
  • packages/contracts-core/script/utils/DeployerUtils.sol (1 hunks)
Files skipped from review due to trivial changes (3)
  • .gitmodules
  • packages/contracts-core/lib/create3-factory
  • packages/contracts-core/remappings.txt
Additional comments (Suppressed): 3
packages/contracts-core/script/DeployCREATE3Factory.sol (1)
  • 7-16: The contract name has been changed from DeployCREATE3 to DeployCREATE3Factory. Ensure that all references to this contract throughout the codebase have been updated to match the new name. Also, verify that the new import path for CREATE3Factory is correct and accessible.
packages/contracts-core/script/utils/DeployerUtils.sol (2)
  • 8-8: The import path for CREATE3Factory has been updated. Ensure that the new path correctly points to the CREATE3Factory.sol file in the create3 module.

  • 10-11: No changes have been made to the ICreate3Factory interface. This is good as it ensures compatibility with existing code that uses this interface.

@trajan0x trajan0x merged commit 616c5ab into feat/devnet-upper Sep 18, 2023
53 of 59 checks passed
@trajan0x trajan0x deleted the feat/remove-create3 branch September 18, 2023 15:51
trajan0x added a commit that referenced this pull request Sep 18, 2023
trajan0x added a commit that referenced this pull request Sep 18, 2023
trajan0x added a commit that referenced this pull request Sep 18, 2023
dwasse added a commit that referenced this pull request Nov 10, 2023
* docker first pass

* fix omnirpc port default behavior

* Docker-compose updates

* add guide for adding new go modules

* typo fix

* indentation fix

* kink fix

* remove vanity

* devnet fix

* backup for max attempt change

* hi

* make retry defaults more sensible

* lint fix

* Revert "hi"

This reverts commit 60dd2b7.

* test

* omnirpc chains endpoint

* close response handle

* actually close response

* more fixes

* wip

* forge install: solmate

* newmates

* create3

* create3factory

* custom config path

* Revert "custom config path"

This reverts commit 47ee5f9.

* deploy config defaults

* Revert "deploy config defaults"

This reverts commit 969a6cd.

* devnet check

* basic updates

* factory fixes

* cleanup

* devnet up/down

* first pass

* guard it up

* guard

* 1 notary

* more updates

* fix wrong domain in deploy script

* diff fix

* comment fix

* agent proof updates

* synapse domain fix

* generic linter fix

* get guard to boot [goreleaser]

* more yaml lint

* some nice debugging tools

* generic linter fix

* finish devnet

* fix lint commandS

* d

* wrap up rudimentary devneT

* create3

* deployer/create fix

* fix ubuntu

* lowercase non-constants

* icreate3 import

* Revert "icreate3 import"

This reverts commit bcb2522.

* move to internal

* another internal change

* update installs on dev containers for easier debugging

* Update packages/contracts-core/README.md

Co-authored-by: χ² <88190723+ChiTimesChi@users.noreply.github.com>

* address #1381 (comment)

* Replace Create3 with ZeframLou/create3-factory (#1387)

* create3

* forge install: create3-factory

* replace create3 w/ external module

* fix naming issues

* create3 factory re-addition to address #1387 (comment)

---------

Co-authored-by: Trajan0x <trajan0x@users.noreply.github.com>

* Revert "Replace Create3 with ZeframLou/create3-factory (#1387)" (#1388)

This reverts commit 616c5ab.

* Replace Create3 with ZeframLou/create3-factory

This reverts commit 6b100fd.

* fix package issue

---------

Co-authored-by: Trajan0x <trajan0x@users.noreply.github.com>
Co-authored-by: χ² <88190723+ChiTimesChi@users.noreply.github.com>
Co-authored-by: Daniel Wasserman <wassermandaniel8@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-reviewers C-Protocol-Critical PRs that modify protocol-critical code. M-contracts Module: Contracts size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants