Skip to content
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

Closed
MiguelLZPF opened this issue May 6, 2024 · 28 comments · Fixed by #125
Closed

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

MiguelLZPF opened this issue May 6, 2024 · 28 comments · Fixed by #125
Labels
Bug A error that causes the feature to behave differently than what was expected based on design docs P1 High priority issue. Required to be completed in the assigned milestone.

Comments

@MiguelLZPF
Copy link
Contributor

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):

  • OS: MacOS
  • Browser: Brave (chrome)
@tmctl
Copy link
Contributor

tmctl commented Jul 24, 2024

@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

@MiguelLZPF
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

tmctl commented Jul 30, 2024

okay, thanks @MiguelLZPF

I'll prioritize getting this over the line

@MiguelLZPF
Copy link
Contributor Author

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.

@itsbrandondev itsbrandondev added Bug A error that causes the feature to behave differently than what was expected based on design docs P1 High priority issue. Required to be completed in the assigned milestone. labels Aug 3, 2024
@hgraphql hgraphql reopened this Nov 7, 2024
@hgraphql
Copy link
Contributor

hgraphql commented Nov 7, 2024

There is an open PR to revert the change that fixed this issue, though has been reported to not work with wallets in production.

#325

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.

@MiguelLZPF
Copy link
Contributor Author

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

@kantorcodes
Copy link
Contributor

@MiguelLZPF

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.

@jaime-iobermudez
Copy link

Hi @kantorcodes here you have a script where you can test both version changes (1.3.4 the one with Miguel changes)
and the (1.3.6 with the revert of the 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();

@kantorcodes
Copy link
Contributor

Hi @kantorcodes here you have a script where you can test both version changes (1.3.4 the one with Miguel changes) and the (1.3.6 with the revert of the 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.

@kantorcodes
Copy link
Contributor

kantorcodes commented Nov 26, 2024

MiguelLZPF

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
      )

@kantorcodes
Copy link
Contributor

kantorcodes commented Nov 26, 2024

@MiguelLZPF I've created a new canary release if you want to test this one out: 1.4.1-canary.abcf6e8.0

It updates dAppConnector to ensure it utilizes signTransaction in the same way as the DAppSigner

https://www.npmjs.com/package/@hashgraph/hedera-wallet-connect/v/1.4.1-canary.abcf6e8.0

@MiguelLZPF
Copy link
Contributor Author

Thank you @kantorcodes ! We will try it and come back to you with the results

@jaime-iobermudez
Copy link

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();

@jaime-iobermudez
Copy link

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.

@kantorcodes
Copy link
Contributor

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?

@jaime-iobermudez
Copy link

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:
https://github.com/hashgraph/stablecoin-studio/blob/main/sdk/src/port/out/hs/multiSig/MultiSigTransactionAdapter.ts#L71

If you want to check the sign code is here :
https://github.com/hashgraph/stablecoin-studio/blob/main/sdk/src/port/out/hs/walletconnect/HederaWalletConnectTransactionAdapter.ts#L343 (using the 1.3.4 HWC version)

@kantorcodes
Copy link
Contributor

kantorcodes commented Nov 28, 2024

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:
https://github.com/hashgraph/stablecoin-studio/blob/main/sdk/src/port/out/hs/multiSig/MultiSigTransactionAdapter.ts#L71

If you want to check the sign code is here :
https://github.com/hashgraph/stablecoin-studio/blob/main/sdk/src/port/out/hs/walletconnect/HederaWalletConnectTransactionAdapter.ts#L343 (using the 1.3.4 HWC version)

To avoid any doubt:

Can you utilize this canary release?

"@hashgraph/hedera-wallet-connect": "1.4.1-canary.abcf6e8.0",

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?

@jaime-iobermudez
Copy link

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

https://github.com/hashgraph/stablecoin-studio/blob/main/backend/src/jobs/autoSubmit.service.ts#L143

@kantorcodes
Copy link
Contributor

kantorcodes commented Nov 28, 2024

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

https://github.com/hashgraph/stablecoin-studio/blob/main/backend/src/jobs/autoSubmit.service.ts#L143

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.

@jaime-iobermudez
Copy link

We get an INVALID_SIGNATURE error like this:

Error submitting the transaction : transaction [email protected] failed precheck with status INVALID_SIGNATURE

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:

      const transactionBytes = transaction.toBytes();
      const transactionFromByes = Transaction.fromBytes(transactionBytes); 

And if you try it with the original transaction object it works fine

      const transaction = new TopicCreateTransaction()
       .setTransactionId(transactionId)
       .freezeWith(Client.forTestnet());

@rocketmay
Copy link

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!

// create the transaction
unsignedTx = new TransferTransaction()
          .addHbarTransfer(signerAddress, -Hbar.from(data.hbarAmount, HbarUnit.Tinybar)._valueInTinybar)
          .addHbarTransfer(recipientAddress, Hbar.from(data.hbarAmount, HbarUnit.Tinybar)._valueInTinybar)
          .setTransactionMemo(data.memo)
          .setTransactionId(transactionId)
          .setNodeAccountIds(nodeIds)   
          .freeze();

// save to bytes for storage
const transaction_bytes = Buffer.from(unsignedTx.toBytes()).toString('base64');

// Create a signature map for async signing
const signatureMap = await signTransaction(transaction_bytes); // see below for signTransaction()

const sigMapString = signatureMap.toString();
const base64encoded = Buffer.from(sigMapString).toString('base64');

// store the signature for later execution
const signatureToStore = base64encoded;

The transaction_bytes and signatureToStore is stored for future usage. We can take the transaction_bytes and sign using multiple keys this way.

// Later on when we want to execute the transaction with all the signatures
  
  // Add all the signatures to the transaction
  const baseTransaction = Transaction.fromBytes(transaction_bytes);
  
  // Create a fresh transaction and add all signatures at once
  const frozenTx = baseTransaction
    .freeze();
  
  // an array of signatures for multi-sig signing
  // this is where the `signatureToStore` is eventually used for each signature. TransactionSigner also includes the accountId and publicKey.
  const signatures:TransactionSigner[] = {...} // define these according to the TransactionSigner interface near the end of this comment.

// Add all signatures to the frozen transaction
  // Add each signature with its own SignatureMap
  
  console.log("Adding signatures for transaction : " + transaction.uuid)
  signatures.forEach(sig => {
    const sigMap = createSignatureMap(sig, frozenTx); // See createSignatureMap() below
    const publicKey = PublicKey.fromString(sig.publicKey);
    console.log("Signature map created:", sigMap, sig.publicKey);
    frozenTx.addSignature(publicKey, sigMap);
  });

  console.log("Signatures added.");

  const signedTransactionBytes = frozenTx.toBytes();
  const finalTransaction = Transaction.fromBytes(signedTransactionBytes);

  let response: TransactionResponse;
  response = await finalTransaction.execute(client);

Here's my signTransaction function:

const signTransaction = async (transaction_bytes:Uint8Array): Promise<Uint8Array>  => {

    const transaction = Transaction.fromBytes(transaction_bytes);

    // get the signer from walletConnect . Obviously if you have a local hedera sdk signer you can just use that.
    const signer:Signer = walletConnect.dAppConnector.signers.find(
        (signer_) => signer_.getAccountId().toString() === accountId.toString()
    ) as Signer;

    const signedTransaction = await signer.signTransaction(transaction);
 
    // Access the internal signed transaction list
    const signedTransactions = (signedTransaction as any)._signedTransactions;
    if (!signedTransactions?.list?.[0]) {
      throw new Error("No signed transaction data found");
    }

    // Get the first signed transaction
    const firstSignedTx = signedTransactions.list[0] as SignedTransaction;
    console.log("First signed transaction:", firstSignedTx);

    // The signature should be in the sigMap
    if (!firstSignedTx.sigMap) {
      throw new Error("No signature map found in signed transaction");
    }

    // Log the sigMap for debugging
    console.log("RAW Signature Map:", JSON.stringify(firstSignedTx));

    // Extract the signature - it should be in the signature pairs
    const sigPairs = firstSignedTx.sigMap.sigPair;
    if (!sigPairs || sigPairs.length === 0) {
      throw new Error("No signatures found in signature map");
    }

    // Define the public key for the signer
    const ourPubKeyHex = getHexFormattedPubKey();     // I retrieve this data asynchronously and return the HEX formatted public key.

    // Check for either Ed25519 or ECDSA signature
    let signature: Uint8Array;
    
    // Find the signature pair that matches our public key
    const matchingSig = sigPairs.find(pair => 
      pair.pubKeyPrefix && Buffer.from(pair.pubKeyPrefix).toString('hex') === ourPubKeyHex
    );
    console.log("Matching signature pair full structure:", matchingSig);
    
    if (!matchingSig) {
      throw new Error("Could not find signature matching our public key");
    }
    if (matchingSig.ed25519) {
      console.log("Found Ed25519 signature");
      signature = matchingSig.ed25519;
    } else if (matchingSig.ECDSASecp256k1) {
      console.log("Found ECDSA signature");
      signature = matchingSig.ECDSASecp256k1;
    } else {
      throw new Error("No valid signature found (neither Ed25519 nor ECDSA)");
    }

    console.log("Found signature:", Buffer.from(signature).toString('hex'));
    return signature;

    } catch (e){
      throw e;
    }

 
}

createSignatureMap() recreates a signature map from a saved signature:

// TransactionSigner is a convenience interface that holds the accountId, publicKey, and the base64 encoded signature from above.
  interface TransactionSigner {
      accountId: string;
      publicKey: string;
      signature: string;
  }

private createSignatureMap(signature: TransactionSigner, transaction: Transaction): SignatureMap {
    // First decode the base64 signature
    const base64Decoded = Buffer.from(signature.signature, 'base64').toString();

    console.log("Decoded signature:", base64Decoded);
    
    // Convert the comma-separated string to a byte array
    const signatureBytes = new Uint8Array(
      base64Decoded.split(',').map(num => parseInt(num))
    );

    console.log("Signature bytes:", signatureBytes);

    if (!transaction.nodeAccountIds?.[0] || !transaction.transactionId) {
      throw new Error("Transaction must have nodeAccountIds and transactionId");
    }

    // Create a new SignatureMap
    const sigMap = new SignatureMap();
    const pubKey = PublicKey.fromString(signature.publicKey);
    
    sigMap.addSignature(
      transaction.nodeAccountIds[0],
      transaction.transactionId,
      pubKey,
      signatureBytes
    );

    console.log("Signature map created:", sigMap);
    
    return sigMap;
  }

@rocketmay
Copy link

And here's a rough plaintext explanation of what's happening in the code:

  1. Create and freeze an unsigned transaction (in this case a TransferTransaction)
  2. Convert the frozen transaction to bytes and encode as base64 for storage
  • This allows the transaction to be stored and signed by multiple parties later
  1. Get initial signature by calling signTransaction() function which:
  • Takes transaction bytes
  • Gets a signer (in this case from WalletConnect)
  • Signs the transaction
  • Extracts the signature from the SignatureMap
  • Returns the signature bytes
  1. Convert the signature to base64 and store it
  2. When ready to execute with all signatures:
  • Reconstruct the base transaction from the stored bytes
  • Freeze it again
  • For each stored signature:
    • Decode the base64 signature
    • Create a new SignatureMap using the signature bytes
    • Add the signature to the frozen transaction using the corresponding public key
  1. Convert the fully signed transaction to bytes
  2. Create final transaction from those bytes
  3. Execute the transaction

@kantorcodes
Copy link
Contributor

@jaime-iobermudez @MiguelLZPF

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?

@jaime-iobermudez
Copy link

Hi @kantorcodes , I'm testing but today the testnet is not helping, I will let you know if the proposal works.

@kantorcodes
Copy link
Contributor

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 :)

@jaime-iobermudez
Copy link

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!!!!!!

@kantorcodes
Copy link
Contributor

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 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A error that causes the feature to behave differently than what was expected based on design docs P1 High priority issue. Required to be completed in the assigned milestone.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants