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

fix: sign actions should wait for preCallback completion before proceeding #193

Merged
merged 4 commits into from
Jan 3, 2024

Conversation

cameronvoell
Copy link
Contributor

@cameronvoell cameronvoell commented Dec 22, 2023

Related PRs:

Description

  • TypeScript code updated
  • Android Kotlin code updated
  • iOS Swift code updated
  • Finish verifying no unexpected error conditions can occur (see To Test section below)

In the PRs linked above we added a callback that could be passed in during identity creation that would be fired to notify when identity creation and enabling occurred.

However those callbacks were implemented in a way that merely notified that sign actions would occur without actually blocking the signing actions until the callbacks were finished.

For example, the preEventCallbacks in xmtp-js block signing actions until after callbacks are finished, which gives developers greater flexibility: https://github.com/xmtp/xmtp-js/blob/e96b2e214bef3965f703be09863fa8b2fb4552d2/src/Client.ts#L178-L194

This change updates our code to wait until callbacks are completed before resuming identity sign request logic.

To Test

The following branch has changes from this PR cherry-picked on to a branch that has in progress code for connecting external wallets in the example app (via thirdweb-sdk + wallet connect): cameronvoell-callback-testing-wallet

Paths to check:

  1. Passing in callbacks blocks signing actions until callback completes on Android + iOS
  2. Identity creation works as expected when no callbacks are passed in to identity creation functions on Android and iOS
  3. Paths where errors occur due to rejected signing attempts or anything else do not cause callbacks to block identity creation indefinitely or lead to any other unexpected errors.

@nplasterer nplasterer marked this pull request as ready for review January 2, 2024 23:35
@nplasterer nplasterer requested a review from a team as a code owner January 2, 2024 23:35
@nplasterer nplasterer merged commit 66cc38c into main Jan 3, 2024
4 of 5 checks passed
@nplasterer nplasterer deleted the cameronvoell/sign-actions-wait-for-callbacks branch January 3, 2024 23:55
Copy link
Contributor

github-actions bot commented Jan 3, 2024

🎉 This PR is included in version 1.23.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants