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.

Sign up for GitHub

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

Jump to bottom

fix(protocol): revert impl deployment V2 #14621

Merged
merged 10 commits into from
Sep 3, 2023
Merged

fix(protocol): revert impl deployment V2 #14621

merged 10 commits into from
Sep 3, 2023

Conversation

adaki2004
Copy link
Contributor

Almost same as #14611 but this is now a combined version so that we can make it TransparentProxyUpgradeable.

Please @cyberhorsey or @davidtaikocha confirm if this is also a working solution (invokeMessageCall() succeeds when bridging erc20 from L1 to L2) and then we can go forward with this type of solution and abandon the other.

@vercel
Copy link

vercel bot commented Aug 31, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bridge-ui-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 2, 2023 4:07pm

@adaki2004 adaki2004 mentioned this pull request Aug 31, 2023
Closed
@cyberhorsey
Copy link
Contributor

Will attempt today and confirm if this one also works.

@cyberhorsey
Copy link
Contributor

@adaki2004 it worked.
https://explorer.internal.taiko.xyz/tx/0x2dd4f9987320677730d75c23b620463087f5f8f7ff4911d2777469cc2ca6122b
here is a TX of it beign deployed, with a fresh ERC20, with the latest changes from this branch as the ERC20Vault on L2.

@adaki2004
Copy link
Contributor Author

adaki2004 commented Aug 31, 2023 •
edited
Loading

@adaki2004 it worked.
https://explorer.internal.taiko.xyz/tx/0x2dd4f9987320677730d75c23b620463087f5f8f7ff4911d2777469cc2ca6122b
here is a TX of it beign deployed, with a fresh ERC20, with the latest changes from this branch as the ERC20Vault on L2.

Amazing! Thanks Jeff! Will proceed then with the others tomorrow!

@adaki2004
Copy link
Contributor Author

Updated the other vaults. The test with deployed FE turned out to be a FE issue (not enough gas was sent to deploy).

@dantaik I addressed one todo (or rather question) to you in ERC20Vault - once it is cleared we can merge this.

Otherwise it works, thanks Jeff for confirming!

@adaki2004 adaki2004 requested a review from dantaik September 1, 2023 07:51
@adaki2004 adaki2004 marked this pull request as ready for review September 1, 2023 07:51
dantaik
dantaik previously approved these changes Sep 1, 2023
View reviewed changes
@dantaik dantaik enabled auto-merge September 1, 2023 14:11
@dantaik dantaik changed the title fix(protocol): Revert impl deployment V2 fix(protocol): revert impl deployment V2 Sep 1, 2023
@dantaik dantaik self-requested a review September 2, 2023 16:05
@dantaik dantaik added this pull request to the merge queue Sep 3, 2023
Merged via the queue into main with commit 7e59e0b Sep 3, 2023
@dantaik dantaik deleted the revert_to_Create2Upgradeable_v2 branch September 3, 2023 01:50
@github-actions github-actions bot mentioned this pull request Sep 3, 2023
Merged
@davidtaikocha davidtaikocha mentioned this pull request Sep 4, 2023
Closed
2manslkh pushed a commit that referenced this pull request Sep 14, 2023
@adaki2004 @dantaik
@dong77 @2manslkh
fix(protocol): revert impl deployment V2 (#14621)
439c7e5 
Co-authored-by: Daniel Wang <[email protected]>
Co-authored-by: Daniel Wang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidtaikocha davidtaikocha davidtaikocha approved these changes

@dantaik dantaik dantaik approved these changes

@cyberhorsey cyberhorsey Awaiting requested review from cyberhorsey

Assignees
No one assigned
Labels
area.protocol
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@adaki2004 @cyberhorsey @dantaik @davidtaikocha @dong77