-
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. |
Hi @kantorcodes here you have a script where you can test both version changes (1.3.4 the one with Miguel changes) The same shows how the verification of the signature generated by the 1.3.6 version fails and also fails the execute of the transaction. import {
AccountId,
Client,
LedgerId,
PublicKey,
TopicCreateTransaction,
Transaction,
TransactionId,
} from "@hashgraph/sdk";
import {
base64StringToSignatureMap,
DAppConnector as HwcDAppConnector,
transactionToTransactionBody,
Uint8ArrayToBase64String,
type SignTransactionParams,
} from "@hashgraph/hedera-wallet-connect";
import * as nacl from "tweetnacl";
import { proto } from "@hashgraph/proto";
async function main() {
try {
// Initialize DAppConnector for Hedera Wallet Connect
const dAppConnector = new HwcDAppConnector(
{
name: "",
description: "",
url: "",
icons: [],
},
LedgerId.fromString("testnet"),
"8fc26370383a50de1c3bd638d334292e"
);
// Init dAppConnector
await dAppConnector.init({ logger: "debug" });
// Connect your wallet
await dAppConnector.connectQR();
// Retrieve account ID from connected wallet
const accountId = dAppConnector.signers[0].getAccountId().toString();
// Create a TopicCreateTransaction and freeze it
const transactionId = TransactionId.generate(accountId);
const transaction = new TopicCreateTransaction()
.setTransactionId(transactionId)
.freezeWith(Client.forTestnet());
// Generate TransactionBody for Hedera Wallet versions
const transactionBody1_3_6 = transactionToTransactionBody(transaction, AccountId.fromString("0.0.3"));
if (!transactionBody1_3_6) throw new Error("Transaction is null or undefined HWC (1.3.6)");
const transactionBody1_3_4 = transactionToTransactionBody1_3_4(transaction, AccountId.fromString("0.0.3"));
if (!transactionBody1_3_4) throw new Error("Transaction is null or undefined HWC (1.3.4)");
// Prepare sign parameters
const signParams1_3_6: SignTransactionParams = {
transactionBody: transactionBodyToBase64String1_3_6(transactionBody1_3_6),
signerAccountId: `hedera:testnet:${accountId}`,
};
const signParams1_3_4: SignTransactionParams = {
transactionBody: transactionBodyToBase64String1_3_4(transactionBody1_3_4),
signerAccountId: `hedera:testnet:${accountId}`,
};
// Sign the transaction using both versions
const signResult1_3_6 = await dAppConnector.signTransaction(signParams1_3_6);
console.log(`✅ Transaction signed successfully HWC (1.3.6)!`);
const signResult1_3_4 = await dAppConnector.signTransaction(signParams1_3_4);
console.log(`✅ Transaction signed successfully HWC (1.3.4)!`);
// Decode signature maps
const signatureMap1_3_6 = base64StringToSignatureMap((signResult1_3_6 as any).signatureMap);
const signatureMap1_3_4 = base64StringToSignatureMap((signResult1_3_4 as any).signatureMap);
// Validate and extract the first signatures
const firstSignature1_3_6 = extractFirstSignature(signatureMap1_3_6);
const firstSignature1_3_4 = extractFirstSignature(signatureMap1_3_4);
// Fetch the public key from the mirror node
const { key: publicKeyString } = await getPublicKey(accountId);
// Deserialize the transaction and prepare verification data
const bytesToSign = transaction._signedTransactions.get(0)!.bodyBytes!;
const publicKeyBytes = PublicKey.fromString(publicKeyString).toBytes();
// Verify signatures for both versions
console.log(
`HWC 1.3.6 Signature verification result: ${nacl.sign.detached.verify(
bytesToSign,
firstSignature1_3_6,
publicKeyBytes
)}`
);
console.log(
`HWC 1.3.4 Signature verification result: ${nacl.sign.detached.verify(
bytesToSign,
firstSignature1_3_4,
publicKeyBytes
)}`
);
//Uncomment this to check that with the signature generate with the 1.3.6 version the transaction fails
const transactionSigner = async (message: Uint8Array): Promise<Uint8Array> => {
return firstSignature1_3_6;
};
// Uncomment this to check that with the signature generate with the 1.3.4 version the transaction is working properly
// const transactionSigner = async (message: Uint8Array): Promise<Uint8Array> => {
// return firstSignature1_3_4;
// };
const signTx = await transaction.signWith(PublicKey.fromString(publicKeyString), transactionSigner);
const txResponse = await signTx.execute(Client.forTestnet());
console.log(`The transaction id is: ${txResponse.transactionId}`);
} catch (error) {
console.error("Error during connection:", error);
} finally {
process.exit();
}
}
// Extract the first valid signature from the signature map
function extractFirstSignature(signatureMap: proto.ISignatureMap): Uint8Array {
const firstSignature =
signatureMap.sigPair[0].ed25519 ||
signatureMap.sigPair[0].ECDSASecp256k1 ||
signatureMap.sigPair[0].ECDSA_384;
if (!firstSignature) throw new Error("No signatures found in response");
return firstSignature;
}
// Fetch public key from the Hedera mirror node
async function getPublicKey(accountId: string, network = "testnet") {
try {
const response = await fetch(
`https://${network}.mirrornode.hedera.com/api/v1/accounts/${accountId}`
);
const data = await response.json();
return { key: data?.key?.key, type: data?.key?._type };
} catch (error) {
console.error("Failed to fetch public key:", error);
throw error;
}
}
// Convert transaction body to base64 (used in HWC 1.3.6)
function transactionBodyToBase64String1_3_6(transactionBody: proto.ITransactionBody) {
return Uint8ArrayToBase64String(proto.TransactionBody.encode(transactionBody).finish());
}
// Convert transaction body to base64 (used in HWC 1.3.4)
function transactionBodyToBase64String1_3_4(transactionBody: Uint8Array) {
return Uint8ArrayToBase64String(transactionBody);
}
// Convert transaction to TransactionBody (used in HWC 1.3.4)
function transactionToTransactionBody1_3_4<T extends Transaction>(
transaction: T,
nodeAccountId: AccountId
) {
return transaction._signedTransactions.current.bodyBytes;
}
main(); |
Thank you, this is incredibly helpful. I am testing now. |
I was able to replicate, and also, I think I found a solution that you can use for the moment. Can you try calling signTransaction from the current signer instead of DappConnector? I am still investigating, but seems like that method is robust and keeps the signatures intact. // Retrieve account ID from connected wallet
const selectedSigner = dAppConnector.signers[0]
// Create a TopicCreateTransaction and freeze it
const transactionId = TransactionId.generate(accountId)
const transaction = new TopicCreateTransaction()
.setTransactionId(transactionId)
.freezeWith(Client.forTestnet())
// Sign the transaction using both versions
const signResult1_3_6 = await selectedSigner.signTransaction(transaction)
console.log(`✅ Transaction signed successfully HWC (1.3.6)!`)
const signatureMap1_3_6 = signResult1_3_6._signedTransactions.current.sigMap;
// // Decode signature maps
// const signatureMap1_3_6 = base64StringToSignatureMap((signResult1_3_6 as any).signatureMap)
// Extract first signatures
const firstSignature1_3_6 = extractFirstSignature(signatureMap1_3_6)
// Fetch public key from mirror node
const { key: publicKeyString } = await getPublicKey(accountId)
const bytesToSign = transaction._signedTransactions.get(0)!.bodyBytes!
const publicKeyBytes = PublicKey.fromString(publicKeyString).toBytes()
// Verify signatures
const verify1_3_6 = nacl.sign.detached.verify(
bytesToSign,
firstSignature1_3_6,
publicKeyBytes
) |
@MiguelLZPF I've created a new canary release if you want to test this one out: It updates https://www.npmjs.com/package/@hashgraph/hedera-wallet-connect/v/1.4.1-canary.abcf6e8.0 |
Thank you @kantorcodes ! We will try it and come back to you with the results |
Hi @kantorcodes , trying both proposals (using the current user and the new version of HWC), it seems that the generated signature is valid only when using the transaction directly. What I mean by this, is that in our use case, we store the transaction in the database and then deserialize the transaction from the bytes. Doing that and sending it to the sign method generates an invalid signature. Here is a script you can use to test it: async function main() {
try {
// Initialize DAppConnector for Hedera Wallet Connect
const dAppConnector = new HwcDAppConnector(
{
name: "",
description: "",
url: "",
icons: [],
},
LedgerId.fromString("testnet"),
"8fc26370383a50de1c3bd638d334292e"
);
// Init dAppConnector
await dAppConnector.init({ logger: "debug" });
// Connect your wallet
await dAppConnector.connectQR();
// Retrieve account ID from connected wallet
const accountId = dAppConnector.signers[0].getAccountId().toString();
// Create a TopicCreateTransaction and freeze it
const transactionId = TransactionId.generate(accountId);
const transaction = new TopicCreateTransaction()
.setTransactionId(transactionId)
.freezeWith(Client.forTestnet());
//In StableCoin-Studio we store the transaction bytes in the database and then deserialize them with the fromBytes method
//To test how generates a invalid sign use the Transaction created from the bytes 'transactionFromByes', otherwise just comment the code below and use
//the 'transaction' to check how generates a valid sign
const transactionBytes = transaction.toBytes();
const transactionFromByes = Transaction.fromBytes(transactionBytes);
// Prepare sign parameters
const signParams: SignTransactionParams = {
transaction: transactionFromByes,
signerAccountId: `hedera:testnet:${accountId}`,
};
// Sign the transaction using both versions
const signResult= await dAppConnector.signTransaction(signParams);
console.log(`✅ Transaction signed successfully HWC!`);
// Decode signature maps
const signatureMap = signResult._signedTransactions.current.sigMap;
// Validate and extract the first signatures
const firstSignature = extractFirstSignature(signatureMap!);
// Fetch the public key from the mirror node
const { key: publicKeyString } = await getPublicKey(accountId);
// Deserialize the transaction and prepare verification data
const bytesToSign = transactionFromByes._signedTransactions.get(0)!.bodyBytes!;
const publicKeyBytes = PublicKey.fromString(publicKeyString).toBytes();
//Verify bytes from Transaction and transactionFromByes
const areEqual = transaction.toBytes().toString() === transactionFromByes.toBytes().toString();
console.log(`Transaction and TransactionFromByes bytes are equal ${areEqual}`)
// Verify signatures for both versions
console.log(
`Signature verification result: ${nacl.sign.detached.verify(
bytesToSign,
firstSignature,
publicKeyBytes
)}`
);
} catch (error) {
console.error("Error during connection:", error);
} finally {
process.exit();
}
}
// Extract the first valid signature from the signature map
function extractFirstSignature(signatureMap: proto.ISignatureMap): Uint8Array {
const firstSignature =
signatureMap.sigPair[0].ed25519 ||
signatureMap.sigPair[0].ECDSASecp256k1 ||
signatureMap.sigPair[0].ECDSA_384;
if (!firstSignature) throw new Error("No signatures found in response");
return firstSignature;
}
// Fetch public key from the Hedera mirror node
async function getPublicKey(accountId: string, network = "testnet") {
try {
const response = await fetch(
`https://${network}.mirrornode.hedera.com/api/v1/accounts/${accountId}`
);
const data = await response.json();
return { key: data?.key?.key, type: data?.key?._type };
} catch (error) {
console.error("Failed to fetch public key:", error);
throw error;
}
}
main(); |
To add more context, something that I've noticed is that even though both transactions have the same bytes, if you transform them to JSON, the original one has a property called "_operatorAccountId" that is missing in the JSON generated from the transactionFromByes, not sure if that is causing that the sign method to generate invalid signature. |
Could you share your code on the backend on how you are recreating the transaction and also the code that is storing the transaction? |
To test that, you can use the script I sent you, basically what we are doing is to store in DB the transaction bytes and then generating again the transaction from that bytes. (in the script you have the transactionFromByes that is generated from the bytes of the transaction) This is the method that sends the transaction to the backend: If you want to check the sign code is here : |
To avoid any doubt: Can you utilize this canary release?
Also, when you store the transaction you might need to store the bytes separately from the signature and reconstitute them in your backend. Are you currently doing this? |
I've tried with that version and got the same valid signature error. And about your last question, yes we are storing apart the signature, Here in the code: we add the signs to the transaction and then execute against the hedera network |
And just to confirm, what is the error you get when executing the transaction? I've recently debugged a similar issue and was able to confirm that the signatures are correct and are able to be reconstituted to submit a transaction at a later point, but you have to be very careful with how you store the bytes from both the body and signatures. I won't be able to return to this until Monday, but I suspect at this point the issue is on the handling and not an error on HWC. |
We get an INVALID_SIGNATURE error like this:
Is the same error that returns the test script I've sent (if you execute the transaction) I get your point, but if you run the script, doing a simple transformation like this causes the signature method to return invalid signs:
And if you try it with the original transaction object it works fine
|
Hi @jaime-iobermudez , I've actually been working on this myself and have recently managed to get this working. The implementation used to be a lot simpler when you could use a UINT8Array as a signature in SignTransaction. Now that only SignatureMap is supported, there's a lot of hoops to find the bytes to save the signature and then retrieve it later. I've had to clean up my code to distill the critical steps, so if there's something you don't understand or a mistake somewhere let me know and I'll try to help. I have a bunch of error checking, try/catch statements and API calls (to grab public keys, store to file etc) in the actual working code that wouldn't be useful to include here. Using this code I'm able to asynchronously create a transaction, get signatures from multiple accounts, and execute transaction with all the signatures. Also I have to shout out @kantorcodes for his help tracking down issues and helping me get this working. Hope this helps!
The transaction_bytes and signatureToStore is stored for future usage. We can take the transaction_bytes and sign using multiple keys this way.
Here's my signTransaction function:
createSignatureMap() recreates a signature map from a saved signature:
|
And here's a rough plaintext explanation of what's happening in the code:
|
Hey team, how are you? I just wanted to circle back to see if the comments from @rocketmay were helpful in your implementation or if you're still running into issues? |
Hi @kantorcodes , I'm testing but today the testnet is not helping, I will let you know if the proposal works. |
No problem. Keep us updated so we can close this one out :) |
Hi @kantorcodes, I was able to make it work (using the HWC 1.4.1). Here is the PR if you want to look at the changes: hashgraph/stablecoin-studio#1232. Thanks for your help!!!!!! |
That is exciting to hear 🙌 |
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: