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

Tim/eng 3475 nonce not updating correctly after switching accounts #725

Conversation

teebszet
Copy link
Member

@teebszet teebszet commented Dec 20, 2023

πŸ”˜ PR Type

  • Bugfix

πŸ“œ Background

Issue Link: https://linear.app/xverseapp/issue/ENG-3475

there is a feature on the transaction request screen which checks a dapp tx request popup payload and does an auto account switch if there is a mismatch.

scenario 1:
if I signed in with account 1 on the dapp, and switched my extension to account 2,
expect the popup for a dapp tx will auto switch my account back to the one matching what I signed into the dapp with.

πŸ”„ Changes

this fix doesn't change the behaviour, but ensures the auto switch also regenerates the unsignedTx with the correct account (and therefore displays the correct nonce in Edit Nonce)

  • fix: bug where transaction request screen would initiate an auto account switch, but not regenerate the unsignedTx

Impact:

  • STX transaction request screen
  • only used when interacting with dapps

πŸ–Ό Screenshot / πŸ“Ή Video

after fix:

trimmed.mov

also tested with pending transactions, and nonce increments as expected

βœ… Review checklist

Please ensure the following are true before merging:

  • Code Style is consistent with the project guidelines.
  • Code is readable and well-commented.
  • No unnecessary or debugging code has been added.
  • Security considerations have been taken into account.
  • The change has been manually tested and works as expected.
  • Breaking changes and their impacts have been considered and documented.
  • Code does not introduce new technical debt or issues.

Copy link

@teebszet teebszet marked this pull request as draft December 20, 2023 12:57
@teebszet
Copy link
Member Author

I see some request looping. not ready for merge

@m-aboelenein m-aboelenein changed the base branch from develop to mahmoud/eng-3111-integrate-with-stacks-raw-hex-signing-on-extension December 22, 2023 07:56
@m-aboelenein m-aboelenein force-pushed the tim/eng-3475-nonce-not-updating-correctly-after-switching-accounts branch from 62932b0 to 3e83755 Compare December 22, 2023 09:52
@m-aboelenein m-aboelenein marked this pull request as ready for review December 27, 2023 13:36
@m-aboelenein m-aboelenein self-assigned this Dec 27, 2023
Copy link
Contributor

@dhriaznov dhriaznov left a comment

Choose a reason for hiding this comment

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

Got a couple of code suggestions, otherwise LGTM

src/app/screens/transactionRequest/index.tsx Show resolved Hide resolved
src/app/screens/transactionRequest/index.tsx Show resolved Hide resolved
@m-aboelenein m-aboelenein merged commit 45d9f63 into mahmoud/eng-3111-integrate-with-stacks-raw-hex-signing-on-extension Dec 27, 2023
@m-aboelenein m-aboelenein deleted the tim/eng-3475-nonce-not-updating-correctly-after-switching-accounts branch December 27, 2023 17:25
m-aboelenein added a commit that referenced this pull request Dec 27, 2023
…652)

* init tx hex handling

* added stx hex tx signing support

* integrate with connect core utils

* fix stx tx serialization

* handle sign-hex for dapp interactions

* update core version

* add auh to tx-creation

* fix pending txs handling

* fix type issues

* update core version

* revert removing signMultipleTransactions

* update core version

* remove unused package aliases

* Tim/eng 3475 nonce not updating correctly after switching accounts (#725)

* fix: regenerate the unsignedTx with correct stxAddress after switch account

* fix: minor cleanup

* fix tx-request account switching

* updated core version

* added type checking and default values for feeMultipliers

---------

Co-authored-by: Mahmoud Aboelenein <mahmoud@secretkeylabs.com>

---------

Co-authored-by: Tim Man <tim@secretkeylabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants