EOSIO / eosjs

General purpose library for the EOSIO blockchain.
http://eosio.github.io/eosjs
MIT License
1.43k stars 463 forks source link

Clarification on how and when signature providers should be injected or instantiated, especially in mobile integrations #409

Closed ismyhc closed 3 years ago

ismyhc commented 6 years ago

Id like to start the process of conforming to the signature provider protocol.

With EOS Lynx, we provide a "dApp Browser" type integration. For instance currently, a web app can be launched in our browser and we then on iOS/Android inject a small javascript sdk to bridge interaction between the web app and the native app. Our small sdk will likely be wrapped in a signature provider to be handed off to eosjs2. I see a couple of problems.

  1. eosjs2 expects to be instantiated with the signature provider.
  2. While iOS can inject js at start and end of document, Android can only inject after.

It seems to be there needs to be some standard way to specify the signature provider to eosjs2 after instantiation OR so standard way to let the web app know about a signature provider so that they can instantiate it.

Any thoughts?

tbfleming commented 6 years ago

A potential idea: the injected code can call a function if it exists and pass in a signature provider. Apps could define that function and instantiate eosjs there.

ismyhc commented 6 years ago

That’s an interesting idea. It could work. As long as the injected code is in the same scope as the function. I normally inject our sdk on the window on mobile. Forgive my lack of js knowledge.

Would it be possible to expose a function on the eosjs library that would allow for setting or updating the signature provider after eosjs instantiation?

ismyhc commented 6 years ago

Something else I’d like to add that I’ve noticed. Currently with all of our integrations, we provide the ability to return the current account object or name. Maybe I’ve missed something here, but with the signature provider interface I only see getAvailableKeys(). This would not indicate which account to use in the case there are multiple accounts associated with the keys. Most if not all web app integrations expect an account to act on. Therefore I could see a divide in standard with different wallet providers having to create an api for this. It seems to me that it would be wise for signature provider to also have a standard function interface that will allow for the web app to determine which account the user would like to use for signing, etc.

tbfleming commented 5 years ago

Getting accounts: maybe we could add an AccountProvider interface:

export interface GetAccountsResult {
    accounts: string[];
    currentAccount?: string;    // Might be undefined
}

export interface AccountProvider {
    getAccounts: (chainId: string, publicKeys: string[]) => Promise<GetAccountsResult>;
}

JsonRpc could implement a default version of this interface like it does with AuthorityProvider and AbiProvider

tbfleming commented 5 years ago

We could recommend apps define this function:

function loadedWallet(args: {
    signatureProvider: SignatureProvider,
    authorityProvider?: AuthorityProvider,
    abiProvider?: AbiProvider,
    accountProvider?: AccountProvider,
})

These could be passed to Api's constructor. We could also add a new function to Api to change these.

ismyhc commented 5 years ago

@tbfleming This would be a welcoming addition. I believe this would satisfy the basic requirements needed for signing and transacting via a wallet like lynx, scatter, etc. I’m totally on board with this.

@nsjames any suggestions on this? :)

nsjames commented 5 years ago

https://github.com/EOSIO/eosjs/issues/409#issuecomment-433545424 this looks good.

There's a question here though, what would the benefit of these actually be? When calling methods eosjs is unaware of which parameter is supposed to be which ( apart from the system builtins ) and wouldn't know that account is supposed to be parameter 1.

If we're just setting up a standard for usage with accompanying getAccountFromWallet() and eosjs.account functions/properties then these are fantastic. If the goal is to internally superimpose those accounts into selected positions then we might run into more issues than we're solving.

Also, perhaps the accountProvider and authorityProvider should be merged into a singular object on the output with possible helper functions like account.withAuthority() or account.name to consolidate the process for devs.

tbfleming commented 5 years ago

AccountProvider and AuthorityProvider serve 2 different purposes:

A wallet may choose to provide none, either, or both. eosjs will provide default implementations that use RPC.

c0d3ster commented 5 years ago

From my understanding, the addition of an AccountProvider would simply lighten the load for developers of extracting each of the accounts from the provided keys for a user to select an identity. This process of pulling accounts from keys, concatenating the names, then removing duplicates will occur across almost every wallet and many applications. I will say that as we increase the number of optional "Provider" parameters, it may become more confusing as to what the benefit of using a custom implementation for each is, but to @nsjames point of concern we're generally avoiding parameter ambiguity by using object literals.

sanaraufx commented 3 years ago

Cleaning up old issues so we can focus on more recent issues. Please reopen this ticket if needed. Thank you.