Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add wallet-rpc-provider to support various extension wallets #994

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Yoon-Suji
Copy link

Motivation

To provide a universal provider that can be used by NEAR-supported extension wallets.

Description

  • Add near-api-js/packages/near-api-js/src/providers/wallet-rpc-provider.ts
  • Can detect chainChanged event when a network change inside the chain.
  • Can detect accountsChanged event when an account change in wallet.
  • In sendJsonRpc method, it receives transaction hash response in array type.
  • You can implement the WalletProvider interface according to the wallet you want to use.

Checklist

  • Add some methods to communicate with extension wallet.
  • Add WalletProvider interface abstracted for compatibility with various extension wallet.
  • Add event for chainChanged, accountsChanged actions
  • Add async signAndSendTransaction(transactions: Transaction[]): Promise<FinalExecutionOutcome> to sign and send transactions to the RPC and wait until transaction is fully complete.

@changeset-bot
Copy link

changeset-bot bot commented Sep 21, 2022

⚠️ No Changeset found

Latest commit: 1494dd7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Yoon-Suji Yoon-Suji changed the title feat: add wallet-rpc-provider to support various extension wallet feat: add wallet-rpc-provider to support various extension wallets Sep 21, 2022
Copy link

@kenobijon kenobijon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests for this change

@andy-haynes
Copy link
Collaborator

What is the benefit of having the consumer of this class provide a request method that replaces the fetchJson call currently used in the JsonRpcProvider.sendJsonRpc method?

Overall these changes don't seem like a great candidate for a new Provider implementation given that most of it is duplicated from the existing JsonRpcProvider. The additions of network and account monitoring in particular seem like they'd be a better fit for a custom class that instantiates JsonRpcProvider and has a dedicated method to implement the new request logic added here.

@Yoon-Suji
Copy link
Author

Yoon-Suji commented Sep 28, 2022

Hi @andy-haynes , Thank you for your comments.

  1. In contrast to JsonRpcProvider, which could only utilize NEAR browser wallet, WalletRpcProvider allows you to implement request method differently depending on the type of NEAR extension wallet used. It has the benefit of offering scalability to handle the various wallets that are released in the future.

  2. The duplicate method with JsonRpcProvider is because WalletRpcProvider must be able to support all the features supported by JsonRpcProvider. And the code to monitor networks and accounts is necessary because it needs to detect events that change in the wallet.

jsonrpc: '2.0'
};
const response = await this._provider.request(request);
if (Array.isArray(response)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Under what conditions is response an array? Is this behavior that this._provider.request is required to implement?

*
* @param transactions The transactions being sent
*/
async signAndSendTransaction(transactions: Transaction[]): Promise<FinalExecutionOutcome> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will end up calling this? Is signAndSendTransation a valid RPC method?

this._provider = provider;
this._provider.on('chainChanged', this.updateNetwork.bind(this));
this._provider.on('accountsChanged', this.updateAccount.bind(this));
this.init();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firing and forgetting an async method in the constructor here is problematic:

  1. Any rejections will be unhandled
  2. There is no way of knowing when this method has completed and thus when the object has been initialized

This initialization must occur outside of this class or as an explicit secondary step after instantiation for behavior dependent on init's resolution to be reliable at all.

@andy-haynes
Copy link
Collaborator

Thanks @Yoon-Suji!

  1. In contrast to JsonRpcProvider, which could only utilize NEAR browser wallet, WalletRpcProvider allows you to implement request method differently depending on the type of NEAR extension wallet used. It has the benefit of offering scalability to handle the various wallets that are released in the future.

The fetchJson method being replaced here consists of fairly low-level logic, it seems unnecessarily complex to require that consumers of this new class provide their own logic for making simple JSON requests. Moreover the backoff and retry logic used in fetchJson provides ease of use and a reasonable limits to avoid inundating the RPC service with unnecessary requests. It does work outside of the browser (though only because near-api-js injects fetch into the global scope 🙁) and is very generic, so it should work fine in any context without presenting any problems for future development.

Would it be possible to get an example of a request implementation that would be provided to this class? Because of how generic fetchJson is I'm having trouble imagining the benefit of requiring the consumer to provide a function for fetching JSON data. As far as I can tell this shouldn't need to be modified to satisfy your use case.

  1. The duplicate method with JsonRpcProvider is because WalletRpcProvider must be able to support all the features supported by JsonRpcProvider. And the code to monitor networks and accounts is necessary because it needs to detect events that change in the wallet.

I think I'm still not quite understanding - why does need to be in a Provider implementation at all? The duplicated code makes it appear that it's not overriding or extending behavior, which is what makes me think it would make more sense as a separate user class. Are there any examples of how this class might be used as a Provider (e.g. being passed to a Connection instance)?

My concern with the network and account monitoring is that it makes the Provider implementation too smart, when it should really remain as a simple, static interface for RPC communication. This change is what strikes me as being a better candidate for a custom class that wraps JsonRpcProvider rather than providing an alternative implementation.

@Yoon-Suji
Copy link
Author

Hey @andy-haynes! thanks for your comment and sorry for late response🥲
We recently became aware of the discuss over NEP 408, which, in our opinion, shares the same objective as the universal wallet RPC provider we are adding. So We’d like to develop this PR according to the standard proposed in NEP 408, and discuss more about it.

  1. We recognize the problem of unnecessary duplicate methods btw JsonRpcProvider and WalletRpcProvider. We will solve this problem by implementing WalletRpcProvider to extend JsonRpcProvider.
  2. The sendJsonRpc in WalletRpcProvider is a method of comunicating with nodes through the rpc of the wallet using WalletProvider that injected into provider. We want to increase the convenience and economy of development by allowing developers to use the rpc in their wallets without having to create a new provider when developing using the injected wallet.
  3. And also, we will modify the WalletProvider interface to the specification proposed by NEP 408.
interface Wallet {
  id: string;
  connected: boolean;
  network: Network;
  accounts: Array<Account>;

  supportsNetwork(networkId: string): Promise<boolean>;
  connect(params: ConnectParams): Promise<Array<Account>>;
  signIn(params: SignInParams): Promise<void>;
  signOut(params: SignOutParams): Promise<void>;
  signTransaction(params: SignTransactionParams): Promise<transactions.SignedTransaction>;
  signTransactions(params: SignTransactionsParams): Promise<Array<transactions.SignedTransaction>>;
  disconnect(): Promise<void>;
  on<EventName extends keyof Events>(
    event: EventName,
    callback: (params: Events[EventName]) => void
  ): Unsubscribe;
  off<EventName extends keyof Events>(
    event: EventName,
    callback?: () => void
  ): void;
}

which can be used to interact with the NEAR RPC API using provider in injected wallet
which signs using NEAR injected wallet
fix near connect to support wallet-rpc-provider and injected-wallet-signer
@Yoon-Suji
Copy link
Author

Yoon-Suji commented Nov 2, 2022

hi @andy-haynes ! We have couple of changes in our code. If you don't mind, could you check it out? Thank you :)

Description

There are some extension wallets that have their own RPC provider. I think that using another json-rpc-provider is not appropriate in this situation because the providers are duplicated. Therefore, we propose a wallet-rpc-provider class that supports a provider within a wallet.

It can connect with the extension wallet using a property called Wallet that adheres to the Injected Wallet API standard suggested by NEP408. It connects directly with the RPC node using the wallet's provider and is compatible with all of the previous functionalities that relied on the json-rpc-provier. DApp developers can easily and freely use the features provided by near-api-js just by implementing Wallet Class.

Changes

  • wallet.types.ts
    • Wallet interface: the NEAR extension wallet API that follow the NEP 408 standard
    • request: request method is required to freely use the rpc provider provided in the wallet. So we suggeest to add the request method to the NEP 408 standard.
    • signMessage : there are only signTransaction and signTransactions methods in NEP408 but we suggest to add signMessage method for compatibility with near-api-js Signer.
  • WalletRpcProvider
    • extends JsonRpcProvider
    • _wallet: Wallet interface type in src/providers/wallet.types.ts same as NEP 408 Injected Wallet Standard
    • on, off : detecting events that triggered in wallet
    • signAndSendTransaction: signs and sends a transaction to the RPC using wallet and waits until transaction is fully complete.
      • singAndSendTransaction is not valid rpc method that implemented in NEAR RPC Node. It should implement in wallet. If the wallet didn’t implement that method, it should throw Error.
    • sendJsonRpc: communicating with RPC node using provider in wallet. So, the wallet you want to use in WalletRpcProvider should implement request method.
      • wallet returns transaction hash string array when sending one or more transactions. In that case, call txStatus method to get transaction status and return receipt.
  • Signer
    • ExtensionWalletSigner: extends Signer class using wallet API
      • createKey method is not implemented.
      • getPublicKey method gets the existing public key in the wallet for a given account.
      • signMessage method signs the message using wallet API
  • near :
    • NearConfig : add wallet property as optional that used to create a WalletRpcProvider
    • constructor: if config.wallet exists, create connection with WalletRpcProvider and ExtensionWalletSigner instead of JsonRpcProvider and InMemorySigner.
  • connection
    • getProvider: if config.type equals to WalletRpcProvider, return new WalletRpcProvider using config.wallet
    • getSigner: if config.type equals to ExtensionWalletSigner, return new ExtensionWalletSigner using config.wallet
  • Injected-wallet-account.ts
    • InjectedWalletConnection : using NEAR extension wallet for key management.
    • InjectedWalletAccount: using NEAR extension wallet in InjectedWalletConnection as RPC provider
    • signAndSendTransaction: sign and send a transaction by using the WalletRpcProvider. If extension wallet didn’t implement signAndSendTransaction method, use signTransaction and sendTransaction method.
    • confirmFullAccess: helper function returning the access key to the receiver that grants the full access permission.

Example (when using WELLDONE wallet)

class Provider implements Wallet {
  id: string;
  network: Network;
  accounts: Account[];

  constructor() {
    this.id = 'welldone';
    this.network = { networkId: '', nodeUrl: '' };
    this.accounts = [];
  }

  get connected() {
    return Boolean(this.accounts.length);
  }

  on(event: keyof Events, listener: (...args: any[]) => void) {
    let message;
    if (event === 'accountsChanged') {
      message = 'dapp:accountsChanged';
    } else if (event === 'networkChanged') {
      message = 'dapp:chainChanged';
    }
    (window as any).dapp.on(message, listener);
    return listener;
  }

  off<EventName extends keyof Events>(event: EventName, callback?: (() => void) | undefined): void {
    throw new Error('Method not implemented.');
  }

  async request(args: RequestParams) {
		// wallet required to implement signAndSendTransaction
    if (args.method === 'signAndSendTransaction') {
      args.method = 'dapp:sendTransaction';
    }
    const result = await (window as any).dapp.request('near', args);
    return result;
  }

  async connect() {
    if (!(window as any).dapp) {
      throw new Error('Wallet is not installed');
    }
    const result = await (window as any).dapp.request('near', { method: 'dapp:accounts' });
    if (result['near'] && result['near'].address) {
      const accounts = {
        accountId: result['near'].address,
        publicKey: nearAPI.utils.PublicKey.from(result['near'].pubKey),
      };
      this.accounts = [accounts];
      return this.accounts;
    }
    this.network.networkId = (window as any).dapp.networks.near.chain;

    return [];
  }

  async signTransaction(): Promise<nearAPI.transactions.SignedTransaction> {
    throw new Error('This method is not necessary for this example.');
  }

  async signMessage(): Promise<nearAPI.transactions.Signature> {
    throw new Error('This method is not necessary for this example.');
  }
}

const config: nearAPI.NearConfig = {
  networkId: 'testnet',
  keyStore: new nearAPI.keyStores.BrowserLocalStorageKeyStore(),
  nodeUrl: 'https://rpc.testnet.near.org',
  wallet: new Provider(),
};

const accountTest = async () => {
  // create NEAR connection
  const nearConnection = await nearAPI.connect(config as nearAPI.ConnectConfig);
  // create Extension Wallet connection
  const injectedWalletConnection = await new nearAPI.InjectedWalletConnection(nearConnection);
  // request user to sign in
  await injectedWalletConnection.signIn();

  // get current connected account from extension wallet
  const account = await injectedWalletConnection.account();
  // account.viewFunction
  const view_result = await account.viewFunction('contract.myaccount8.testnet', 'get_num', {});
  console.log('view_result: ', view_result);
  // account.functionCall
  const call_receipt = await account.functionCall({
    contractId: 'contract.myaccount8.testnet',
    methodName: 'increment',
    args: { count: 10 },
  });
  console.log('function_call_receipt: ', call_receipt);
};

const contractTest = async () => {
  // create NEAR connection
  const nearConnection = await nearAPI.connect(config as nearAPI.ConnectConfig);
  // create Extension Wallet connection
  const injectedWalletConnection = await new nearAPI.InjectedWalletConnection(nearConnection);
  // request user to sign in
  await injectedWalletConnection.signIn();

  // get current connected account from extension wallet
  const account = await injectedWalletConnection.account();
  // load contract using extension wallet account
  const contract = await new nearAPI.Contract(account, 'contract.myaccount8.testnet', {
    viewMethods: ['get_num'],
    changeMethods: ['increment', 'reset'],
  });
  // call contract
  await (contract as any).reset();
  // view contract
  console.log('view_contract_result: ', await (contract as any).get_num());
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog 🥶
Development

Successfully merging this pull request may close these issues.

None yet

4 participants