-
Notifications
You must be signed in to change notification settings - Fork 391
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
Match cosmjs interface and fix intent #444
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, however a f/u question for @bkrem @ganchoradkov, are there any security risks/benefits when encoding base64 over hex? Any change to the amount of storage needed?
@finessevanes security is independent of the encoding used (encoded data can be encrypted or not) so that doesn't change here. Base64 is more efficient than hex (base16) anyways so that's a plus here regarding payload size and storage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the amendments @Thunnini 🙏
If base64 is the common encoding for cosmjs then base16/hex just seems to create unnecessary ceremonies here.
We would have to update our examples to reflect this change, so I will create a follow-up issue for this in https://github.com/WalletConnect/web-examples once this merges 👍
cc @ganchoradkov you see any possible issues here or can we merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all good on my side 💯
cosmos rpc seems to be built along cosmjs's offline signer/direct signer. However, there is a difference from the actual cosmjs interface.
Also, since cosmjs encodes most byte arrays to base64, we suggest replacing the existing hex-encoded part with base64.
Changes
cosmos_getAccounts
response.signAmino/signDirect
response.