Add JSON-RPC WalletConnect Support #386
Conversation
| @@ -0,0 +1,180 @@ | |||
| package cosmos | |||
There was a problem hiding this comment.
nit, I think we should eventually restructure the packages to be rpc/namespaces/ since we are not only including ethereum namespaces
There was a problem hiding this comment.
can be done in a separate PR though
| } | ||
| for _, info := range list { | ||
|
|
||
| addr := sdk.AccAddress(info.GetAddress()) |
| type SignDirectRequest struct { | ||
| SignerAddress sdk.AccAddress `json:"signerAddress"` | ||
| SignDoc SignDocDirect `json:"signDoc"` | ||
| } | ||
| type SignDirectResponse struct { | ||
| Signature string `json:"signature"` | ||
| SignDoc SignDocDirect `json:"signDoc"` | ||
| } |
There was a problem hiding this comment.
I'd write comments on top of the structs as they are public
| func (api *WalletConnectAPI) convertToTxType(signDoc SignDocDirect) (txtypes.SignDoc, error) { | ||
| accountNumber, err := strconv.ParseUint(signDoc.AccountNumber, 10, 64) | ||
| if err != nil { | ||
| api.logger.Error("failed to parse account number: %s, err: %s", signDoc.AccountNumber, err.Error()) |
There was a problem hiding this comment.
wrap below error instead
| api.logger.Error("failed to parse account number: %s, err: %s", signDoc.AccountNumber, err.Error()) |
| } | ||
|
|
||
| // This method returns a signature for the provided document to be signed | ||
| // targetting the requested signer address corresponding to the keypair returned |
There was a problem hiding this comment.
| // targetting the requested signer address corresponding to the keypair returned | |
| // targeting the requested signer address corresponding to the keypair returned |
| type SignDocAmino struct { | ||
| AccountNumber string `json:"account_number"` | ||
| ChainID string `json:"chain_id"` | ||
| Sequence string `json:"sequence"` | ||
| Memo string `json:"memo"` | ||
| Msgs []sdk.Msg `json:"msgs"` | ||
| Fee legacytx.StdFee `json:"fee"` | ||
| } | ||
| type SignAminoRequest struct { | ||
| SignerAddress sdk.AccAddress `json:"signerAddress"` | ||
| SignDoc SignDocAmino `json:"signDoc"` | ||
| } | ||
| type SignAminoResponse struct { | ||
| Signature string `json:"signature"` | ||
| SignDoc SignDocAmino `json:"signDoc"` | ||
| } |
| } | ||
|
|
||
| // This method returns a signature for the provided document to be signed | ||
| // targetting the requested signer address corresponding to the keypair returned |
There was a problem hiding this comment.
| // targetting the requested signer address corresponding to the keypair returned | |
| // targeting the requested signer address corresponding to the keypair returned |
| SignDoc SignDocAmino `json:"signDoc"` | ||
| } | ||
|
|
||
| // This method returns a signature for the provided document to be signed |
There was a problem hiding this comment.
| // This method returns a signature for the provided document to be signed | |
| // SignAmino returns a signature for the provided document to be signed |
| signDocAmino := req.SignDoc | ||
| accountNumber, err := strconv.ParseUint(signDocAmino.AccountNumber, 10, 64) | ||
| if err != nil { | ||
| api.logger.Error("failed to parse account number: %s, err: %s", signDocAmino.AccountNumber, err.Error()) |
There was a problem hiding this comment.
wrap error
| api.logger.Error("failed to parse account number: %s, err: %s", signDocAmino.AccountNumber, err.Error()) |
| } | ||
| seq, err := strconv.ParseUint(signDocAmino.Sequence, 10, 64) | ||
| if err != nil { | ||
| api.logger.Error("failed to parse blockchain account sequence: %s, err: %s", signDocAmino.AccountNumber, err.Error()) |
|
Excited to see this PR taking place 💪 I'm curious from your POV if the current API (params and result) for these JSON-RPC methods looks good. These are mostly derived from cosmJS APIs but I would love feedback on what could be changed or improved For example, I find the mixing of encoding on the params to be odd (utf8 strings and hex strings) However if this seems to fit most APIs within the Cosmos ecosystem then I have no objections to sticking to the existing spec |
|
@pedrouid I think we can get rid of the signdoc field from the response types. Also, the SDK doesn't support bec32 for pubkeys anymore, they just print the string value of the protobuf |
|
Do you think there would be cases where wallets would change the signed doc? Say for example to adjust how much fees are used or which coins are used to pay those fees? |
|
Will open a PR later with the conflicts fixed. |
Closes: N/A
Description
This builds off of #328.
This PR adds a new
cosmosnamespace and implements the following methods in accordance with the WalletConnect JSON-RPC Cosmos API:GetAccountsSignDirect— corresponds to SignDocSignAmino— corresponds to the legacy StdSignDocRelevant docs: https://github.com/cosmos/cosmos-sdk/blob/master/docs/core/transactions.md#signing-transactions
Testing can be done by:
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)