Feat/multiple function access keys #156

petersalomonsen · 2023-11-25T08:44:22Z

Preview URL:

https://psalomobos.near.page/devhub.near/widget/app

Or see a demonstration video here:

https://youtu.be/CuwWv1zAKf4?si=lBUi9kXxRrhVjv-s

resolves #148 and NEAR-DevHub/neardevhub-bos#137

frol · 2023-11-25T09:57:27Z

@petersalomonsen I don't understand what is the intended flow here. I don't see "don't ask me again" checkbox, when I tried to "like" on DevHub, there was a popup prompt that redirected me to the wallet (MNW) with some unparsable transaction confirmation (it tried to display it as a function call confirmation but there was no function call). Please, record a screencast as otherwise I don't even understand how you see it can work.

petersalomonsen · 2023-11-25T11:59:14Z

Sorry @frol I must have missed something when I published the first version. Unfortunately what you experienced was that you were not redirected to the wallet for the add_key transaction, but instead just sent for the add_like. This is because of a missing await for the add_key transaction, and I've corrected this now.

Here's a demonstration video as requested: https://youtu.be/0oG9_-KEi0Q?si=BEeUYz5E7wQApWNm

frol · 2023-11-26T15:45:22Z

@petersalomonsen This is a nice proof of concept, and I believe we should push it forward. Yet, there are a number of UX tweaks that need to be implemented:

Instead of the browser prompt, there should be just a "Don't ask me again" checkbox similar to the SocialDB interactions on the BOS modal popup
Use a regular Wallet login flow page. Using a generic transaction signing page does not provide relevant UX to the users:

I would expect to see:

3. Avoid refreshing the page after sending the transaction (once the access key is already there) - similarly to how it is done for SocialDB interactions (like on the NEAR Social feed does not refresh the page)

petersalomonsen · 2023-11-26T17:04:36Z

Thanks @frol , good to hear that we can move forward on this. I agree to all of your 3 points, and I've started working on the UX experience ( not using a browser prompt ).

As you can see in the screenshot below, from my local dev environment, there's a checkbox to toggle if you want to create a function access key ( we can consider naming this "Don't ask me again", as you propose ):

When you check that toggle, I'm now trying to include the function access key transaction, so that you don't loose the like or post you are about to confirm. So as you can see in the screenshot below, when checked, another transaction, the "addKey" is also showing.

Not sure yet how to fix it so that the wallet says that "an application is requesting limited access to your account", but I'll try to figure that out.

Also totally agree that we should not reload the page, but right now I think this involves work on the DevHub code to handle transaction feedback. Everything we have right now is based on the fact that the page is reloading ( after a completed transaction ). I'll see what I can do there, but it might have to be a separate issue for DevHUB after this feature for creating function access keys is merged to the VM codebase.

Let me know what you think

petersalomonsen · 2023-11-26T19:31:14Z

@frol @evgenykuzyakov I've done some UX tweaks now so that the adding of a new access key is part of the regular VM confirmation popup. Just as in the screenshots from the previous comment. Note that you only should tick the checkbox once, for the next calls you already have an access key stored, so you don't need to create it again. This might be confusing, so I will figure out how to make this more clear in the UI. "Don't ask again" might also be confusing since it could be misunderstood as you would not get the VM confirmation popup either.

src/lib/data/near.js

petersalomonsen · 2023-11-27T19:59:14Z

UX is improving bit by bit, so if you already have an access key stored, the popup will open with the checkbox checked, and also indicating which key that will be attempted for signing the transaction:

frol · 2023-11-28T07:48:37Z

"Don't ask again" might also be confusing since it could be misunderstood as you would not get the VM confirmation popup either.

@petersalomonsen That is how SocialDB interactions are done - they allow “don’t ask again” for the specific component interacting with SocialDB. From UX perspective, I believe it makes sense. Obviously, you don’t want to allow any component to interact with a contract on behalf of the user just because there is already an access key, but it doesn’t make a lot of sense to confirm every like with the popup either. Take inspiration from Social.set implementation

petersalomonsen · 2023-11-29T10:23:50Z

"Don't ask again" might also be confusing since it could be misunderstood as you would not get the VM confirmation popup either.

@petersalomonsen That is how SocialDB interactions are done - they allow “don’t ask again” for the specific component interacting with SocialDB. From UX perspective, I believe it makes sense. Obviously, you don’t want to allow any component to interact with a contract on behalf of the user just because there is already an access key, but it doesn’t make a lot of sense to confirm every like with the popup either. Take inspiration from Social.set implementation

Thanks @frol , that's a good point. I'll look into how Social.set is solved, and try avoiding the VM confirmation popup as well.

ailisp · 2023-12-01T09:52:00Z

Looks great! I would also suggest to have this checked by default:

And I have a question when it's run out of function call key allowance, what will happen?

frol · 2023-12-01T15:09:37Z

@petersalomonsen On a general note, please, put yourself in the shoes of a regular user, they don't know about "access key" and why they would want to create one.

petersalomonsen · 2023-12-01T16:40:04Z

yes @frol . I'm moving forward in the direction you proposed with "Don't ask again", and not even showing the confirmation popup in the VM. @ailisp When the key runs out of allowance, we should detect that and pop up the confirmation dialogs again. It might be good to provide some optional detailed information for the user to explain what is happening behind the scenes.

petersalomonsen · 2023-12-01T22:27:05Z

@frol @ailisp See the updated progress. Now it's using the regular wallet login, it's executing the "pending" transaction after login. After that there is no need to confirm when posting or liking. Just for now I've added a small popup ( just for information ), that shows that there is a transaction going on ( not sure how we want to solve this yet ).

Also see the new demo video of the flow: https://youtu.be/CuwWv1zAKf4?si=lBUi9kXxRrhVjv-s

The preview URL also contains the latest version.

What's remaining, as I see is:

Try to avoid reloading after the transaction
Some indication on what's going on during sending the "Pending" transaction ( after login )
Handling out of allowance
Handling deleted function access keys ( that are not deleted in localstorage, but deleted on chain )

petersalomonsen · 2023-12-02T19:37:26Z

Now the page does not reload after the transaction.

petersalomonsen · 2023-12-02T19:54:21Z

src/lib/data/near.js

@@ -111,33 +172,42 @@ async function accountState(near, accountId) {
 }

 async function sendTransactions(near, functionCalls) {
+  if (functionCalls.length == 1) {
+    const { contractName, methodName, args, gas, deposit } = functionCalls[0];
+    //if (deposit == Big(0)) {


This check needs to be in place before merging, and also include the gas from the original transaction

petersalomonsen · 2023-12-02T19:54:53Z

src/lib/data/near.js

        if (
          transactions[transactions.length - 1]?.receiverId !== contractName ||
          newTotalGas.gt(MaxGasPerTransaction)
        ) {
          transactions.push({
            receiverId: contractName,
-            actions: [],
+            actions: actions ?? [],


actions is not used. should be removed

petersalomonsen · 2023-12-02T19:55:21Z

src/lib/data/near.js

          });
          currentTotalGas = gas;
        } else {
          currentTotalGas = newTotalGas;
        }
-        transactions[transactions.length - 1].actions.push(action);
+        if (!actions) {


there will not be actions here.

petersalomonsen · 2023-12-02T19:56:02Z

src/lib/data/near.js

+          contractId: contractName,
+          methodName,
+          args,
+          //gas


need to include gas from the original transaction

petersalomonsen · 2023-12-02T19:58:14Z

These are remaining, but could maybe be separate issues?

Some indication on what's going on during sending the "Pending" transaction ( after login )
Handling out of allowance
Handling deleted function access keys ( that are not deleted in localstorage, but deleted on chain )

frol · 2023-12-03T09:43:33Z

Some indication on what's going on during sending the "Pending" transaction ( after login )

Well, given that Social.set also lacks any indication on Pending transaction, indeed, we can address it across the board in a separate issue.

Handling out of allowance
Handling deleted function access keys ( that are not deleted in localstorage, but deleted on chain )

Social.set also does not handle it gracefully either. Let's address these in separate issues, indeed. Please, create follow-up issues.

frol · 2023-12-03T09:57:36Z

After that there is no need to confirm when posting or liking.

I hope it only whitelists interaction with the contract only for the given BOS component. We don't want random components on BOS to be able to interact with devhub contract without user's acknowledgement. Again, see Social.set behavior.

(Also, let's use the same UI layout and message in both dialogs if possible)

Just for now I've added a small popup ( just for information ), that shows that there is a transaction going on ( not sure how we want to solve this yet ).

Let's follow Social.set, it does not have any indication on BOS level, but exposes onCommit API (#105)

petersalomonsen · 2023-12-03T12:11:53Z

I hope it only whitelists interaction with the contract only for the given BOS component. We don't want random components on BOS to be able to interact with devhub contract without user's acknowledgement. Again, see Social.set behavior.

That's a good point. I did not think of that.

So that's a remaining issue then before this can be merged, but I will look into it.

petersalomonsen · 2023-12-04T18:04:48Z

Now I've fixed so that only interaction for the given BOS component is whitelisted, and you also only whitelist calling a specific method on the smart contract:

Given all the changes, I do feel the need for setting up a testing framework for this now. Are there any automatic tests of the VM anywhere?

frol · 2023-12-04T19:49:23Z

@petersalomonsen I see it is moving in the right direction and we are close to completion. Please, review your PR yourself, see if you can re-use any parts from Social.set implementation, and mark it as ready for review

petersalomonsen · 2023-12-04T20:18:18Z

Yes, the only thing I would like to see resolved before moving to ready for review is to remove deposit and set a lower gas amount in the devhub widgets as described here: NEAR-DevHub/neardevhub-bos#549

Then I will be able to remove the overriding of deposit / gas that I had to do for demo purposes as you can see here:
https://github.com/NearSocial/VM/pull/156/files#diff-cc75698545c1506b569386ec69e64f50d829fc759a7fb175b256b88630782859R177

I will also go over the code a few more times and look for reusing of existing code.

evgenykuzyakov · 2023-12-07T23:25:09Z

Overall, it's the right direction. Not sure about 2 things:

the TransactionResultContext implementation
pending transactions

frol · 2023-12-21T13:41:18Z

@petersalomonsen Thanks for the thorough testing.

Sender does not support adding access keys in a transaction, and does not have a wallet url for near-api-js to redirect too, resulting in that the "Don't ask again" feature is not supported, and an error message is displayed instead.

Well, Sender has a different operation model and it breaks UX even for SocialDB interactions requiring explicit confirmation of every single transaction, so we cannot do anything here.

I've tested with Meteor wallet now, and since the wallet selector interfaces does not support signing into multiple contracts, and since Meteor wallet does not have a walletUrl, the requestSigninmethod of near-api-js will not work either.

This is unexpected given that MeteorWallet is a browser wallet.

Not sure how to have the requestSignin experience also for MeteorWallet without modifying the wallet selector. I could do another PR for the wallet selector later to add support for this.

I expected all the wallets (except Sender) to support the most standard login flow :-(

This rabbit hole is too deep. Please, create an issue on the Wallet Selector repo first and share it in the "NEAR Wallet Builder Group"

frol · 2023-12-21T13:43:07Z

@evgenykuzyakov I think this current state already brings reasonable improvements to UX even if with some UX glitches on some of the wallets, so I suggest finishing the review and merging it as it is now and improving iteratively.

petersalomonsen · 2023-12-22T04:58:09Z

@frol Thanks for the feedback

This rabbit hole is too deep. Please, create an issue on the Wallet Selector repo first and share it in the "NEAR Wallet Builder Group"

That is done now: near/wallet-selector#1045

petersalomonsen · 2023-12-29T18:50:12Z

Just discussed with @Megha-Dev-19 that Near.call does not return anything today. This is already an issue with the wallets that does not redirect, but either open a separate window or extension for confirming the transaction. A widget does not have any way to know when a transaction is finished, or the result of it.

With this PR, the cache will be invalidated when the transaction is finished. This helps a lot, since it will trigger re-rendering. It would be even better though, if Near.call returned a promise with the result of the transaction.

Should I add a promise result to Near.call @evgenykuzyakov @frol ?

Megha-Dev-19 · 2023-12-30T08:28:26Z

thanks @petersalomonsen for adding this, it helped me fixed other issues as well, now I can initiate an action after the user approves/reject the txn since the cache is invalidated.
@frol Would really appreciate if we can get this merged asap 🙏

frol · 2023-12-31T10:16:12Z

It would be even better though, if Near.call returned a promise with the result of the transaction.
Should I add a promise result to Near.call @evgenykuzyakov @frol ?

I think we should mimic Social.set behavior with the onCommit and onCancel callbacks as you suggested in #105. I would make it a separate PR.

src/lib/components/ConfirmTransactions.js

petersalomonsen · 2023-12-31T11:52:59Z

It would be even better though, if Near.call returned a promise with the result of the transaction.
Should I add a promise result to Near.call @evgenykuzyakov @frol ?

I think we should mimic Social.set behavior with the onCommit and onCancel callbacks as you suggested in #105. I would make it a separate PR.

Thanks for reminding me, I had completely forgotten that I wrote that. There is already a PR for that also.

frol

Looks good to me.

@evgenykuzyakov please, review once again

evgenykuzyakov

Looks good. Minor comments

src/lib/components/ConfirmTransactions.js

evgenykuzyakov · 2024-01-04T00:00:53Z

src/lib/data/near.js

+async function createWalletConnectionForContract(near, contractId) {
+  const keyStore = getKeyStoreForContract(contractId);


We should probably check somewhere that the contractId doesn't match social.near (for the current network) to avoid some random issues.

There is already such a check on functionCall ( see https://github.com/NearSocial/VM/pull/156/files#diff-cc75698545c1506b569386ec69e64f50d829fc759a7fb175b256b88630782859R179 )

Do we check for this in the ConfirmTransaction?

No. And I believe if call a function on social.near, you might be offered don't ask again. This could be solved by also adding this check to eligibleForDontAskAgain.

Do we want to not offer the don't ask again feature for calls to near.social?

Yeah, let's not give this option for social.near.

src/lib/components/Widget.js

petersalomonsen · 2024-01-04T18:57:10Z

In the latest commit, there's also a try/catch when sending the pending transaction, so that if the transaction fails, the page will still be loaded ( without this, the page would just be blank, and reloading would not help ).

evgenykuzyakov · 2024-01-04T22:58:59Z

Please rebase to dev branch

petersalomonsen · 2024-01-05T05:43:21Z

Please rebase to dev branch

Rebased to upstream/dev, and also a quick re-test. Also updated preview environment with the latest rebased: https://psalomobos.near.page/devhub.near/widget/app?page=post&id=2688

evgenykuzyakov · 2024-01-05T21:48:22Z

Let's remove don't ask again for social.near

when sending transactions, offer to not ask for confirmation later for transactions calling the same contract and method

petersalomonsen · 2024-01-06T05:02:29Z

Let's remove don't ask again for social.near

OK this is now implemented in the latest push, and can be tested here, where you see that calling social.near does not present the "Don't ask again" option, while calling another contract does.

petersalomonsen changed the title ~~Feat/multiple funciton access keys~~ Feat/multiple function access keys Nov 25, 2023

petersalomonsen mentioned this pull request Nov 25, 2023

Avoid a need to confirm every DevHub post/like through a Wallet NEAR-DevHub/neardevhub-bos#137

Closed

kujtimprenkuSQA reviewed Nov 27, 2023

View reviewed changes

src/lib/data/near.js Outdated Show resolved Hide resolved

petersalomonsen mentioned this pull request Nov 28, 2023

Feat/137 avoid wallet confirmation NEAR-DevHub/neardevhub-bos#488

Closed

petersalomonsen mentioned this pull request Nov 29, 2023

Editing community leads to a 400 error message NEAR-DevHub/neardevhub-bos#491

Closed

petersalomonsen mentioned this pull request Dec 2, 2023

Deposit should be zero and gas should not exceed allowance or max gas for a transaction NEAR-DevHub/neardevhub-bos#549

Closed

petersalomonsen commented Dec 2, 2023

View reviewed changes

petersalomonsen force-pushed the feat/multiple-funciton-access-keys branch from ea1823f to ede36f7 Compare December 6, 2023 16:28

petersalomonsen marked this pull request as ready for review December 6, 2023 16:51

petersalomonsen mentioned this pull request Dec 22, 2023

Provide standard login flow for multiple contracts near/wallet-selector#1045

Open

petersalomonsen mentioned this pull request Dec 30, 2023

auto close after posting NEAR-DevHub/neardevhub-bos#616

Closed

frol reviewed Dec 31, 2023

View reviewed changes

src/lib/components/ConfirmTransactions.js Outdated Show resolved Hide resolved

frol reviewed Dec 31, 2023

View reviewed changes

src/lib/components/ConfirmTransactions.js Outdated Show resolved Hide resolved

frol approved these changes Jan 2, 2024

View reviewed changes

Megha-Dev-19 mentioned this pull request Jan 3, 2024

Reuse PostEditor instead of Create component NEAR-DevHub/neardevhub-bos#615

Merged

evgenykuzyakov reviewed Jan 4, 2024

View reviewed changes

petersalomonsen force-pushed the feat/multiple-funciton-access-keys branch from 39a70ed to 24d3521 Compare January 5, 2024 05:41

petersalomonsen changed the base branch from master to dev January 5, 2024 05:44

feat: don't ask again for similar transactions

895a294

when sending transactions, offer to not ask for confirmation later for transactions calling the same contract and method

petersalomonsen force-pushed the feat/multiple-funciton-access-keys branch from 24d3521 to 895a294 Compare January 6, 2024 04:39

evgenykuzyakov approved these changes Jan 8, 2024

View reviewed changes

evgenykuzyakov merged commit 6bb10d1 into NearSocial:dev Jan 8, 2024
1 check passed

This was referenced Jan 14, 2024

Migrate to dev VM to enable multiple access keys NEARBuilders/gateway#53

Closed

Migrate to dev VM to enable multiple access keys near-everything/viewer#40

Open

frol mentioned this pull request Jan 28, 2024

add function access keys to additional contracts by using multiple key stores near/wallet-selector#887

Closed

		async function createWalletConnectionForContract(near, contractId) {
		const keyStore = getKeyStoreForContract(contractId);

Feat/multiple function access keys #156

Feat/multiple function access keys #156

Conversation

petersalomonsen commented Nov 25, 2023 • edited Loading

frol commented Nov 25, 2023

petersalomonsen commented Nov 25, 2023

frol commented Nov 26, 2023

petersalomonsen commented Nov 26, 2023

petersalomonsen commented Nov 26, 2023

petersalomonsen commented Nov 27, 2023

frol commented Nov 28, 2023

petersalomonsen commented Nov 29, 2023

ailisp commented Dec 1, 2023

frol commented Dec 1, 2023

petersalomonsen commented Dec 1, 2023 • edited Loading

petersalomonsen commented Dec 1, 2023 • edited Loading

petersalomonsen commented Dec 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petersalomonsen commented Dec 2, 2023

frol commented Dec 3, 2023

frol commented Dec 3, 2023

petersalomonsen commented Dec 3, 2023

petersalomonsen commented Dec 4, 2023

frol commented Dec 4, 2023

petersalomonsen commented Dec 4, 2023

evgenykuzyakov commented Dec 7, 2023

frol commented Dec 21, 2023

frol commented Dec 21, 2023

petersalomonsen commented Dec 22, 2023 • edited Loading

petersalomonsen commented Dec 29, 2023

Megha-Dev-19 commented Dec 30, 2023

frol commented Dec 31, 2023

petersalomonsen commented Dec 31, 2023

frol left a comment

Choose a reason for hiding this comment

evgenykuzyakov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petersalomonsen commented Jan 4, 2024

evgenykuzyakov commented Jan 4, 2024

petersalomonsen commented Jan 5, 2024

evgenykuzyakov commented Jan 5, 2024

petersalomonsen commented Jan 6, 2024

petersalomonsen commented Nov 25, 2023 •

edited

Loading

petersalomonsen commented Dec 1, 2023 •

edited

Loading

petersalomonsen commented Dec 1, 2023 •

edited

Loading

petersalomonsen commented Dec 22, 2023 •

edited

Loading