search

hashgraph / hedera-wallet-connect

This package is a messaging relay between decentralized applications and wallets in Hedera network based on Wallet Connect relays.
Apache License 2.0
8 stars 16 forks source link

Error verifying the signature using DAppConnector.signTransaction() #124

Open MiguelLZPF opened 4 months ago

MiguelLZPF commented 4 months ago

Describe the bug A clear and concise description of what the bug is.

In our testing of the signTransaction functionality from ioBuilders for integration into the Hedera Stablecoin project, we have encountered an issue with signature verification. The current implementation fails to verify signatures within our testing environment. Interestingly, we have successfully verified signatures using Blade, HasPack, and Metamask wallets.

In an attempt to address this, we modified our code to verify transactions created with hedera-wallet-connect against a customized verification function. However, when these transactions were submitted to the Hedera Network for verification, they were rejected. This discrepancy leads us to suspect that the signTransaction method may be producing incorrect signatures, possibly due to inaccuracies in the signed bytes generated.

To Reproduce Steps to reproduce the behavior:

  1. Clone the Hedera Stablecoin repository
  2. Setup Stablecoin with a backend for multi-signature instance
  3. Use a multi-signature account to create a cash in transaction (for example)
  4. Use a Wallet Connect connection wallet to sign the previously created transaction
  5. Verification exception should be thrown from the backend

Expected behavior A clear and concise description of what you expected to happen.

The same verification process should verify signatures from hedera-wallet-connect.

Desktop (please complete the following information):

tmctl commented 1 month ago

@MiguelLZPF did this PR solve the issue for you - https://github.com/hashgraph/hedera-wallet-connect/pull/170

the most recent sdk version has these changes.

If not, we'll continue to test the PR you've added

MiguelLZPF commented 1 month ago

Hey Tyler @tmctl! Yes, we checked it last week but didn't have the chance to try it out yet. I believe it improves how the node account is managed, with the main change being the payload that needs to be signed, if I recall correctly.

I'll try the new version today. If we still need the changes in this PR, I'll update it with the latest modifications. Thank you for the heads up.

By "most recent SDK version," you're referring to the Hedera Wallet Connect repository, right?

MiguelLZPF commented 1 month ago

Unfortunately, PR #170 does not provide a correct signature, resulting in an "Invalid signature" error with version 1.3.2-1 of hedera-wallet-connect. Therefore, we will incorporate the changes into PR #125.

tmctl commented 1 month ago

okay, thanks @MiguelLZPF

I'll prioritize getting this over the line

MiguelLZPF commented 1 month ago

To provide more clarity, the main issue is that the payload being signed is incorrect. As a result, when attempting to validate the signature, the two hashes do not match, leading to an invalid signature. The node ID is irrelevant in this context, maybe if there were more than one signature or whatever it is relevant, I don't know.

Additionally, based on my experience with the code, the tests currently in place only check if something is signed. They do not verify the validity of the signature. Therefore, these test signatures are likely invalid. I think that it is a good idea to implement tests that actually verify the signatures to ensure their correctness.