feat: Add Tokenary wallet

[grachyov]

Description

Add Tokenary Safari extension support.

Tokenary implements an API almost identical to Sender wallet.

For example, it works with https://app.burrow.cash in Safari when you enable Tokenary extension and select Sender wallet from wallet selector.

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. This type of change is the main reason for the PR.

• [x] FEATURE - a PR of this type introduces a new feature.

Breaking changes

• [x] NO BREAKING CHANGE - this PR doesn't contain any breaking changes and it's backwards compatible

[grachyov]

Hello @kujtimprenkuSQA @lewis-sqa

Could you please take a look? 🙌

[lewis-sqa]

Hey @grachyov,

I've done some testing with Tokenary this morning and found a few problems with it's current implementation:

• Imported wallets with named account ids (test.testnet) aren’t working as it assumes implicit account ids.
• I can’t seem to change the network to testnet.
• signAndSendTransaction resolves even when there’s an error. What’s the reason it doesn’t reject instead?
• It declares the isSender flag as well as isTokenary which will confuse dApps.
• isSignedIn returns true before I’ve interacted with the dApp.
• It populates the accountId with my imported account before I’ve signed in.
• signOut doesn’t seem to clear the accountId or isSignedIn status.

[grachyov]

Hey @lewis-sqa, thanks for the detailed feedback!

• Is there only support for FunctionCall Actions? — Yes, only FunctionCall actions are supported right now.

• Are the error messages exactly the same? — No. Are dapps relying on these?

• Are all the events applicable? — Do you mean emitting signIn, signOut, accountChanged and rpcChanged?

• How is sign in and sign out handled? Does it delete the key upon sign out? — I assumed from this line and from using dapps, that responding with true without actually creating an AccessKey is enough right now. Should we create it anyways?

• Imported wallets with named account ids (test.testnet) aren’t working as it assumes implicit account ids. — Would it be necessary for the initial integration? If I import an account from Trust wallet, I would actually expect an implicit account id since it is displayed this way in Trust wallet.

• I can’t seem to change the network to testnet. — Tokenary does not support testnet yet. Should we add it?

• signAndSendTransaction resolves even when there’s an error. What’s the reason it doesn’t reject instead? — I assumed that this is how Sender handles an error and we should do it the same way. Would reject be better here?

• It declares the isSender flag as well as isTokenary which will confuse dApps. — We've added it to be able to connect Tokenary by selecting Sender option while integration is not completed. Going to remove isSender flag when the integration is ready to be merged.

• isSignedIn returns true before I’ve interacted with the dApp. It populates the accountId with my imported account before I’ve signed in. — As I mentioned, we do not actually create AccessKey and just return accountId and true for isSignedIn right after user selects an account in a popup. Should we fix this behaviour?

• signOut doesn’t seem to clear the accountId or isSignedIn status. — Will fix it.

[lewis-sqa]

@grachyov, apologies for the delayed response:

Are the error messages exactly the same? — No. Are dapps relying on these?

Sadly, Wallet Selector depends on the messages to identify cancel scenarios (e.g. Sender): https://github.com/near/wallet-selector/blob/main/packages/sender/src/lib/sender.ts#L73

Are all the events applicable? — Do you mean emitting signIn, signOut, accountChanged and rpcChanged?

Yeah that's right. Does Tokenary support all of these and with the same event payloads? You mentioned Tokenary only supports mainnet so I'm guessing rpcChanged isn't applicable?

How is sign in and sign out handled? Does it delete the key upon sign out? — I assumed from this line and from using dapps, that responding with true without actually creating an AccessKey is enough right now. Should we create it anyways?

Until a standard officially exists you're right, though "sign in" generally means creating a FunctionCall access key for a dApp's smart contract in NEAR which is why it accepts a contractId and (optionally) methodNames. There are cases where these limited access keys aren't applicable such as dApps that don't make any gas-only function call transactions (but again a standard such as the one referenced will eventually address this problem).

Imported wallets with named account ids (test.testnet) aren’t working as it assumes implicit account ids. — Would it be necessary for the initial integration? If I import an account from Trust wallet, I would actually expect an implicit account id since it is displayed this way in Trust wallet.

I'm not an expert on this recent change in NEAR but the current integration doesn't work when a user has a named account. For instance, Tokenary thinks my lewis-sqa-2.testnet account is 80d028da4035d532519c081d71901b19e4ded4f3eb0713c37fad2bd0f2d5c7ec.

I can’t seem to change the network to testnet. — Tokenary does not support testnet yet. Should we add it?

If you could add support for this network, it would greatly help us test the integration 🙂

signAndSendTransaction resolves even when there’s an error. What’s the reason it doesn’t reject instead? — I assumed that this is how Sender handles an error and we should do it the same way. Would reject be better here?

You're right, though (in my honest opinion) I don't think Sender is a great example of good practices with JavaScript, particularly in this case where a core concept of Promises is effectively ignored 🙁

It declares the isSender flag as well as isTokenary which will confuse dApps. — We've added it to be able to connect Tokenary by selecting Sender option while integration is not completed. Going to remove isSender flag when the integration is ready to be merged.

Ah, that makes sense. No problem 👍

isSignedIn returns true before I’ve interacted with the dApp. It populates the accountId with my imported account before I’ve signed in. — As I mentioned, we do not actually create AccessKey and just return accountId and true for isSignedIn right after user selects an account in a popup. Should we fix this behaviour?

The problem I had was I don't recall selecting an account (maybe because I only imported one?), but the object had a reference to the account I imported. Basing behaviour on Math Wallet (which is probably more similar to the approach you're heading towards in terms of no FunctionCall access key support), I'd expect these fields to be populated only once I've called signIn

signOut doesn’t seem to clear the accountId or isSignedIn status. — Will fix it.

Excellent 🎉

[grachyov]

@lewis-sqa, thanks for your response! I'll add changes to Tokenary and be back soon.

[AmmarHumackicSQA]

Hey @grachyov , when can we expect these changes as we planned it for our next release? Thanks!

[janewang]

Hey @grachyov - is this a correct email to reach out to you? [email protected]

[grachyov]

Hey @AmmarHumackicSQA , I am not quite sure yet.

[grachyov]

Hey @janewang , this one or [email protected] is good.

[janewang]

@grachyov Please kindly confirm that the following criteria is met or can be met within the required timeline. Thank you.

Criteria for Including New Wallets for Wallet Selector

Product Criteria:

A wallet project must have comply the following product criteria to listed on Wallet Selector.

• Non-custodial: The user controls their funds.
• Conformity to Wallet standards: The wallet product conforms to NEAR NEP wallet standards (Injected Wallet https://github.com/near/NEPs/pull/370 and Bridged Wallet https://github.com/near/NEPs/pull/368)
• Ease of use: The wallet product provides a usable interface for the end user. Please provide a user guide for review.
• Ability to recover accounts: The wallet product allows users to be able to recover accounts.
• Actively maintained: The wallet is actively maintained by a team and can answer user queries.

Security Criteria:

A wallet project must have the majority of these security program features in place with self-certification that remaining items will be in place within 6 months of being listed.

Wallets shall checkbox a statement of compliance to be maintained on the wallet’s github account. When asserting that open items will be in place within six months of being listed, target dates shall be included. The overall statement must be dates and the date shall be commensurate with the commit date. All wallet projects must maintain compliance. If a wallet is no longer in compliance, or no longer supported, the security statement must reflect that change. The wallet project shall make verification of the following requirements as easy as possible maximizing transparency through the use of relevant links pointing toward program descriptions, audit reports, etc. for the user/researcher.

• Has a security program in place that covers or is dedicated to the wallet
• Publishes information about its security program in an easily findable place
• Conducts regular audits of wallet code, at regular intervals of less than a year or based on meaningful code changes
• Conducts regular penetration tests, both “authenticated” and “non-authenticated” upon significant code changes
• Conducts penetration tests on related infrastructure, such as databases, virtual machines, web servers, etc.
• Remediates any critical, high, or medium findings from audits (3, 4, an 5 above) and tests in a rapid fashion, as suggested by auditors and penetration testers–with auditors validating the remediation in their reports.
• Makes such reports (audits, penetration tests) publicly available, on at least a summary level
• When making reports (7) available, wallet projects should ensure the equivalent reports appear on the security vendor’s site or simply links to the security vendor’s report. This ensures the authenticity of the audit reports.
• Conducts operational readiness reviews and testing or an equivalent process before deployment to production to ensure that code changes have not resulted in unanticipated behavior, compatibility issues, or in
• Maintains a testnet wallet
• Maintains a bug bounty program
• Implements minimal privilege and access policies with regard to supporting infrastructure
• Implements MFA and strong passwords for access to critical related systems, such as domain registration, hosting platforms, cloud platforms, etc.
• Conducts known vulnerability and vulnerable dependency checks; and remediates critical, medium, and high findings before bringing into testnet or production.
• If the wallet is a browser “extension,” it is listed on the official extension marketplace

[AmmarHumackicSQA]

Hey @grachyov , can you update us on the progress regarding the mentioned changes and confirm that the criteria are met or can be met within the required timeline? Thanks!

[AmmarHumackicSQA]

Hey @grachyov , can you update us on the progress regarding the mentioned changes and confirm that the criteria are met or can be met within the required timeline? Thanks!

Hi @grachyov any news on this? Thanks.

[AmmarHumackicSQA]

Hey @grachyov , can you update us on the progress regarding the mentioned changes and confirm that the criteria are met or can be met within the required timeline? Thanks!

Hi @grachyov any news on this? Thanks.

Hey @grachyov should we leave this PR open, and can you share more info regarding the status of the work you are doing?

[AmmarHumackicSQA]

@grachyov gentle bump 🙂

[DamirSQA]

Hey @grachyov , any news on this one?

[AmmarHumackicSQA]

Hey, @grachyov can you let us know what is the status regarding this PR? We might need to close it soon if we don't get any response. Thanks