feat(web-components): allow custom signer instead of keplr #81
Conversation
| agoricRegistryTypes, | ||
| agoricConverters, | ||
| } from './src/wallet-connection/signerOptions.js'; |
There was a problem hiding this comment.
Nice, glad to see to see these exported. Should we also update the README to include this?
There was a problem hiding this comment.
We should but I think I'd like to land the react-components stuff first so we can have a more cohesive view/example
| const b64address = toBase64(toAccAddress(address)); | ||
|
|
||
| const act1 = { | ||
| typeUrl: AgoricMsgs.MsgProvision.typeUrl, | ||
| value: { | ||
| address: b64address, | ||
| nickname: 'my wallet', | ||
| powerFlags: [PowerFlags.SMART_WALLET], | ||
| submitter: b64address, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Worth exporting makeProvisionSmartWalletMsg(address) along with the types and converters?
There was a problem hiding this comment.
I'm not sure why someone would need this exported if they can just use the wallet connection object. I could be convinced otherwise, but wanted to keep these changes as minimal as possible for the goal of the wallet connection thing. Ditto for makeWalletSpendMsg
There was a problem hiding this comment.
Thinking here was to have exports for folks in this camp, but don't feel strongly about it. Maybe these folks can use @agoric/cosmic-proto which is being updated to use telescope.
My intention is if someone's using a signer for non-agoric stuff also, they should provide their own signing client with the help of agoricRegistryTypes/agoricConverters.
| const act1 = { | ||
| typeUrl: AgoricMsgs.MsgWalletSpendAction.typeUrl, | ||
| value: { | ||
| owner: toBase64(toAccAddress(address)), | ||
| spendAction, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Same as above, worth exporting makeWalletSpendMsg(address, spendAction)?
| * @param {string} chainId | ||
| * @param {string} rpc | ||
| */ | ||
| export const connectKeplr = async (chainId, rpc) => { |
There was a problem hiding this comment.
Maybe add aminoTypes and registry as arguments with default values? accountParser is another thing a developer may want exposed.
There was a problem hiding this comment.
This isn't exported in the package level, it's just a backward-compatible default in case clientConfig isn't provided. My intention is if someone's using a signer for non-agoric stuff also, they should provide their own signing client with the help of agoricRegistryTypes/agoricConverters. If there's any accountParser stuff specific to agoric that we're missing we should provide that here though, is there?
There was a problem hiding this comment.
Correct nothing specific from agoric wrt to account parser. But if someone is using this signer implementation to connect to other chains, like Agoric/wallet-app#128, they might need one.
More in general, if I'm a library author and wrapping something that takes options, I like to expose those options to the consumer to give more them flexibility and the code a longer shelf life.
refs Agoric/agoric-sdk#8813
Also now exports the amino converters and registry types for clients that use custom signers, and did some refactoring to clean up.
Tested the existing keplr connection flow in dapp-inter using
yarn linkto check for regressions.