-
Notifications
You must be signed in to change notification settings - Fork 22
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
Error verifying the signature using DAppConnector.signTransaction() #124
Comments
@MiguelLZPF did this PR solve the issue for you - #170 the most recent sdk version has these changes. If not, we'll continue to test the PR you've added |
Hey Tyler @tmctl! 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? |
okay, thanks @MiguelLZPF I'll prioritize getting this over the line |
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. |
There is an open PR to revert the change that fixed this issue, though has been reported to not work with wallets in production. This issue remains to find a solution that both works with the current implementation without breaking changes and works with StableCoin Studio - https://github.com/hashgraph/stablecoin-studio Let's add a PR into that repository if that is what is needed. |
Hi! Great. It might be worth taking a closer look at this issue, as simply rolling back the changes may not be the final solution. Before PR 126, the Hedera Testnet itself couldn’t validate a signed transaction created with this method. Ideally, we can identify the underlying issue and work toward a real solution where the signatures are fully valid. @hgraphql |
By any chance could you provide me with a set of minimally reproducible code snippets so we can try this irregardless of implementation, e.g. the most bare bones example outside of stablecoin studio? I am looking into this. |
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:
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):
The text was updated successfully, but these errors were encountered: