Display account in app nav whenever signing via API

[314159265359879]

Some more testing revealed that this https://github.com/leather-wallet/extension/issues/4599 has different has a different result on Stacks compared to Bitcoin.

Reproduction steps on Bitcoin dapps

1. Sign in to gamma.io on account 1
2. Then on the leather extension switch to account 2
3. Try to buy an ordinal or create an inscription
4. See the screen pop up with Account 2 top left (expected is account 1).

A user reported that using unisat they signed transactions with another account than expected. I think it is caused by this. When you mint an inscription the transaction is signed with the active account on the wallet rather than the account signed in with on the dapp:

Trying the same thing buying an ordinal with unisat (signed in with account 1 and account 2 active will throw an error):

Trying the same thing on Magic Eden and Gamma resulted in no transmission of the transaction.
I tested minting a text inscription on Magic Eden after clicking confirm, the confirm screen closes but nothing is transmitted.

On Gamma I tested buying a print and the result was similar as on Magic Eden, clicking confirm closes the confirm screen but no transaction is transmitted.

Possible solutions: (a) better error handling, (b) the ME and gamma can show similar errors (c) (perhaps suggest them to make sure the active account on the extension is the same), (d) and allow trying again (not possible on ME without a refresh). (f) It would be great if the wallet could detect users who are trying to send with a different wallet and then allow an easy way for the user to switch.

[edgarkhanzadian]

Gamma.io doesn't seem to require a specific account index on sign in:

testgamma.mov

cc @314159265359879

[314159265359879]

with bitcoin it will sign in with the account that is active in the extension when you click connect. Then a user can switch in the extension and be surprised the transactions are being signed by a different address then expected. @edgarkhanzadian

Related stuff also due to reaching API limits

The same name can be displayed with different account numbers.

The active account (what ever is shown in the extension modal) is displayed in the top of the popup to sign. But what should be shown is the account the dapp is connected with. And that should be the only account that can be signed with (as is the case with Stacks network).

[markmhendrickson]

@314159265359879 does this fix require an API implementation change on the app side, or is it wallet side purely?

[markmhendrickson]

Ping @314159265359879

[314159265359879]

@markmhendrickson This is partially wallet side, when it comes to the visual issue #4859 (comment) possibly fixed with #4923

However I believe that bitcoin issue (title) requires API implementation changes to prevent. It could be covered by the modern wallet API SIP if this is scoped in stacksgov/sips#166 (comment)

[pete-watters]

@314159265359879 is this still causing a problem or has it been fixed by #4923 ?

[314159265359879]

@pete-watters After a recent redesign you can nolonger see the account number/name on the approve transaction screen. So you can only know if you're signing with the right account after confirming.

The transaction is not signed. When I am signed in on Gamma with Account 1, and have Account 2 active in the extension. This is not fixed. The result is just different now.

(current) Reproduction steps
0. Switch extension to account 1

1. Sign in to gamma.io (you will sign in with the active account = account 1)

2. Then on the leather extension switch to account 2

3. Try to buy an ordinal or create an inscription: for example one of these https://gamma.io/prints/c5528f39379cbaa6380a38c129c2788d4ca97259f15ddf27e6b0bc3a4568a569i0?tab=presale

4. See this popup (without an account number or name)
   

5. Click confirm and see
   

I propose the following solutions:
0. short-term fix, let the user sign with the signed in account rather than the active account?

1. Warn users when they're not signed in on the right account, this likely requires additional API's: Missing listener: onAccountChanged #4891, Whenever there is a difference between the active account (on Leather extension) compared to what they are signed in with on the dapp, warn the user and ask for their intent (did they mean to sign with account 1 or account 2, then let them switch to it).

2. Let users choose an account when they connect to a bitcoin dapp (as we have with Stacks connections): Add option to switch accounts on requestAddresses screen #4182

3. Show the account name/number + addresses and balances on every screen especially on the approval screen because then users can identify if they're on the account intended. @mica000 @fabric-8

[markmhendrickson]

Show the account name/number + addresses and balances on every screen especially on the approval screen because then users can identify if they're on the account intended.

Let's implement this suggestion as a quick fix of sorts, reverting to the app nav's display of the account used for signing?

[markmhendrickson]

0. short-term fix, let the user sign with the signed in account rather than the active account?

