Navigation Menu

Skip to content
This repository has been archived by the owner on Mar 11, 2024. It is now read-only.

Include default keyType for chain account for walletconnect #581

Conversation

ross-weir
Copy link
Contributor

This is in relation to alephium/alephium-web3-react#10

keyType is required in the wallet connect provider as of alephium/walletconnect@8629df4#diff-e0e6d91455eef50c5793e300039e5cdc7e6d372707172b639564495e4fa0a2f3R353

As far as I can see different key types aren't supported in the desktop wallet yet so send through default for now, or correct me if this is wrong 馃憤

Also note I think formatAccount alephium/walletconnect@8629df4#diff-e0e6d91455eef50c5793e300039e5cdc7e6d372707172b639564495e4fa0a2f3R346 might need to use a slash for the keytype separator otherwise walletconnect complains about improper formatting:

image

Copy link
Member

@nop33 nop33 left a comment

Choose a reason for hiding this comment

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

馃憦 馃檹

src/modals/WalletConnectModal.tsx Show resolved Hide resolved
@nop33
Copy link
Member

nop33 commented Mar 14, 2023

Is there an easy way I can test this so that I can approve the PR? Sorry, my brain is atm fully devoted to the v2.0 of the desktop wallet 馃

@ross-weir
Copy link
Contributor Author

Is there an easy way I can test this so that I can approve the PR? Sorry, my brain is atm fully devoted to the v2.0 of the desktop wallet 馃

Not sure if there's easier unfortunately, the way I'm doing it at the moment is a bit cumbersome:

@ross-weir
Copy link
Contributor Author

Also note I think formatAccount alephium/walletconnect@8629df4#diff-e0e6d91455eef50c5793e300039e5cdc7e6d372707172b639564495e4fa0a2f3R346 might need to use a slash for the keytype separator otherwise walletconnect complains about improper formatting

Just noticed this observation is outdated, the walletconnect lib has already been updated to use a slash for the sep: https://github.com/alephium/walletconnect/blob/master/src/provider.ts#L346

@nop33
Copy link
Member

nop33 commented Apr 12, 2023

The commit of this PR is already merged in the v2.0 branch :) 6ed6598

Thanks a lot for your work @ross-weir!

Closing PR

@nop33 nop33 closed this Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants