Integration of Wallet Connect 2.0 Protocol for Wallets and dApps on Hedera

[bugbytesinc]

Thread to discuss HIP-820 Integration of Wallet Connect 2.0 Protocol for Wallets and dApps on Hedera

[hgraphql]

Consensus on this HIP will inform changes to the docs that show up on WalletConnect - here's a link to an open PR - WalletConnect/walletconnect-specs#117

[hgraphql]

@bugbytesinc what is your thinking behind the preface hip820_functionName vs hedera_functionName? The other specs use the networkName_methodName format such as with near https://specs.walletconnect.com/2.0/blockchain-rpc/near-rpc

The first pass we took at method names were:

hedera_signAndExecuteTransaction
hedera_signAndReturnTransaction
hedera_signMessage

So it looks like proposal is to change:

signAndReturnTransaction -> signTransactionBody
signAndExecuteTransaction -> signTransactionAndSend

looks like the signTransactionAndSend would have the dApp construct a transaction, send it to a wallet for signing, and then send back to dApp to the submit to the network? For this would it be better to send an unsigned transaction to a wallet for signing and if approved by the wallet user, have the wallet send to the network?

[bugbytesinc]

<prefix>_signTransactionAndSend is intended to be the method that signs the transaction and submits it to the network. The method that is intended to sign and return is <prefix>_signTransactionBody.

signTransactionAndSend ... repackages the message as appropriate for transmission to the Hedera network and sends it to the network. The controller shall wait for the initial response from the Hedera node and return the resulting Node Precheck Code generated by the submission.

[bugbytesinc]

The idea behind using the hip820 in the namespace:

• It is immediately recognizable to Hedera Community members as to where more information can be found
• Other chains do use their improvement proposals in their namespace, most notably chainids
• If the protobuf changes or the community decides to blow this up and replace it, we didn't pollute the hedera namespace, in other words when this is superseded, the new method names most likely need to start with hipXXX anyway to differentiate from this spec. Might as well set the precedent now.

[hgraphql]

_signTransactionAndSend is intended to be the method that signs the transaction and submits it to the network. The method that is intended to sign and return is _signTransactionBody.

thanks, should the parameter be unsignedTransaction or transactionBytes?

[hgraphql]

The idea behind using the hip820 in the namespace:

Definitely appreciate the thinking behind this. In the context of wallet connect, other chains use their chain name as a descriptor. So I think Wallets and dApps that are going to support multiple chains, following that convention might be a good way to go.

That being said, we should consider some sort of versioning for the spec to allow for it to change and evolve.

[bugbytesinc]

I think it should be signedTransaction because it is the bytes of a SignedTransaction. The reason to use SignedTransaction as opposed to TransactionBody in this case is because the use case may or may not include other signatures. This facilitates a simple solution to supporting the 80% use case where the controller is the only signer, but yet opens up the opportunity for being the final signer in a transaction that requires multiple signatures. unsignedTransaction - well, its not exclusively an unsigned transaction, and transactionBytes is, well doesn't match any message schema name.

[hgraphql]

I suggest we add a signMessage function. There are use cases where a dApp could submit an arbitrary string or other data to be signed that does not need to be submitted to the network

[bugbytesinc]

Added method.

[bugbytesinc]

While working thru a prototype, I just realized that the HAPI presently uses keccak256 to digest the message prior to signing for ECDSA keys, but not when signing Ed25519 keys. First, to be consistent with other HAPI functionality, should we do the same (meaning not compute the keccak256 of the string prior to signing with an Ed25519 key); and if so, should be amend the HIP to be more specific regarding this matter, including specifying UTF8 or ASCII encoding of the message into bytes for signing (probably the former)?

[bugbytesinc]

Actually, it might be OK to define it as the UTF8 encoding of the string:

"\x19Hedera Signed Message:\n" + len(message) + message

signed in the same manor as HAPI transactions are signed

and since we're returning a SignatureMap anyway, this becomes very consistent with the HAPI and future proof if HAPI adds support for an additional key format?

[hgraphql]

hey @bugbytesinc sorry about the delay in response on this. If the additional keccack256 hash is superfluous, I don't see any reason to include it.

[bugbytesinc]

Resolved

[hgraphql]

we should have a function for queries as well - https://docs.hedera.com/hedera/sdks-and-apis/sdks/queries

[bugbytesinc]

I'm still on the fence with the idea of supporting queries, however if we were to add on to the spec I'd recommend something like <prefix>_signQueryAndSend a little verbose of a name, but consistent with <prefix>_signTransactionAndSend. Dome details:

• It adds its signature to the query and sends it to the network, waits for results
• The returned results are the protobuf of the response (this is to future proof future the method, it is not necessary to explode the results into some other format such as JSON because we already must be speaking protobuf. Additionally WC2 SDKs/Clients can hide the complexity if necessary)

We should not support signing a query and then letting a system send it on the controllers behalf (sign and return) because of the way the query protobuf format is formatted, you can rip out the transaction and execute it without actually doing the query; while this has limited bad actor utility, but because it can happen, we should not build it into the protocol. This is different from other transactions where the transaction that is signed can not be altered by the dApp after it is signed.

[hgraphql]

I have had some groups, for example some teams in Swirlds Labs that have requested that Queries be supported.

Queries are less used that Transactions and many use cases can use the mirror node instead. Though with the intention of unlocking the full feature set of Hedera, I suggest we add this unless Hedera removes this functionality. With that said, I imagine we'll have some special considerations when adding this.

Another consideration for queries, who is responsible for assembling the payment? Would it be the controller (wallet) or the dApp? Either approach has it tradeoffs with complexity. If it is the dApp, the wallet might object to the payment bribe amount in the query header, if left to the controller --- then what part of the query is necessary to transmit to the controller.

I think it'd be the controller for this. If the dApp was to pay, they wouldn't need to send to a controller, the could just just do the query on their end?

Either way, I'm wondering how many wallets would go to the effort to implement this (a controller can refuse to support any given method by my understanding of the WC2 spec.)

In regards to implementation, we've started on a helper library with our initial pass at the spec (https://github.com/hgraph-io/hedera-walletconnect-utils/pull/3/files). As a community, we can develop a helper library to help ease implementation for wallets & dApps.

[hgraphql]

_signQueryAndSend a little verbose of a name, but consistent with _signTransactionAndSend

Yeah, definitely a trade off on this. Though I think consistency helps most people.

We should not support signing a query and then letting a system send it on the controllers behalf (sign and return) because of the way the query protobuf format is formatted, you can rip out the transaction and execute it without actually doing the query; while this has limited bad actor utility, but because it can happen, we should not build it into the protocol. This is different from other transactions where the transaction that is signed can not be altered by the dApp after it is signed.

Okay, thanks for the callout.

[bugbytesinc]

Added use case and spec for hip820_signQueryAndSend to the HIP.

[bugbytesinc]

Resolved

[hgraphql]

Starting a thread on Github governance.

we've started a helper library here: https://github.com/hgraph-io/hedera-walletconnect-utils.

• we're currently in the process of moving this to the @hashgraph organization
• our goal is to build a library out that is significantly features complete to help wallets & dApps implement this spec and WalletConnect.
• I think we need contribution and feedback from wallets and dApps interested interested in implementing WalletConnect

[hgraphql]

Also wanted to call out that the WalletConnect Web3Wallet sdk supports multiple languages, not just javascript. So it'd be great to have reference implementations in other platforms as well

I think it'd also be best to house this in 1 repo if possible

https://docs.walletconnect.com/web3wallet/about

[bugbytesinc]

Straw poll, which namespace do you prefer, please upvote this comment if you prefer the

hedera_

method namespace.

[kenthejr]

If I was implementing an SDK, it'd identify which HIP I'm adhering to...so I would say something like "This SDK implements the HIP820 spec for wallet connect" and if we supersede it, I'd update my SDK accordingly.

[hgraphql]

we had begun a helper library - https://github.com/hgraph-io/hedera-walletconnect-utils based on the original spec we wrote before this HIP.

We can use that as a base and together we can utilize that to build a "Hedera <> WalletConnect" SDK based on this spec.

Additionally, an update to the spec might result in an update to a function name, in which case a dApp would know if requesting permissions to call the function that does not support a new function, so versioning in this case is built in.

If a function needs to change in parameter or return structure, but keep the same function name, there would need to be some release / support management.

There is some built in functionality WalletConnect has that allows a wallet to describe the functions they support. Maybe there could be an optional parameter to specify a HIP version?

[hgraphql]

@kenthejr hbar_* is 4 letters, though seems reasonable. both fullnetworkname_* and someshorteridentifier_* seem to have precendence for chains that have integrated such as https://specs.walletconnect.com/2.0/blockchain-rpc/cosmos-rpc

[tonycamero]

hbar vs hed = token vs network
+1 for network IMO

[hgraphql]

Closed

[bugbytesinc]

Straw poll, which namespace do you prefer, please upvote this comment if you prefer the

hip820_

method namespace.

[bguiz]

My 2c:

• For now, in the first version/ initial public beta: hip820_ or something else that implies it is still WIP
• When the spec is finalised: hedera_

[bguiz]

the reason I say the above is that Metamask has gone through multiple versions of some things, and the namespace is now "cluttered"

example: https://docs.metamask.io/wallet/reference/eth_signTypedData_v4/

[hgraphql]

@bguiz I think the idea of an initial public beta is a good one.

The naming strategy could potentially be used in the future to introduce new functionality.

For example a hip1234_greatNewFunctionality could be called in the future before a bit of functionality reaches widespread adoption. On the other hand it is actually up to a wallet to decide to implement the functions of their choosing, and determining the which functions can be used is handled during the connection process.

[hgraphql]

Closed

[bugbytesinc]

Comments above discuss the pros and cons of choosing a namespace for the methods defined in this HIP. At the moment, there are two ideas: using this HIP’s number (820) as a namespace, resulting in the hip820_ prefix, or simply using hedera_ as the prefix. Both ideas have tradeoffs. Using hedera_ obviously identifies the methods as specific to hedera and using hip820_ ties the methods to a specific HIP within the hedera ecosystem and by side-affect, a specification version. The original draft of this document chose hip820_ as to futureproof any ambiguity if follow on HIPs were generated replacing this one. This may or not be a concern for the community at this time, so I would like to perform a straw poll as to which prefix is preferred. Please upvote one of the comments above to signal your preference.

[teacoat]

I would suggest hedera to keep as close to https://hips.hedera.com/hip/hip-30 as possible, also much more readable
Also mentioned here: https://github.com/ChainAgnostic/CAIPs/blob/main/CAIPs/caip-19.md

[hgraphql]

Closed

[AdrianKBL]

I'm gonna choose hedera

[hgraphql]

Closed

[bugbytesinc]

It appears hedera_ is the popular opinion, any one else want to chime in?

[tonycamero]

Is the namespace getting "cluttered" a reason to start with hip820 then graduate to hedera?

[bugbytesinc]

Nah, it was more about having a very explicit namespace for versioning purposes (that was my motivation anyway), since the protobuf changes so infrequently its probably a red-herring and going with hedera_ matches what the other chains (except Ethereum) have done. Same is better than better sometimes.

[bugbytesinc]

Votes counted, hedera_ wins.

[bugbytesinc]

HIP Updated to use hedera_ method prefix.

[hgraphql]

Closed

[hgraphql]

Should we add an Account ID parameter to all new methods, just like it was done in Ethereum JSON-RPC specification? Since WalletConnect allows approving multiple accounts for a session, we'd want to be able to specify which account to sign with.

cc: @zanctor

[hgraphql]

cool, so sounds like we're at a group vote here?

I think wallet implementers should probably have the biggest influence here. At the end of the day, most developers are going to leverage the fuctionality of the SDKs. These SDKS already throw errors and exceptions

[teacoat]

I agree - leaving the precheck out on success is the least effort and conforms to what the sdk already provides - same with the errors, we should just stick as close to what is already returned

[hgraphql]

@AdrianKBL, @Volind as members of the HIP working group leading teams actively implementing can you weigh in here?

The point of consensus we need to come to in this thread is return values for the 2 functions that send transactions to the Hedera network.

One suggestion is to send the same payload as the TransactionResponseJSON in the javascript SDK OR send an error when the precheck fails and pass along an error object similar to PrecheckStatusError
#819 (reply in thread)

it will look like this - #819 (reply in thread)

The second suggestion is to always send back a success response as success with this payload - #819 (reply in thread)

If we go the route of sending back an error object when precheck fails (and the sdk's throw errors) we're able to remove the precheck code that will always be 0 on the response object. dApps then can handle these errors like the handle the other types of errors in the session, vs. having blocks of if(precheck !=0) handlePrecheck error

[hgraphql]

Here's a relevant draft PR into the reference we're building that might provide additional context on what the payload / error handling might look like - https://github.com/hgraph-io/hedera-walletconnect-utils/pull/88/files

on the dApp side we currently have code like:

const response: TransactionResponseJSON & { precheckCode: number } = await signClient.request(
   {
     topic: activeSession.topic,
     chainId: HederaChainId.Testnet,
     request: {
       method: HederaJsonRpcMethod.SignTransactionAndSend,
       params: {
         signedTransaction: transactionToBase64String(transaction),
         signerAccountId: state['sign-send-from'],
       },
     },
   },
 )

 console.log(response)

 const transactionResponse = TransactionResponse.fromJSON(response)
 if (response.precheckCode !== 0) {
   alert(`${transactionResponse.transactionId}:${response.precheckCode}:FAIL!`)
 }

[franfernandez20]

I agree with keeping the formats consistent across different contexts. I would vote to follow the SDK approach and avoid re-encoding.

[bugbytesinc]

We may need to include some statement regarding error codes: First, it appears that 90%+ of the failure cases are already covered by the spec and codes enumerated in the Sign API.
However, given the nature of Hedera we have a few possible edge cases worth looking into:

• No Route to Gossip Node – in this edge case, the controller is unable to submit the transaction to the network because the gossip node identified in the signed transaction is not routable from the controller.
• Gossip Node is Unresponsive – well maybe this is covered in WC spec, I’d need to look into it further, but this is definitely a hedera'ish kind of error. This is when the controller is unable to connect to the gossip node due to gRPC low level errors (not to be confused with precheck BUSY responses).
• Transaction Expired – in this edge case the user did not approve a transaction in time for the controller to submit it to the network in time for it to be executed before the transaction time to live expired (or would this be considered a passive-aggressive rejection and return the WC code for this error, userRejectedRequest, or sessionRequestExpired [I think this last one means something else]) Not the same as having submitted and received a precheck of TRANSACTION_EXPIRED, more when the controller knows there is no point in submitting the transaction because it is expired.

To be clear, we only need to cover errors that prevent a PRECHECK code from being returned, if we have a PRECHECK code (either success or failure) it is to be returned to the caller, and the method invocation itself is considered a “success”.

• For example, if the controller does provide retry logic, and receives BUSY until the transaction expires, then should it return a PRECHECK of BUSY? (I would vote yes on this interpretation).

So, the question is: what is considered good practice in the WC space for dealing with chain specific errors? I would think that the first choice might be to reach out to Wallet Connect to see if there is a set of error code numbers that are reserved specifically for chainids; or is it considered a good practice to address these custom edge cases directly in the network specific rpc call protocols themselves?

[gregscullard]

Imo we should distinguish precheck from receipt error in the response, a transaction may have been successfully sent (Precheck=OK) and a receipt never obtained (network connection loss between tx send and receipt request) so the dApp knows that the transaction may have been successful (it may have failed too despite precheck=OK) and can subsequently check with a mirror or receipt query before deciding whether to resubmit the transaction.

Rationale: there are two steps in sending and verifying a transaction, submitting the transaction which results in a synchronous precheck response, then asynchronously check for a receipt which may succeed (with a receipt status OK or NOK) or fail (receipt/transaction status is unknown). In the event the transaction status is unknown, the dApp should implement a strategy to :

• either assume it failed and resubmit (warn the user this may result in a duplicate and the user should check before proceeding)
• check if it did indeed fail or succeed by querying a mirror node and determining next steps as a result (my preference).

[gregscullard]

This is particularly relevant to mobile wallet signing imo, mobile wallets are often on the move and connection loss is more likely than with desktop computers (can't be precluded there either).

[bugbytesinc]

The spec we're proposing does not involve waiting for a receipt. It just submits the transaction and returns the precheck code without waiting for consensus. The user can use the query method to retrieve a receipt if necessary, and client toolkits are quite able to knit these two things features together in a wait-for-receipt semantics for dApps if that is the desired programming model.

[hgraphql]

this HIP is different from the original spec we had put together in this regard to not waiting for a receipt. I'm working through building a demo here https://github.com/hgraph-io/hedera-walletconnect-utils/tree/feature/demo that can be used to test out these differences as well as working out querying the mirror node and determining next steps

[gregscullard]

Understood, this makes sense, we can iterate on this later (or maybe add a 'getReceipt' and 'getRecord') to the messages we support, reasoning is to try and abstract the dApp from having to have an up to date address book and avoid issues where the dApp and wallet have different address books resulting in Invalid node account id errors, imo, the wallet should be 100% in control of transport in the fullness of time.

[gregscullard]

Not sure where one of my comments from yesterday went... it was related to including a "transactionType" to the sign and signAndSend requests to help golang developers decode the transaction bytes (in golang, you can't determine the type of transaction from the result of a bytes->object conversion), from memory you have to try each possible transaction type until it works which then tells you what the type of the transaction is.
Having a string attribute to indicate the transaction type would enable golang devs to implement a much simpler switch/case statement to decode the transaction.

I'm also thinking that (while out of scope just now) it may help future proof the implementation by enabling an "ethereum" transaction type so that wallets know to send the signed transaction to a JSON/RPC relay rather than a consensus node.

This could be done as an optional flag/boolean in the future too I guess.

[bugbytesinc]

I see including it as a security issue. You can't trust it, you can only trust the bytes that you're asked to sign. It is the controller's responsibility to verify it is signing what the user thinks they are signing, so it must rehydrate that message and examine it, regardless of any out-of-band claim of what it is.

[bugbytesinc]

Also, the messages in question are the outer wrappers and buried inside are the XXXBodyBytes for each specific message, are you saying that GO is unable to parse a one of field construct from a protobuf message, if this is the case, I would think that would be a terrible ommission on Go. Since Go is from google, along with protobuf and gRPC, I'm confused as to why there would be a lack of parity with this language.

[robkainz]

FYI, as for the golang it's not an issue. You can use (type) in a switch statement like I do here:

func ConvertHederaTransaction(transaction interface{}) (interface{}, error) {
     switch transaction.(type) {
        case hedera.AccountCreateTransaction:
            t := transaction.(hedera.AccountCreateTransaction)
            return ConvertHederaAccountCreateTransaction(&t)
        case hedera.AccountDeleteTransaction:
            t := transaction.(hedera.AccountDeleteTransaction)
            return ConvertHederaAccountDeleteTransaction(&t)
...

[hgraphql]

Coming to conclusion on this discussion, I believe we're at a consensus where NOT including is the best path.

Regarding ethereum transactions - we'll want this to go into the eip155 namespace (which we're currently developing)

For the utility of knowing the type before re-constructing from bytes, we're going forward with the pattern of deriving transaction type vs. defining to keep the payload concise and reducing error potential. (i.e one could just say the wrong transactionType)

[hgraphql]

Though, "Last call" on strong differing opinions on this to make sure we're not missing anything

[hgraphql]

Starting a separate thread specifically on this comment by @gregscullard as this is good to call out specifically.

I'm also thinking that (while out of scope just now) it may help future proof the implementation by enabling an "ethereum" transaction type so that wallets know to send the signed transaction to a JSON/RPC relay rather than a consensus node.

Wallets that implement using the Hedera-Ethereum JSON-RPC relay should be able to specify support via:

// ------- namespaces builder util ------------ //
const approvedNamespaces = buildApprovedNamespaces({
    proposal: params,
    supportedNamespaces: {
        hedera: {...},
        eip155: {...}
    },
});
// ------- end namespaces builder util ------------ //

So unless we uncover something while implementing, I think we should be able to keep the hedera_XXX namespace strictly hedera native services

[hgraphql]

Here's one more detail. A wallet that may implement both the eip155 namespace and the hedera namespace, might have a WalletConnect object like this:

 requiredNamespaces: {
    eip155: {
      methods: [
        'eth_sendRawTransaction',
      ],
      chains: ['eip155:295'],
      events: ['chainChanged', 'accountsChanged']
    },
    hedera: {
      methods: [
        'hedera_signTransactionAndSend',
      ],
      chains: ['hedera:mainnet'],
      events: ['chainChanged', 'accountsChanged']
    }
  }

it appears that the WalletConnect repo will need to be updated to utilize the chain identifiers as specified here - https://deploy-preview-820--hedera-hips.netlify.app/hip/hip-820#specification

Currently, the hedera namespace is using the EIP155 namespace

[hgraphql]

A topic that has been brought up is the concept of a user defined field:

The user story might be "As a wallet, I want to provide bespoke functionality only applicable to users and dApps of my wallet."

Is this something we want to add to the spec or set a precedent of how to achieve this in WalletConnect? At the end of the day, dApps & wallets can send data back and forth whatever data through the messaging relay, though it might be helpful to create a field within certain functions that would accept user defined fields.

From a conceptual level, the idea has roots in User Defined Fields in the FIX protocol - https://www.fixtrading.org/standards/user-defined-fields/

This would provide a way to extend the standard.

[teacoat]

I believe this was discussed previously, and it was decided to focus on getting the core functionality working before adding

[hgraphql]

precheck code vs. transaction response for transaction methods.

the hedera docs for the javascript/go/java sdks support a response object that has transaction id, node id, and transaction hash - https://docs.hedera.com/hedera/sdks-and-apis/sdks/transactions/submit-a-transaction

we may ease some burden by returning this instead of the precheck code?

@bugbytesinc does the .Net sdk support this format as well?

I understand being as close to the HAPI responses as possible for payloads, though I wonder if in practice we're re-implementing this payload on the dApp side?

[teacoat]

+1 just return the response

[bugbytesinc]

I think there may be some confusion regarding the sendXXX methods. They do not wait for a receipt, what the HAPI returns in this case is a TransactionResponse:

/**
 * When the client sends the node a transaction of any kind, the node replies with this, which
 * simply says that the transaction passed the precheck (so the node will submit it to the network)
 * or it failed (so it won't). If the fee offered was insufficient, this will also contain the
 * amount of the required fee. To learn the consensus result, the client should later obtain a
 * receipt (free), or can buy a more detailed record (not free).
 */
message TransactionResponse {
	/**
	 * The response code that indicates the current status of the transaction.
	 */
	ResponseCodeEnum nodeTransactionPrecheckCode = 1;

	/**
     * If the response code was INSUFFICIENT_TX_FEE, the actual transaction fee that would be
     * required to execute the transaction.
	 */
	uint64 cost = 2;
}

The cost property on that response has been determined to be generally not reliable, so the only info of value is the ResponseCodeEnum aka the Precheck response. There has been some disussion (which needs to be included in here) on the utility of adding two methods that do wait for a reciept, (we'll get to that soon) I think those two potential methods may return a TransactionReceipt which may include the info you refer to. Although on upon further reflection on the docs, I think what you're looking at is some behind-the-scenes slight of hand with the javascript sdk keeping some state for you for convenience. The protocol does not return any of that info with the precheck reponse at the wire level.

[bugbytesinc]

Adding the following to the wire protocol return values:

transactionId – a string encoded transaction identifier of the signed message that was sent to the Hedera node, in <shard>.<realm>.<number>@<seconds>.<nanos> format.

nodeId – a string encoded transaction identifier of the Hedera Gossip Node's Account that the transaction was submitted to, in <shard>.<realm>.<number> format.

transactionHash– a base64 encoding of the SHA384 digest of the signedTransactionBytes of the Transaction message that was submitted to the Hedera Network.