Skip to content

fix: deploy new factory with old sso beacon#179

Closed
cpb8010 wants to merge 3 commits intomainfrom
factory-fix
Closed

fix: deploy new factory with old sso beacon#179
cpb8010 wants to merge 3 commits intomainfrom
factory-fix

Conversation

@cpb8010
Copy link
Contributor

@cpb8010 cpb8010 commented Sep 3, 2025

Description

Fix disallowed storage slot bug when doing any signing using newly deployed accounts.

This wasn't just affecting OIDC, the address.

Additional context

The problem was in the upgrade process a new SSO beacon proxy was
deployed for the new implemention instead of re-using the existing one.
This was because it used the era-sepolia.json which only contains the
contracts necessary for the auth-server and didn't have the SSO Beacon
address.
The SSO beacon address needs to stay the same because its address is
specifically allowed by the sequencer.

Might also need to update + fund the paymaster, will test in PR.

The problem was in the upgrade process a new SSO beacon proxy was
deployed for the new implemention instead of re-using the existing one.
This was because it used the era-sepolia.json which only contains the
contracts necessary for the auth-server and didn't have the SSO Beacon
address.
The SSO beacon address needs to stay the same because its address is
specifically allowed by the sequencer.

Might also need to update + fund the paymaster, will test in PR.
@cpb8010 cpb8010 self-assigned this Sep 3, 2025
@github-actions
Copy link

github-actions bot commented Sep 3, 2025

Visit the preview URL for this PR (updated for commit c350f2e):

https://zksync-auth-server-staging--pr179-factory-fix-w0z0dlsi.web.app

(expires Thu, 11 Sep 2025 04:05:13 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 509a9c9ea42583076f531c53cf2979c544d5d0b7

This has the new factory
Attempted to upgrade factory and now paymaster is broken
@cpb8010 cpb8010 requested a review from Copilot September 4, 2025 04:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a deployment issue where a new SSO beacon proxy was incorrectly deployed instead of reusing the existing one, causing storage slot conflicts for newly deployed accounts. The fix updates contract addresses to use the correct existing SSO beacon that is allowlisted by the sequencer.

  • Updates account factory and paymaster addresses in era-sepolia.json configuration
  • Reverts salt service URL to localhost for development and adds proper production URL for CI
  • Updates contracts submodule to include the corrected deployment

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/contracts Updates submodule commit to include corrected contract deployments
packages/auth-server/stores/era-sepolia.json Updates account factory and paymaster addresses to use correct deployments
packages/auth-server/nuxt.config.ts Reverts salt service URL to localhost for development
.github/workflows/deploy-preview.yml Adds production salt service URL environment variable for CI deployment

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

oidc: {
googlePublicClient: "69763429492-f7nl555i50akmail80pid3m4hhsg7u2n.apps.googleusercontent.com",
saltServiceUrl: process.env.NUXT_PUBLIC_SALT_SERVICE_URL || "https://sso-oidc.zksync.dev/salt",
saltServiceUrl: process.env.NUXT_PUBLIC_SALT_SERVICE_URL || "http://localhost:3030/salt",
Copy link

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

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

The default salt service URL has been changed from a production URL to localhost. This could cause issues if the environment variable is not set in production deployments. Consider documenting this change or ensuring all deployment environments properly set the NUXT_PUBLIC_SALT_SERVICE_URL variable.

Copilot uses AI. Check for mistakes.
@cpb8010
Copy link
Contributor Author

cpb8010 commented Sep 4, 2025

Closing, squashed elsewhere. The fact that the auth server preview didn't update when changes were made was pretty confusing, but maybe there's a cool down

@cpb8010 cpb8010 closed this Sep 5, 2025
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.

2 participants