obsidiansystems / ledger-app-nervos

MIT License
10 stars 10 forks source link

Integration on Electron #145

Closed katat closed 4 years ago

katat commented 4 years ago

Hi there,

Great works on the CKB app on ledger!

I am a developer from the Neuron team at Nervos.

Neuron is the official CKB wallet manager, which supports HD wallets and is based on Electron. We are currently evaluating the status of this CKB app on ledger, and I just want to have a quick ask to see if it is ready for the integration on Neuron.

If it is ready, could you please provide us a tutorial or educational materials demonstrating how to interact with the CKB app in JS?

If possible, a prototype on Electron, similar to https://github.com/LedgerHQ/ledgerjs-examples/tree/master/basic-electron-hid, would be highly appreciated for us to play around to see how to interact with the CKB app to get the xpubkey and sign the multiple inputs for a transaction as Neuron does.

Best regards, Kata

mikereinhart commented 4 years ago

Hi Kata! The Ledger app is ready, but we are actively working on LedgerJS support, which you'll need for the integration with your Electron app. We're doing that development here - https://github.com/obsidiansystems/ledgerjs/tree/nervos - and will provide sample code as soon as it is ready.

mikereinhart commented 4 years ago

Hi @katat - please see this example for how to use LedgerJS for Nervos! https://github.com/obsidiansystems/ledgerjs/blob/nervos/packages/hw-app-ckb/examples/sampleapp.js

katat commented 4 years ago

Thanks @mikereinhart for providing the demo code.

I had a play around with the sample code you help provide for the ledgerjs. I got it to work with my physical ledger device by replacing the emulator transport to the hid transport.

Most of the demo functions seem to work well on my macOS catalina. However, based on what I see in the signTransaction code, the way it signs a transaction appears not to be compatible with Neuron. Let me try to explain the differences below with an example.

Here is an example signed transaction that the send_transaction RPC accepts on CKB nodes:

{
  "cellDeps": [
    {
      "outPoint": {
        "txHash": "0xf8de3bb47d055cdf460d93a2a6e1b05f7432f9777c8c474abf4eec1d4aee5d37",
        "index": "0"
      },
      "depType": "depGroup"
    }
  ],
  "headerDeps": [],
  "inputs": [
    // input #1 
    {
      "previousOutput": {
        "txHash": "0xa6366415738c3bfc3a4b3ac7b308dd5fc0e51e3c35e24ca77316b0e2b76c4b76",
        "index": "0"
      },
      "since": "0"
    },
    // input #2
    {
      "previousOutput": {
        "txHash": "0x7ce565c2b019c0b12e1af9afee1059f5ff70873101032cd78a4151ec6b0ee4f5",
        "index": "1"
      },
      "since": "0"
    }
  ],
  "outputs": [
    {
      "capacity": "19999999508",
      "lock": {
        "args": "0xfa246178207211ee6dec035591e1d13aac5fca87",
        "codeHash": "0x9bd7e06f3ecf4be0f2fcd2188b23f1b9fcc88e5d4b65a8637b17723bbda3cce8",
        "hashType": "type"
      }
    }
  ],
  "witnesses": [
    "0x55000000100000005500000055000000410000004817a4bb9df58fc6714934b47fb0bfe88e48dcd8b180e9cbcb61b23cd6c58c072cdc883a3bb15607f6c280a647ac152ae67a8b25e997771d6df116a89e5a6c2f00",
    "0x5500000010000000550000005500000041000000678f7c8e5a4f40cf9016e631e1e3d5dda7b0badae00078c3900b4a76cbb21a7c27efffc1820caaf7adb92b55877bf88f203c2d2d02a27949abe4b91cd77b6acb01"
  ],
  "outputsData": [
    "0x"
  ]
}

There are two items I would like to discuss in this example:

Multiple inputs with different locks by an HD wallet

In Neuron, the wallets are HD based, which means it will generate new addresses if the previous set of addresses are all used for previous transactions. A wallet in Neuron has multiple receiving addresses by default. The senders can transfer CKB to any one of them. Each address is generated based on a unique derivation path. From the user perspective, they don't need to manually manage balances across the different addresses when making a transfer. In other words, take the transfer use case for example, the users only need to know the amount of the total balance under an HD wallet and the recipient address.

Therefore, under an HD wallet, it is very likely to require a transaction to collect multiple inputs from summing up enough CKB capacity for one output. These inputs could be owned by different derivation paths.

Take the sample transaction above for example, the input #1 is owned by the private key represented by the path "m/44'/309'/0'/0/1", while the input #2 is owned by another private key represented by the path "m/44'/309'/0'/1/1".

The input witnesses will need to be signed with the private keys corresponding to the paths. In detail, it has to sign with both the precomputed transaction hash and the serialized witnesses. Then the signatures are pushed into the witnesses array in the transaction object before being sent to the transaction pool.

The sample code seems only to allow specify one path to sign a transaction. I am not sure if this means that it doesn't support signing a transaction with multiple inputs owned by different derivation paths, and whether the signature does include both the information transaction hash and serialized witnesses.

send_transaction RPC

I noticed the sample function signTransaction returns a hex string instead of a transaction JSON object with the newly signed witnesses array.

With signed transaction in the hex string, I am not sure how to put it into the CKB transaction pool via the send_transaction RPC.

There might be something I am missing in the current implementation of ledgerjs, so please feel free to comment.

mikereinhart commented 4 years ago

Hi Kata - in generally it seems LedgerJS is a lower-level tool than expected.

Regarding Multiple inputs with different locks by an HD wallet:

The use of HD wallets, address derivation, and transactions that take inputs from multiple addresses (or derivation paths) all associated with the same account makes complete sense.

However, unlike software keys, the Ledger device can only sign for one of these derivation paths at a time. Each signature requires a unique prompt on the Ledger device to inform the user what that address is signing.

The Ledger device can sign a single input of a multi-input transaction (in a single prompt), and I recommend calling SignTransaction for each address individually to accomplish your goals. Note the user will be prompted for each input.

Regarding send_transaction RPC:

The functionality we have in place is similar to how signing happens with LedgerJS for all chains, so from that perspective it seems the hex string should be integrated into the transaction JSON in the right place by the wallet.

katat commented 4 years ago

I recommend calling SignTransaction for each address individually to accomplish your goals. Note the user will be prompted for each input.

I think this is the way to go with hardware wallets.

I am trying to figure out how to individually sign a single input using the ledgerjs, particularly how to get the ledgerjs to sign the transaction hash and witnesses data following the algorithm in this sign function.

Since the signTransaction example you provided is to sign a whole transaction, would you please provide an example of how to sign the witness following the algorithm above?

The functionality we have in place is similar to how signing happens with LedgerJS for all chains, so from that perspective it seems the hex string should be integrated into the transaction JSON in the right place by the wallet.

I think it makes sense. Just for my knowledge, does this mean it requires updates to the RPC to accept the hex string in certain ways?

jhartzell42 commented 4 years ago

The Ledger does in fact implement that algorithm, so the existing example should work as a model. When we say "sign a transaction," we mean signing a transaction so that the signature would be usable on-chain, including hashing the transaction and relevant witnesses as part of that. The Ledger hashes the data that it is sent appropriately to accomplish this.

katat commented 4 years ago

Take the transaction example I outlined above, which has two inputs needed to be signed by two paths under an HD wallet, could you please explain how to use the current ledgerjs API signTransaction to sign that example transaction?

Also, how should I place, the signature returned by the signTransaction API, inside a transaction JSON object that the CKB RPC send_transaction accepts?

jhartzell42 commented 4 years ago

You have to call the signTransaction multiple times, one per each input path. Then, you have to do the logic yourself to add it appropriately to the signature array.

This would be analogous to the following code in CKB-CLI: https://github.com/nervosnetwork/ckb-cli/blob/develop/src/subcommands/wallet/mod.rs#L438-L442 https://github.com/nervosnetwork/ckb-cli/blob/develop/ckb-sdk/src/tx_helper.rs#L159-L163 https://github.com/nervosnetwork/ckb-cli/blob/develop/ckb-sdk/src/tx_helper.rs#L208-L258 LedgerJS just involves sending the Ledger what it needs to produce an appropriate signature and verify to the user what the signature is. The actual application of the signature is outside the scope, but would be the same as how it would be handled in a software wallet situation.

jonored commented 4 years ago

With the raw part of that transaction in txn and witnesses (including space for a signature, but not the signature itself) in wit, you could do:

sig1 = await signTransaction(path1, txn, [wit[0]], contexts, change_path);
sig2 = await signTransaction(path2, txn, [wit[1]], contexts, change_path);
signedTxn = txn;
witPrefix="0x5500000010000000550000005500000041000000";
signedTxn.witnesses = [ witPrefix + sig1, witPrefix + sig2 ]

An important point to note here is that we only send the input group's witnesses (and any extra witnesses not associated with an input) to signTransaction; the ledger zeroes the section of the first one where it's witness should go and otherwise just accepts the witnesses. We do need to send the full list of contexts. Also of note is that this example is taking a shortcut around properly handling the WitnessArgs data structure by prepending witPrefix; I'd assume that there's some handling for WitnessArgs already in the Neuron codebase already and that should be used here. Ultimately, the hex string returned by the ledger needs to get put into a Bytes and put into the "lock" field of WitnessArgs, serialized, and that result needs to get put into an array in the "witnesses" field of the transaction. For this particular case, and anywhere that particular shortcut around handling WitnessArgs works, wit[0] and wit[1] are both ckb.defaultSighashWitness.

katat commented 4 years ago

Ultimately, the hex string returned by the ledger needs to get put into a Bytes and put into the "lock" field of WitnessArgs, serialized, and that result needs to get put into an array in the "witnesses" field of the transaction.

I am not quite sure what this would imply the actual implementation for signing. Maybe it would be helpful to demonstrate via some sample code?

Also, could you please explain where the witPrefix is coming from?

jonored commented 4 years ago

witPrefix is the header of a WitnessArgs, as defined here: https://github.com/nervosnetwork/ckb/blob/develop/util/types/schemas/blockchain.mol#L106 that contains just a 65-byte signature and no input_type or output_type, as expected by https://github.com/nervosnetwork/ckb-system-scripts/blob/master/c/secp256k1_blake160_sighash_all.c - it is of course different for the witness for a multisig cell, and preferably you use a binding generated from the molecule file to fill out your WitnessArgs structure, you should handle it correctly (replacing the lock field with the signature and leaving anything in the type parts alone). There is a version of that binding in hw-app-ckb alongside the ledger-specific data structures, but I don't see how Neuron could operate without having them already and using two is probably not ideal.

None of what needs done after signTransaction is specific to a hardware wallet, so it's all code that should already exist in any app that isn't being written from scratch to only support a ledger; hw-app-ckb isn't currently scoped to be a general-purpose Nervos SDK.