wallet-selector icon indicating copy to clipboard operation
wallet-selector copied to clipboard

feat: Add `signInMulti` method

Open kujtimprenkuSQA opened this issue 2 years ago • 8 comments

Description

This PR is about adding the signInMulti method to the public API of the wallets in Wallet Selector based on https://github.com/near/NEPs/pull/428 .

  • Added interfaces in wallet.types for the signInMulti.
  • Added signInMulti in each wallet, it initially throws when calling this method.
  • Added logic in the core package to allow sign-in with multiple contracts.
  • Updated modal-ui and modal-ui-js to pass the list of contracts to the signInMulti method.
  • Added docs for signInMulti.

Wallets that support signInMulti in this PR

  • none

Checklist:

  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings

Type of change.

  • [ ] FIX - a PR of this type patches a bug.
  • [x] FEATURE - a PR of this type introduces a new feature.
  • [ ] BUILD - a PR of this type introduces build changes.
  • [ ] CI - a PR of this type introduces CI changes.
  • [ ] DOCS - a PR of this type introduces DOCS improvement.
  • [ ] STYLE - a PR of this type introduces style changes.
  • [ ] REFACTOR - a PR of this type introduces refactoring.
  • [ ] PERFORMANCE - a PR of this type introduces performance changes.
  • [ ] TEST - a PR of this type adds more tests.
  • [ ] CHORE - a PR introduces other changes than the specified above.

kujtimprenkuSQA avatar May 19 '23 12:05 kujtimprenkuSQA

Hi @kujtimprenkuSQA @MaximusHaximus What is the status of this PR? Our team is recently tried to add a relevant feature and discovered this. WIP branch. I wonder if we can move this forward

ailisp avatar Aug 18 '23 06:08 ailisp

Hi @kujtimprenkuSQA @MaximusHaximus What is the status of this PR? Our team is recently tried to add a relevant feature and discovered this. WIP branch. I wonder if we can move this forward

Hi, this PR is not ready to be merged yet as we still need this additional PR https://github.com/near/wallet-selector/pull/847 related to the comment https://github.com/near/wallet-selector/pull/811#discussion_r1242130459 to be reviewed and make a decision if we should go with it or not since it will introduce breaking changes in Wallet Selector.

As of right now, we are not aware if any wallet supports the signInMulti. For browser wallets I think this update in NAJ is needed: https://github.com/near/near-api-js/issues/1153 there was some work done related to it: https://github.com/near/near-api-js/pull/1163

kujtimprenkuSQA avatar Aug 18 '23 08:08 kujtimprenkuSQA

OK, so maybe I may move forward with my experiments in here then: https://github.com/near/wallet-selector/pull/887 ?

This approach does not require change in wallets, and does not introduce any breaking change to the wallet selector. It simply offers the option to add a function access key to another contract, and store it in a separate keystore ( near-api-js supports multiple keystore with different prefixes ).

As you can see from the test case there's a new method addContractConnection that you can call to add a function access key to an additional contract and store it locally.

For the particular issue for NEAR DevHub the idea is then to create the access key for the devhub contract when you submit something for the first time, and then reuse that access keys for later calls. This way, the user could also authenticate with other contracts on demand.

cc @ailisp @frol

petersalomonsen avatar Aug 18 '23 15:08 petersalomonsen

See a proof of concept demo video here ( for the wallet-selector guestbook ):

https://www.youtube.com/watch?v=ocZKP2-Lxes

petersalomonsen avatar Aug 20 '23 11:08 petersalomonsen

@petersalomonsen the demo is very cool! I wonder how can we invoke wallet.addContractConnection from the DevHub widget, since there is no BOS api to access wallet directly

ailisp avatar Sep 04 '23 02:09 ailisp

@petersalomonsen the demo is very cool! I wonder how can we invoke wallet.addContractConnection from the DevHub widget, since there is no BOS api to access wallet directly

@ailisp thanks, and actually you don't need to invoke that from the DevHub widget. This should be handled by the nearsocial viewer ( or maybe VM ) so that it detects if there's a wallet redirection, and then offers to add a function access key for that contract. Here's an example of how I've added this to the viewer for my PoC with the DevHub widget: https://github.com/NearSocial/viewer/pull/189/files

And this is also what's used in the demo you can see here: https://youtu.be/DVoZK5CJ2uE

petersalomonsen avatar Sep 04 '23 16:09 petersalomonsen

What is the plan for this feature moving forward? Any timeline estimates or prediction of when it could be available?

petarvujovic98 avatar Sep 21 '23 09:09 petarvujovic98

What is the plan for this feature moving forward? Any timeline estimates or prediction of when it could be available?

The initial work for adding the signInMulti into the wallet-selector has been done in this PR, but to address the comment left here https://github.com/near/wallet-selector/pull/811#discussion_r1242130459 there was a need to make some additional changes https://github.com/near/wallet-selector/pull/847 that would be breaking changes in wallet-selector.

We still need a review from Pagoda and further discuss this implementation before it's merged.

As mentioned in the above comments the currently available keystores will have to catch up in order to support multiple keys per account https://github.com/near/near-api-js/issues/1153 there was some work done related to it: https://github.com/near/near-api-js/pull/1163

kujtimprenkuSQA avatar Sep 22 '23 10:09 kujtimprenkuSQA