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: Change UpdatePassword to take multiple addresses, roll back on error #175

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

jefft0
Copy link
Contributor

@jefft0 jefft0 commented Sep 11, 2024

BREAKING CHANGE: Change UpdatePassword to take an array of addresses, roll back on error.

Explanation: Currently, UpdatePassword takes a single address of the account to update the password. If a native application wants to change multiple accounts to the new password, it does a loop to call UpdatePassword multiple times. But if one of the calls has an error, the application breaks out of the loop and leaves the Keybase in an inconsistent state where some passwords are changed but not all. The application could write some logic to roll back these changes, but it is better for the Gno Native Kit service to handle this.

This PR has two commits:

  1. Change the UpdatePassword request to take a required array of account addresses. (If the application only wants to update the password of one account, it is easy to make an array of one address.) In api.go, we also change UpdatePassword to roll back to the old password in case of error.
  2. In types.ts and GnoNativeApi.ts, change updatePassword to take a required array of Uint8Array for the addresses.

@jefft0 jefft0 force-pushed the fix/UpdatePassword-rollback branch from 49a15b5 to 229982a Compare September 16, 2024 07:19
@jefft0 jefft0 marked this pull request as ready for review September 16, 2024 07:19
@jefft0 jefft0 force-pushed the fix/UpdatePassword-rollback branch from 229982a to 68b0b75 Compare September 16, 2024 07:23
@jefft0 jefft0 requested a review from D4ryl00 September 16, 2024 09:29
Copy link
Contributor

@D4ryl00 D4ryl00 left a comment

Choose a reason for hiding this comment

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

LGTM

@jefft0 jefft0 merged commit 7534c7d into gnolang:main Sep 17, 2024
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