search

Concordium / concordium-dapp-libraries

A coherent set of building blocks for making it as easy as possible for developers to build web-based dApps.
Apache License 2.0
7 stars 5 forks source link

wallet-connectors: Add support for signing both string and binary messages #35

Closed bisgardo closed 1 year ago

bisgardo commented 1 year ago

Purpose

Add support for signing both string and binary messages in the WalletConnection interface. Fully implemented for Browser Wallet, partially for WalletConnect.

Changes

Replaced the string message type in signMessage with a new type SignableMessage which is a discriminated union of a binary message (which has a type/parameter schema) and a simple string message.

The mobile wallets don't yet support signing binary messages, so the WalletConnect implementation throws an exception for that case. It's very non-desirable to throw exceptions at use time for unimplemented features as compared to enabling the dApp to detect available features for a given connection. But in this case, the warts that this would impose on the interface are not deemed worth it: The MW aren't lacking the feature, it's more like they aren't entirely done implementing it...

The sample dApp sign-message is updated to support both variants by adding a "schema" input for the parameter schema (represented as base64). If this input is non-empty, the message is assumed to be binary (represented as hex). Otherwise it's a plain string.

Also, renamed ParameterSchema to TypeSchema to make it more semantically correct to use this in the context of message values.

This PR is an alternative to #32.

bisgardo commented 1 year ago

LGTM, I think, The sample doesn't really work for me (I get some Watchpack errors shrug, and before the website crashed because it couldn't find stringMessage thinking, but I'll check again tomorrow)

I've just done a fresh install on two different machines where it worked fine.

shjortConcordium commented 1 year ago

LGTM, I think, The sample doesn't really work for me (I get some Watchpack errors shrug, and before the website crashed because it couldn't find stringMessage thinking, but I'll check again tomorrow)

I've just done a fresh install on two different machines where it worked fine.

Yes, it works for me again after reinstalling the repo :thinking:

bisgardo commented 1 year ago

LGTM, I think, The sample doesn't really work for me (I get some Watchpack errors shrug, and before the website crashed because it couldn't find stringMessage thinking, but I'll check again tomorrow)

I've just done a fresh install on two different machines where it worked fine.

Yes, it works for me again after reinstalling the repo thinking

The wonders of the javascript ecosystem :joy: