Skip to content

Conversation

@andygruening
Copy link
Contributor

@andygruening andygruening commented Apr 5, 2025

On master, account federation fails after recovering a wallet from storage. When recovering the session, the call to SetupAuthenticator creates a new _sessionWallet and overwrites the sessionId, because TryToLoginWithStoredSessionWallet is not updating the _connectedWalletAddress value (first if-expression inside SetupAuthenticator equals to true - which creates a new _sessionWallet)

Version Increment

Please ensure you have incremented the package version in the package.json as necessary.

  • I have incremented the package.json according to semantic versioning
  • No version increment is needed; the change does not impact SDK or Sample code/assets

Docs Checklist

Please ensure you have addressed documentation updates if needed as part of this PR:

  • I have created a separate PR on the sequence docs repository for documentation updates: Link to docs PR
  • No documentation update is needed for this change.

@andygruening andygruening requested a review from a team April 5, 2025 08:06
@andygruening andygruening requested a review from a team as a code owner April 5, 2025 08:06
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 5, 2025

Deploying sequence-unity with  Cloudflare Pages  Cloudflare Pages

Latest commit: f04f7dd
Status:🚫  Build failed.

View logs

@andygruening andygruening changed the title Fixed Account federation after recovering wallet from secure storage Fixed Account federation after recovering wallet from storage Apr 5, 2025
Copy link
Contributor

@BellringerQuinn BellringerQuinn left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks Andy!

Do you think we can remove this section on re-using the login panel for federation?
https://docs.sequence.xyz/sdk/unity/onboard/authentication/federated-accounts#re-using-the-default-loginpanel
It seems like this is no longer relevant with these changes? Is that correct?

BellringerQuinn

This comment was marked as resolved.

Copy link
Contributor

@BellringerQuinn BellringerQuinn left a comment

Choose a reason for hiding this comment

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

Sweet! Working now!

@caballoninja caballoninja self-requested a review April 15, 2025 22:00
@andygruening andygruening merged commit 72c77d9 into master Apr 16, 2025
1 of 2 checks passed
@andygruening andygruening deleted the fix-federation-to-guest branch April 16, 2025 12:49
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