web icon indicating copy to clipboard operation
web copied to clipboard

[Spike] Ledger multi-account functionality

Open 0xean opened this issue 10 months ago • 4 comments

  1. why do we block user from adding additional accounts without TX history? Is this to help dumb dumbs from losing assets? if so, can we allow them to override? Screenshot 2024-04-09 at 3 07 52 PM

  2. Can we offer an account selection screen similiar to Rabby for ledger uses (would be nice to allow this for all chains / UTXOs) Screenshot 2024-04-09 at 3 08 21 PM

  3. Currently when we add a new account on a ledger, it adds it for that specific EVM.... can we instead add it for all EVMs. Said differently If an address is connected, we connect it for everything. Screenshot 2024-04-09 at 3 11 47 PM

Product is working on mocks to allow the rabby like selector flow, engineering should do a little research so we are aware of any limitations ahead of this implementation. So this ticket is mainly just prep to be able to help inform product decisions before we move forward with implementation.

0xean avatar Apr 09 '24 19:04 0xean

existing product spec for reference

https://www.notion.so/shapeshift/Ledger-Improvements-d72c7d4c53564d26b4f45917cc65ff05

0xean avatar Apr 09 '24 19:04 0xean

  1. why do we block user from adding additional accounts without TX history? Is this to help dumb dumbs from losing assets? if so, can we allow them to override?

That one is an historical decision - we use this as heuristics to know when to stop fetching account, see https://github.com/shapeshift/web/pull/2702/files#diff-68fa73b2eac2fc2d43c7e89f845090ff6f7e55830383e855b59502c227702d9bR82

And matching selector: https://github.com/shapeshift/web/blob/d3f42415bbc6cfb41fe8fb0425b741e04709bac6/src/state/slices/txHistorySlice/selectors.ts#L201-L208

The main reason for this was auto-discovery, so we had a way to short-circuit fetching the next account number. Though we can probably do a lot better here heuristics-wise.

2. Can we offer an account selection screen similiar to Rabby for ledger uses (would be nice to allow this for all chains / UTXOs)

We absolutely could - not only for Ledger - and that's definitely interlinked to the above. If we have sane heuristics on the number of accounts we "discover", then we could do something similar. If we're only going to display the addresses without any e.g balance, then that would be straightforward and wouldn't be a performance bottleneck. As soon as we want to display (hence fetch) balances in this screen, then we'll have to keep perf in mind and probably come up with heuristics (e.g pagination, fetch and display 0-5 first then allow users to load next).

  1. Currently when we add a new account on a ledger, it adds it for that specific EVM.... can we instead add it for all EVMs. Said differently If an address is connected, we connect it for everything.

Also not Ledger-specific, and the same current heuristics of only fetching accounts with activity (meaning stopping fetching accounts as soon as one has no Tx history). Accounts on diff. chains are entirely diff. entities and we have no notion of fetching the same account numbers for all EVM, this is decoupled.

I believe this would be a nice addition as well - it could be implemented relatively easily currently, though we probably want to wait and see whether or not we rip the current "Add Account" / automagical discovery of account numbers before doing so.

gomesalexandre avatar Apr 09 '24 22:04 gomesalexandre

thanks @gomesalexandre

cc @twblack88 do you have enough info here to spec out the desired UX with mocks further and then we can take it back to engineering for implementation. Seems like there really isnt any technical blockers here, just old ways of doing things that we can invest the time into fixing

0xean avatar Apr 10 '24 17:04 0xean

Yeah we got the working draft here @reallybeard ~wanted to make~ made into prototype to communicate idea/flow

Onboard with fetching only 5 accounts @gomesalexandre that seems sane.

twblack88 avatar Apr 10 '24 17:04 twblack88