klaytn / caver-js

Official caver-js repository
GNU Lesser General Public License v3.0
114 stars 75 forks source link

How to append signatures to a built transaction? #513

Closed codemaster101 closed 3 years ago

codemaster101 commented 3 years ago

Freely describe your questions or issues here. Hi, as part of the Klaytn integration, we support the signing of transactions using digital signatures (without the use of exposed private keys). We have managed to create the built transaction using const builtTx = valueTransfer.create(tx)

Followed by:

await builtTx.fillTransaction();
const rlpHash = builtTx.getRLPEncoding();

const digSig = await createDigitalSignature(rlpHash) // rs low v

My question is does caver-js support a way to append this signature or use it to sign the transaction instead of using the private key?

// Something like this
const signedTx = builtTx.appendSignature(digSig);
rpc.sendRawTransaction(signedTx);

Would be grateful if you all could point us in the right direction with respect to digital signatures

codemaster101 commented 3 years ago
    await builtTransaction.fillTransaction();
    const rlpHash = builtTransaction.getRLPEncoding().slice(2); // Slice to remove 0x while creating a buffer

    const [result] = external.sign({
      uuid: this.uuid,
      data: Buffer.from(rlpHash, 'hex'),
    });

    if (result && Buffer.isBuffer(result.signature)) {
      const signatureObject = Caver.utils.transformSignaturesToObject(`0x${result.signature.toString('hex')}`);
      const signatureArray = Object.values(signatureObject);
      const signedTx = builtTransaction.appendSignatures(signatureArray);

      return {
        success: true,
        rawTx: signedTx,
      };
    }

But seems when I try to submit this transaction I run into the following error: Returned error: invalid argument 0: json: cannot unmarshal non-string into Go value of type hexutil.Bytes

Any help would be much appreciated

jimni1222 commented 3 years ago

tx.appenedSignatures will return void.

So you can try just calling tx.appendSignatures and then return rawTx with tx.getRLPEncoding(). You can check signatures field after calling tx.appendSignatures.

codemaster101 commented 3 years ago

@jimni1222 Thanks, tx.appendSignatures indeed returns void. But when I change it to getRLPEncoding() I run into the following error: Returned error: invalid transaction v, r, s values.

Now I know, that the signature might be wrong, but we use the same signature generator for ethereum and a bunch of other blockchains. What I would like to confirm is:

  1. When I get the RLPEncoding of the built transaction, do I create a signature out of that unsigned transaction hash?
  2. The signature is returned as a buffer, in the RS LOW V format. Is that format correct for the signature?

The signed TX being generated is:

    ValueTransfer {
      _type: 'TxTypeValueTransfer',
      _from: '0xb82cffb9dd11ae01734983bd5c560c27c12e59ec',
      _gas: '0x5209',
      _signatures: [
        SignatureData {
          _v: '0x1b',
          _r: '0x42b5eaef06f244179bb1b1395be362e15ee18f03e8fa835078261fd46f0d4c15',
          _s: '0x572748183cb4dcb6e88eb0234a14577ee66c71ff8f8af2ff10913fd892a5d364'
        }
      ],
      _to: '0x626242eb6cab3d5ce443ebd846292208d56d5e41',
      _value: '0x1',
      _chainId: '0x3e9',
      _gasPrice: '0x5d21dba00',
      _nonce: '0x0'
    }

Any idea what could be wrong here?

codemaster101 commented 3 years ago

@jimni1222 We tried implementing it this way as well:

    await builtTransaction.fillTransaction();
    const transactionHash = TransactionHasher.getHashForSignature(builtTransaction).slice(2);
    console.log(transactionHash);
    // const rlpHash = builtTransaction.getRLPEncoding().slice(2); // Slice to remove 0x while creating a buffer

    const [result] = await external.sign({
      uuid: this.uuid,
      data: Buffer.from(transactionHash, 'hex'),
    });

    if (result && Buffer.isBuffer(result.signature)) {
      const signatureObject = Caver.utils.transformSignaturesToObject(`0x${result.signature.toString('hex')}`);
      const signatureArray = Object.values(signatureObject);
      builtTransaction.appendSignatures(signatureArray);
      const signedTx = builtTransaction.getRLPEncoding();

      return {
        success: true,
        rawTx: signedTx,
      };
    }

But we still run into the error of invalid v, r, s

Returned error: invalid transaction v, r, s values Is the hash that I am creating correct? Or is this the way to do it for digital signatures?

codemaster101 commented 3 years ago

It seems it returns two different error messages: invalid transaction v, r, s values and invalid transaction v, r, s values of the sender.

What I get from this is that the signature we are generating is incorrect. But, since this has worked repeatedly for numerous blockchains (ETH for ex.) with (RS Low V, which hopefully you can confirm), it has to do something with how we create the hash for the signature.

I have tried to Different ways of generating the hash but to no avail. @jimni1222 would be great if you could guide us through this as the deadline for the Klaytn integration is coming up soon.

codemaster101 commented 3 years ago

Also, I see the following line of code: Nat.toNumber(chainId) * 2 + 35 in the sign function in keyring/privateKey.js. Is this something we have to do when creating the signature. Should I manually change the value of chainId in the transaction object before sending it to create the external digital signature?

jimni1222 commented 3 years ago

That errors are came from Klaytn.

What you gave to me example is actually has invalid signatures

    ValueTransfer {
      _type: 'TxTypeValueTransfer',
      _from: '0xb82cffb9dd11ae01734983bd5c560c27c12e59ec',
      _gas: '0x5209',
      _signatures: [
        SignatureData {
          _v: '0x1b',
          _r: '0x42b5eaef06f244179bb1b1395be362e15ee18f03e8fa835078261fd46f0d4c15',
          _s: '0x572748183cb4dcb6e88eb0234a14577ee66c71ff8f8af2ff10913fd892a5d364'
        }
      ],
      _to: '0x626242eb6cab3d5ce443ebd846292208d56d5e41',
      _value: '0x1',
      _chainId: '0x3e9',
      _gasPrice: '0x5d21dba00',
      _nonce: '0x0'
    }

You can check via caver.validator.validateTransaction.

This is recover public key from signatures and compare public key which is stored in Klaytn Account data structure in the Klaytn network.

I think other logic looks fine, and also i checked with converting signatures buffer -> string -> object types. That means, actually external.sign returns invalid signature in Klaytn network.

It's hard to help you because i don't know the internal implementation of external.sign or the actual return value. If you prepare an environment that can reproduce the error, i can debug.

jimni1222 commented 3 years ago

@codemaster101 If there is not additional issue for this, i will close. Please feel free to reopen this issue.

codemaster101 commented 3 years ago

Hi @jimni1222 well noted, will update our external signer accordingly. However, we ran into an issue with the signer provided by caver-js as well.

Returned error: invalid transaction v, r, s values of the sender

The following is our code:

Unsigned Transaction Object Generated by the Package:

    ValueTransfer {
      _type: 'TxTypeValueTransfer',
      _from: '0x17fbbc15e23cc885334b1c582b914e71ca2cc484',
      _gas: '0x5209',
      _nonce: '0x1f',
      _gasPrice: '0x5d21dba00',
      _chainId: '0x3e9',
      _signatures: [ SignatureData { _v: '0x01', _r: '0x', _s: '0x' } ],
      _to: '0x626242eb6cab3d5ce443ebd846292208d56d5e41',
      _value: '0x1'
    }

Creating signature data from the package: key.sign is using this function: https://github.com/klaytn/caver-js/blob/dev/packages/caver-wallet/src/keyring/privateKey.js#L65

const signatureData = this.wallet.key.sign(unsignedTx.getTransactionHash(), transactionData.chainId);
unsignedTx.appendSignatures(signatureData);

Which generates the following signed transaction:

    ValueTransfer {
      _type: 'TxTypeValueTransfer',
      _from: '0x17fbbc15e23cc885334b1c582b914e71ca2cc484',
      _gas: '0x5209',
      _nonce: '0x1f',
      _gasPrice: '0x5d21dba00',
      _chainId: '0x3e9',
      _signatures: [
        SignatureData {
          _v: '0x07f6',
          _r: '0x8ac830ef1146873e7997b166e5dfe4d62ca7753ff681adf8762fd02b9aab5948',
          _s: '0x63055ca06895e570f9cd79d47103e9441e02b1904f439c98272b36be7562b870'
        }
      ],
      _to: '0x626242eb6cab3d5ce443ebd846292208d56d5e41',
      _value: '0x1'
    }

Keep in mind, this.wallet.key.getDerivedAddress() (from the privateKey class) does return the correct address which is: 0x17fbbc15e23cc885334b1c582b914e71ca2cc484.

And then we finally send the transaction:

const sentTx = await caver.rpc.klay.sendRawTransaction(unsignedTx); // ignore the naming for this example

But this returns the error:

Unable to sign transaction. Error: Returned error: invalid transaction v, r, s values of the sender

Is it possible that the sign transaction is expecting something else? I tried:

const signatureData = this.wallet.key.sign(unsignedTx.getRLPEncoding(), transactionData.chainId);

But no luck!

Any help would be greatly appreciated.

codemaster101 commented 3 years ago

This is weird because everything we do is internal to the package, but maybe the function calls are not right or maybe it is expecting something else?

codemaster101 commented 3 years ago

Also what is weird is that we can use the RPC signer to sign this transaction BUT not the signer in the package. But unfortunately we cannot use RPC as this is offline signing. @jimni1222

codemaster101 commented 3 years ago

Also does not work with:

      const signatureData = this.wallet.sign(
        unsignedTx.getTransactionHash(),
        transactionData.chainId,
        0,
        0,
      );

@jimni1222

jimni1222 commented 3 years ago

The transactionHash and transactionHashForSigning are different. You can see this documentation.

So what you need to use for getting transactionHashForSigning is TransactionHasher.getHashForSignature that you used before.

And the getDerivedAddress function simply returns the address derived from the privateKey, regardless of the key actually used by the account on the Klaytn network. However, if it is correct that you use the Baobab network, the key of the account 0x17fbbc15e23cc885334b1c582b914e71ca2cc484 is searched as LegacyAccountKey, so it is correct to use the corresponding key because it is an account that has not been updated.

You can try below way.

const signatureData = this.wallet.key.sign(TransactionHasher.getHashForSignature(unsignedTx), unsignedTx.chainId);
unsignedTx.signatures = signatureData; // or you can use `appendSignatures`, but in this case you will use only one signature so i think using setter of the `signatures` also works.
const receipt = await caver.rpc.klay.sendRawTransaction(unsignedTx);
codemaster101 commented 3 years ago

@jimni1222 thank you so much!!!

ksjang0515 commented 2 years ago

getRLPEncodingForSignature