[client] Adds signWithEckoWallet and quicksignWithEckoWallet functions

[ash-vd]

Adds two functions to sign a transaction with the Ecko Wallet API.

Usage, where unsignedTransaction is a IPactCommand:

const signWithEckoWallet = createEckoWalletSign();
await signWithEckoWallet(unsignedTransaction);

const quicksignWithEckoWallet = createEckoWalletQuicksign();
await quicksignWithEckoWallet(unsignedTransaction);

On both signWithEckoWallet and quicksignWithEckoWallet there are several helpers available:

const isInstalled = signWithEckoWallet.isInstalled();
const isConnected = await signWithEckoWallet.isConnected();
const accountInfo = await getAccountInfo();
await signWithEckoWallet.connect();

connect() is also automatically called when signing for a transaction

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
alpha-docs	⬜️ Ignored (Inspect)	Visit Preview		Aug 29, 2023 9:19am
docs-storybook	⬜️ Ignored (Inspect)	Visit Preview		Aug 29, 2023 9:19am
immutable-records	⬜️ Ignored (Inspect)	Visit Preview		Aug 29, 2023 9:19am
react-ui	⬜️ Ignored (Inspect)	Visit Preview		Aug 29, 2023 9:19am
tools	⬜️ Ignored (Inspect)	Visit Preview		Aug 29, 2023 9:19am

[Takadenoshi]

Is this the best way to check for active status or should we return true on a positive signal?

[ash-vd]

Although Ecko returns a fail status when it isn't connected, I agree that it might be better to turn things around to only return true when the returned status is success. Then it'll also return false when Ecko returns capybara or something else that's just weird.

[Takadenoshi]

Does this ever throw?

[ash-vd]

Only when request doesn't exist on window.kadena, which is checked in isConnected. I've now added an extra check in that function to check if window.kadena.request is there.

[Takadenoshi]

LGTM aside from two small questions

[javadkh2]

mixing a function with properties might lead to a confusing api for users since function itself has some methods like apply, call ,...
what if we instead add the sign as a method like this

export interface IEckoSignFunction {
   isInstalled: typeof isInstalled;
   isConnected: typeof isConnected;
   connect: typeof connect;
   sign : ISingleFunction,
   quickSign : ISingleFunction,
}

[alber70g]

We've thought about that, but we wanted to make the usage of this function the same as the other functions to allow drop-in replacement.

[javadkh2]

Maybe instead of exposing createEckoWalletSign and createEckoWalletQuicksign we can have one creator like createEchoWalletConnector

export interface IEckoWalletConnector {
   isInstalled: () => boolean;
   isConnected: (networkId:string) => Promise<boolean>;
   connect: (networkId:string) => Promise<boolean>;
   sign : ISingleSignFunction,
   quickSign : ISignFunction,
}
const echoWalletConnector: IEckoWalletConnector = createEchoWalletConnector();

What do you say @alber70g
Maybe we even can have a separate package for connectors? like @kadena/wallet-connectors
then we can focus of creating some connectors for common wallets there.

[alber70g]

Maybe instead of exposing createEckoWalletSign and createEckoWalletQuicksign we can have one creator like createEchoWalletConnector

export interface IEckoWalletConnector {
   isInstalled: () => boolean;
   isConnected: (networkId:string) => Promise<boolean>;
   connect: (networkId:string) => Promise<boolean>;
   sign : ISingleSignFunction,
   quickSign : ISignFunction,
}
const echoWalletConnector: IEckoWalletConnector = createEchoWalletConnector();

What do you say @alber70g Maybe we even can have a separate package for connectors? like @kadena/wallet-connectors then we can focus of creating some connectors for common wallets there.

There are multiple things at hand here

1. as said in above comment, we wanted to make it as uniform as possible when working with the wallets and sign functions
2. we want to deprecate sign at some point. It's easier to deprecate that function when it has its own separate code-branch
3. A separate package would be nice. This would provide a clear entry point into all the external connections that can be made and provide an easy way to integrate new connections. Let's think about this, and create an issue for that

[LindaOrtega]

Do we still want to merge this pull request?

[changeset-bot]

🦋 Changeset detected

Latest commit: 8748d4b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@kadena/client	Minor
@kadena/tools	Patch
@kadena/client-examples	Patch
@kadena/cookbook	Patch
@kadena/pactjs-generator	Minor
@kadena/pactjs-cli	Minor

Not sure what this means? Click here to learn what changesets are.

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