As far as this solution, we already provide it. However, the app needs to indicate the "signed in account" while triggering API calls for signing transactions, and it seems Gamma isn't doing this. So we'd need them to upgrade their implementation to use that parameter?

[pete-watters]

I linked this bug as it relates to the same thing.

I will make a change so that instead of showing the logo on these screens we show the users account

[314159265359879]

I noticed the same issue on Luminex.io when signing a runes transfer transaction. Connected with account 1 on the dapp. Switched extension to account 2. And the transaction gets rejected on device. After switching back to account 1 I was able to sign the transfer transaction normally.

[314159265359879]

0. short-term fix, let the user sign with the signed in account rather than the active account?

As far as this solution, we already provide it. However, the app needs to indicate the "signed in account" while triggering API calls for signing transactions, and it seems Gamma isn't doing this. So we'd need them to upgrade their implementation to use that parameter?

@markmhendrickson want to add a link to docs where devs can read more about how to resolve this issue?

[markmhendrickson]

See https://leather.gitbook.io/developers/methods/getaddresses#accounts

[pete-watters]

I've made a small change here so that instead of showing the logo on the sign PSBT screen we show the users account and allow them to change the account.

We are still just showing the logo on the initial message signing screen:

Do I need to update that also? When implementing the global header, it was unclear how some headers should display so I may have made more mistakes. I believe it a good idea to audit these when working on approval UX

[markmhendrickson]

Do I need to update that also?

Yep! Let's update there, too 🙏 The general idea is that we should show the account in the top nav wherever the user is signing something (transaction or message).

[markmhendrickson]

@fabric-8 @mica000 could one of you review the previous comment that @pete-watters links here (#4371 (comment)) in case there requires any further design input on how the header details work?

[pete-watters]

@fabric-8 @mica000 could one of you review the previous comment that @pete-watters links here (#4371 (comment)) in case there requires any further design input on how the header details work?

Thanks @markmhendrickson . That comment is a bit outdated now so I will gather some info on the current state of things so we can review it. I've updated my PR so that it handles the first signing step also. I have started looking into moving the Test App into CodeSandbox so I think that could help here. Then we will be able to test things more easily.

My code works by configuring the Header to show items based on the RouteUrl.

Before we were showing account info only on:

• RouteUrls.TransactionRequest: /transaction
• RouteUrls.ProfileUpdateRequest: /update-profile
• RouteUrls.RpcSendTransfer: /send-transfer

I have updated it to also include:

• RouteUrls.RpcSignPsbt: /sign-psbt(That's the APPROVE TRANSACTION popup first reported here)
• RouteUrls.RpcSignBip322Message: /sign-bip322-message (That's the SIGN MESSAGE popup I later mentioned)

There are other places I'm not sure if we need to show it, for example on the summary / choose fee pages:

• RouteUrls.RpcSignPsbtSummary: /sign-psbt/summary
• RouteUrls.RpcSendTransferChooseFee: /send-transfer/choose-fee
• RouteUrls.RpcSendTransferConfirmation: /send-transfer/confirmation
• RouteUrls.RpcStacksSignature: /sign-stacks-message
• RouteUrls.SignatureRequest: /signature
• RouteUrls.RpcSendTransferSummary: /send-transfer/summary

[markmhendrickson]

There are other places I'm not sure if we need to show it, for example on the summary / choose fee pages:

Based on the route names, these six routes appear to pertain to screens during transaction or message-signing flows. So yes, let's show the account in the nav there, too.

The one exception may be the choose-fee route, since perhaps the nav should just say "Edit fee" or similar with back option.

[markmhendrickson]

These may be a good opportunity to ensure all of these relevant routes / views are included in our views database. If you could include a screenshot of each per entry, that'd help evaluate them.

[pete-watters]

These may be a good opportunity to ensure all of these relevant routes / views are included in our views database. If you could include a screenshot of each per entry, that'd help evaluate them.

Yes I think that's a good idea. I'll work on gathering that info and documenting it as I go. We have about 16 popup routes so it seems like we want to show that account info more often than not. I'll do an audit and take notes and we can decide on it.

[pete-watters]

@markmhendrickson - FYI I merged my code that updates the header for sign-psbt and sign-bip322-message.

I will audit the others but I opened a separate task for that here https://github.com/leather-io/issues/issues/157 .

That way we can get this more urgent improvement out first and fix the others once I have documented and tested them all