Coder Social home page Coder Social logo

ledgerhq / app-bitcoin-new Goto Github PK

View Code? Open in Web Editor NEW
92.0 13.0 68.0 16.8 MB

Modern Bitcoin Application based on PSBT and Descriptors

License: Apache License 2.0

Makefile 0.30% Python 38.64% C 43.94% Shell 0.05% CMake 0.20% C++ 3.08% JavaScript 0.01% TypeScript 5.01% Jupyter Notebook 2.46% Rust 6.32%
bitcoin

app-bitcoin-new's Introduction

Ledger Bitcoin Application

Prerequisite

Be sure to have your environment correctly set up (see Getting Started) and ledgerblue installed.

If you want to benefit from vscode integration, it's recommended to move the toolchain in /opt and set BOLOS_ENV environment variable as follows

BOLOS_ENV=/opt/bolos-devenv

and do the same with BOLOS_SDK environment variable

BOLOS_SDK=/opt/nanos-secure-sdk

Compilation

make DEBUG=1  # compile optionally with PRINTF
make load     # load the app on the Nano using ledgerblue

Documentation

High level documentation on the architecture and interface of the app:

  • bitcoin.md: specifications of application commands.
  • wallet.md: supported wallet signing policies.
  • merkle.md: rationale and specifications for the usage of Merkle trees.

Additional documentation can be generated with doxygen

doxygen .doxygen/Doxyfile

the process outputs HTML and LaTeX documentations in doc/html and doc/latex folders.

Client libraries

A Python client library, a TypeScript client library and a Rust client library are available in this repository.

Tests & Continuous Integration

The flow processed in GitHub Actions is the following:

It outputs 4 artifacts:

  • bitcoin-app-debug within output files of the compilation process in debug mode
  • code-coverage within HTML details of code coverage
  • documentation within HTML auto-generated documentation

app-bitcoin-new's People

Contributors

bigspider avatar edouardparis avatar fametrano avatar fbeutin-ledger avatar fionn avatar jibeee avatar kewde avatar krosenbaum-ledger avatar landabaso avatar lpascal-ledger avatar mlafon-ledger avatar pgrange avatar sgliner-ledger avatar startup-dreamer avatar stratisiain avatar tamtamhero avatar tjoly-ledger avatar tjulien-ledger avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

app-bitcoin-new's Issues

Implement proofs of ownership

Proof of ownership can be used to attest the ownership of UTXOs. SLIP-19 defines a generic protocol for proofs of ownership.

The protocol also specifies proof of non-ownership, which can be used to attest that certain UTXOs belong to someone else.
Understanding and being able to generate and validate proofs of ownership would allow to securely sign transactions which contain external inputs (which currently can be signed, but a warning is given as there is a risk for the user). Such transactions have numerous use-cases (dual-funded lightning channels, various privacy solutions, and probably more to come in the future).

Add clear message signing

The app can already sign arbitrary (binary) messages (using the legacy Bitcoin Message signing scheme that prepends "\x18Bitcoin Signed Message:\n" to the message), but only a hash of the message is shown.
We could improve the UX (and potentially the practical security for certain protocols) by showing the actual message if it's not very long and displayable as text.

Ideally, the user should be able to scroll back and forth if the message is long (but that could be non-trivial if we can't hold the entire message in memory; perhaps we can have different thresholds on the length for Nano S versus other devices).

SIGHASH_DEFAULT is not recognized

If a PSBT has inputs with the sighash type field populated, taproot inputs that have SIGHASH_DEFAULT set are rejected with Only SIGHASH_ALL is currently supported. However SIGHASH_DEFAULT is the same as SIGHASH_ALL so this should not be rejected.

Add NanoX and NanoS+ tests to CI

Currently, only tests on NanoS are part of the continuous integration.
Test suites (at least the main ones) should be configured to run on all the devices instead.

Mark input as external PSBT Partial sign

I'm trying to combine inputs from 2 different ledger to create to output a single UTXO

Example:

Inputs
#0   |  0.1 BTC
#1   |  0.1 BTC

Outputs
#0   |  0.18 BTC

In bitcoinjs-lib, I'd do:
A. Sign a PSBT with input 0 and output 0 with SIGHASH_SINGLE | SIGHASH_ANYONECANPAY
B. Sign for input 1 with SIGHASH_ALL

However A is not possible with ledger it seems to do a check of outputs < inputs

So I'm guessing the way to do this would be to provide the full inputs and outputs, while marking input 1 initially as external.

Is this correct? If so, how do I mark input 1 as external?

So far I have for a single payment:

    const result = await ledgerBtc.createPaymentTransaction({
      inputs: [
        [splitTx, +index, undefined, undefined],
      ],
      associatedKeysets: [`86'/0'/0'/0/0`],
      outputScriptHex,
      additionals: ['bech32m'],
      // sigHashType: BitcoinLikeSigHashType.SIGHASH_SINGLE | BitcoinLikeSigHashType.SIGHASH_ANYONECANPAY,
    });

This already throws an error if I uncomment sigHashType

Optimize memory usage of UI flows

in display.c, instead of using a union large enough to contain all the strings of the biggest UX layout, we could use the approach used here with UX_STEP_NOCB_INIT to limit the global memory usage to largest strings used in a single UX step.
That should free up over 128 bytes of RAM.

(thanks @xchapron-ledger)

Allow signing and showing addresses without registering policies, or without policies at all

Currently, for any wallet policy which is not one of the standard wallet policies, it is necessary to register it first, and then use the registration hmac in calls to get_wallet_address or sign_psbt.

It might be useful to allow unregistered (and unnamed) wallet policies, where the policy verification (similarly to the registration flow) is executed right before showing the address or signing a transaction, and no hmac is returned.
This is particularly useful since policies are not revocable, forcing registration when a one-time usage is desired could by itself be a security risk.

Miniscript support

WIP on the miniscript branch, limited to script on segwit.

Relevant info:

=== TODO LIST ===

General changes:

  • Extend descriptors with miniscript operators
  • Support MAX_PUBKEYS_PER_MULTISIG=16 keys per multisig by improving the sorting technique for sortedmulti. (This could also save some memory by avoiding to keep many keys in memory.)
  • Optimize memory usage for Nano S

Changes to APIv1:

  • Allow longer wallet name, 16 bytes is too little (adapt UI accordingly)
  • Allow key expressions without origin information (assumed external)
  • Allow policies with more than one internal key. (TODO: is it useful to allow 0 internal keys?)
  • Specify the index of the signing key when signing from a wallet; special value for "all" can be added in a future version, adding custom fields to the PSBT
  • Modify the wallet description API to allow very long policies (necessary for future complex tapscripts).
  • Add /<m;n> syntax to key derivations

Miniscript safety checks:

  • Ensure only B-type expressions are valid in the top level
  • Ensure script size is at most MAX_STANDARD_P2WSH_SCRIPT_SIZE = 3600
  • Ensure the script can always be satisfied in non-malleable way (it has m type)
  • Ensure the script always needs a signature (it has s type)
  • Ensure the script always has no timelocks mixing (it has k type)
  • Ensure the same placeholder never has the appears for the same xpub and the same derivation (to ensure no duplicate key)
  • Ensure it respects the ops and stack size limits
  • Do we need to validate the nSequence/nLocktime for inputs that contain older/after (that is, OP_CSV/OP_CLTV)? naah, we good

UX improvements:

  • Show first receive address during policy registration?

Other nice-to-have:

  • allow raw pubkeys will do in a future version

Revisit wallet policy memory limits (esp. on Nano S)

The following policy fails to register on Nano S.

@has_automation("automations/register_wallet_accept.json")
def test_register_miniscript_rob(client: Client, speculos_globals, model):
    wallet = WalletPolicy(
        name="Rob's policy",
        descriptor_template="wsh(or_i(or_d(multi(2,@0/**,@1/**),and_v(v:pkh(@2/**),older(30))),or_i(and_v(v:pkh(@3/**),older(90)),or_i(and_v(v:pkh(@4/**),older(180)),and_v(v:pkh(@5/**),older(365))))))",
        keys_info=[
            "[f5acc2fd/48'/1'/0'/2']tpubDFAqEGNyad35aBCKUAXbQGDjdVhNueno5ZZVEn3sQbW5ci457gLR7HyTmHBg93oourBssgUxuWz1jX5uhc1qaqFo9VsybY1J5FuedLfm4dK",
            "tpubDE7NQymr4AFtewpAsWtnreyq9ghkzQBXpCZjWLFVRAvnbf7vya2eMTvT2fPapNqL8SuVvLQdbUbMfWLVDCZKnsEBqp6UK93QEzL8Ck23AwF",
            "tpubDDV6FDLcCieWUeN7R3vZK2Qs3KuQed3ScTY9EiwMXvyCkLjDbCb8RXaAgWDbkG4tW1BMKVF1zERHnyt78QKd4ZaAYGMJMpvHPwgSSU1AxZ3",
            "tpubDDmHUWoxdgxTWtk3mDaounzgji6Qfgm7xZL9WvhkXkqyyMcmsGgyepDDFJDu31anRfdTPvLCktR5wiJwzCmkUt2QtAKQXXozWaZU13vs63Q",
            "tpubDCNXTBKeWKnWn18FJMxf9Hb89Pj5PbHxDPjJfEWGqiGKVFgLgSRuwFSdj1SnV3wGAoGQ55KGRiFqmXyDLmwJxKKYFPWwmoSfHk4gtYfKU76",
            "tpubDC6RYke2oWqmkt7UZrQiADdkT66fyJFRZMUWoHcD2W92BK6y7ZBS8oLRw6W66epPqPVisVFBnuCX214yieV2cq9jxdEYe1QJxxNoYZEi6Fb",
        ])

    wallet_id, wallet_hmac = client.register_wallet(wallet)

    assert wallet_id == wallet.id

    assert hmac.compare_digest(
        hmac.new(speculos_globals.wallet_registration_key, wallet_id, sha256).digest(),
        wallet_hmac,
    )

Despite fitting in the 192-bytes limit for the descriptor template, it turns out that parsing it requires 340 bytes, which is higher than the current limit of 264 bytes (on Nano S).

It might be worth increasing the limits on other devices as well.

Improve `check_merkle_tree_sorted` to work with preimages of arbitrary length

Currently, the call_check_merkle_tree_sorted function puts a limit on the length of the preimages in the Merkle tree that is checking; #90 increased it to 162, but Merkle trees used in PSBTs can have much longer elements (over 4kb for taproot control blocks defined in BIP-341, but potentially unlimited for future extensions or PSBT vendor-specific fields).

call_check_merkle_tree_sorted is currently keeping two consecutive elements at the time (in order to check the correct ordering), but memory limitations prevent from using this approach for larger preimages.

Ideally, we would like to stream each byte sequence only once, and still be able to detect which one is larger.
That's impossible in theory, but it's likely possible in most cases, and that might be good enough in practice.

Revisit signing flow and UX in the presence of sighash flags

Currently, if any input is not signed with SIGHASH_ALL/SIGHASH_DEFAULT, we show a generic warning.
There are potentially several aspects that might be changed:

  • we currently check that SUM(output_amounts) ≤ SUM(input_amounts); that check is too restrictive if inputs don't commit to the entire transaction (e.g.: SIGHASH_SINGLE; related issue #123)
  • How should the UX reflect and handle those advanced use cases?

One way could be to have an "advanced" transaction validation mode, where all inputs (and their SIGHASH_FLAG if appropriate) and all outputs are shown on-screen.

Improve UX for self-transfers

Currently, when sending coins to an address controlled by the wallet policy, the UX only considers an output a change address if the the BIP-32 ends with /1/* (or more generally /N/*, for wallet policies containing <M;N>/*).

This is consistent with the UX the previous app had (and probably most hardware signers); however, it might be confusing for users, as it might not match their expectations. Moreover, for security, the (internal) output address would have to be first validated on the secure screen, and then one would have double-check that the output is sent to the same address. Users are likely to skip some steps, or performing them insecurely (for example, copying the address on an editor on the same machine they are using to send the transaction).

Possible solutions:

  • Consider any internal outputs change; for a transaction where all the inputs/outputs are internal, one could have an explicit UX showing it as "self-transfer", as the sent amount is 0 (apart from the fees). Some care needs to be taken to check to handle the case where there are also external inputs.
  • Keep the same UX flow, but explicitly label outputs with /0/* or /M/* as "your own" in the UX.

Refactor e2e tests

End-to-end tests using bitcoin-core currently have a lot of repeated code, which could be generalized.
Moreover, the boilerplate of the tests (generating core wallets, getting xpubs, etc. could be automated as well.

Improve the interface of the ledger-bitcoin python library

Listing the issues found while working on the Electrum integration:

  • The Transport interface doesn't allow to specify the path when using hid mode.
  • The sign_psbt should allow to pass a str or bytes as a psbt, and return the same type; we don't want to bound callers to use the custom PSBT class.
  • Add __version__ to the module.

Can't sign a P2TR Output from a Segwit P2SH UTXO on Electrum

I just tried to send to a P2TR Taproot address (bc1p...) using an old Segwit P2SH UTXO (starting with a 3). My Ledger Nano S (Firmware 2.1.0, Bitcoin Version 2.0.6) wasn't able to sign that TX using Electrum. The confirmation request didn't appear on the Ledger. I had to use a P2WPKH address instead. With this address type, the signing process worked.

Could you please look into this and apply a fix?

Optimize performance of internal/external input verification

The new app is very paranoid in the process it follows to identify internal inputs/outputs: for each input that has keys that match the fingerprint, it rederives them and compiles the descriptor to verify if the scriptPubKey of the input does indeed match the result.

This allows us to have a very strong security model, where you know not only that you don't sign transactions with external inputs unknowingly, but you also know exactly what wallet policy you are using to sign − so the user can't be tricked into spending from a different account they control.

Rather than trying to gain on performance by loosening the security model, we definitely have some room for optimizing the current logic.

The most obvious is: deriving from internal keys is currently delegated to Bolos, therefore it always starts from the root (master key).
Most likely, it is going to be a lot faster to derive the master xpub of the current policy/key_placeholder, and then do the final two steps of derivation within the app's code. This is going to be a lot easier thanks to the latest refactoring :) (which also saved some memory).
This should save some time even in simple transactions with 1 input and 1 ouput + 1 change address.

The other parts of the input script matching (compilation of the policy, etc.) could be profiled as well, but imho they are less likely to have much room for improvement.

Unable to sign PSBT with both segwit and non-segwit inputs

I am unable to sign a PSBT with a segwit wallet if it has both segwit and non-segwit inputs, and the non-segwit inputs do not include a witness_utxo. It appears that the function sign_segwit expects all inputs to have witness_utxos, but that is not the case if at least one of the inputs is non-segwit.

A solution to this issue is to include the witness_utxo for non-segwit inputs, but this goes against the PSBT spec. Furthermore, doing that means that it is not possible sign the PSBT with a legacy wallet as the signing function to use is determined by the presence of witness_utxo.

Refactor and speed up e2e tests

test_e2e_multisig.py, test_e2e_miniscript.py, and test_e2e_tapscripts.py Have a lot of shared code that could be abstracted and put in common utility file.

Moreover, we can reduce a lot the running time of the tests by skipping the registration of the wallet policies (that is, computing the hmac locally instead of having that as part of the test); since we have plenty of speculos tests for that, this would reduce the running time of the suite without significantly reducing the test coverage.

Update Rust client library for tapscript support

When signing tapscript transactions, the SIGN_PSBT command returns pubkey||tapleaf_hash instead of just pubkey.
Cient library would be cleaner by replacing the older pubkey object with a structured object that always contains a pubkey (33-bytes if non-taproot, 32 byte if taproot) and optionally the tapleaf_hash (for taproot scripts only).

This is a breaking change for the client library, so it needs to be respected with the semver versioning.

Support for tapscripts (scripts within `tr` descriptor)

This tracks the work necessary to spend P2TR outputs containing scripts.

Since signatures for the key-path spend and for each tapleaf are distinct, the interface should take that into account; the default behaviour could be to sign for all the valid paths (key path spend + each script, repeating for each internal placeholder).

To be decided:

  • How does the client specify what spending paths should be used? Should all paths be signed as default behaviour? Should the client be able to specify which internal keys should sign (if more than one is present)?
  • Does the UX tell the user which spending path is being used? If so, how? Probably: show the descriptor for that tapleaf, but it might not be a great UX. Different applications might have different requirements, so we should aim for flexibility.

TODO:

  • Parse correctly tr(KP, TREE) policies, for the supported descriptors.
  • Figure out if and how to support tr(<unspendable>,TREE) not standardized yet; this can be added later by extending the language for KEY expressions.
  • Support descriptors within tr:
    • pk
    • multi_a, sortedmulti_a
  • Generalize the signing code with the Common Signature Message Extension per BIP-342
  • implement any necessary sanity checks for TREE

Consider adding a timelock policy

I've developed a vault-wallet (it will published/opensourced soon) that is able to spend locked utxos such as this one:

<KEY_1> OP_CHECKSIG OP_NOTIF
  <KEY_2> OP_CHECKSIG
OP_ELSE
  <LOCK_TIME> OP_CHECKSEQUENCEVERIFY
OP_ENDIF

I have successfully integrated the wallet with Ledger devices using with the old btc app using signP2SHTransaction.

However, it is currently not possible to use the new app with such script now.

I understand you'll allow all sort of policies in the future but it looks like they're currently restricted to more battle-tested: P2PKH, P2WPKH, P2SH-P2WPKH, TR, and multisig.

I would like to ask you to include this policy in the collection of initial supported policies so that I can start preparing the integration with the new application some time in advance:

wsh(or(and(pk(KEY_1),older(TIME)),pk(LOCK_TIME)))

Amount types mismatch in psbtv2.ts - PR proposal (breaking change)

In the current implementation, the amount parameter in the following declarations is sometimes defined as a Buffer and other times as a number:

setOutputAmount(outputIndex: number, amount: number);

setInputWitnessUtxo(
  inputIndex: number,
  amount: Buffer,
  scriptPubKey: Buffer
);

(also getInputWitnessUtxo returns a Buffer for the amount)

This can cause confusion and make it easier to make errors, as the user must remember to use uint64LE(amount) when using setInputWitnessUtxo, but not when using setOutputAmount. Additionally, uint64LE is not exposed in the API.

I propose a change to consistently use number for the amount parameter in both declarations, as follows:

setOutputAmount(outputIndex: number, amount: number);

setInputWitnessUtxo(
  inputIndex: number,
  amount: number,
  scriptPubKey: Buffer
);

(also change getInputWitnessUtxo)

This would simplify the API and reduce the risk of errors. However, this would be a breaking change and would require careful consideration of its impact on existing users.

Please let me know your thoughts on this proposal. Thank you for the great work on this library, I've been able to use it to spend wsh miniscripts with the Ledger with no issues at all.

Sign segwit transactions when the non-witness-utxo is missing

The legacy application would sign segwit transactions where the non-witness-utxo is not provided, although it showed a warning.
That was to solve a known bug of segwit (see for here for details), known and fixed in several hardware wallets since 2020 (although the weakness in BIP-143 was already discussed in April 2017.

The new app treats segwit inputs the same way as legacy inputs to avoid the security risks. This has been reported as a problem for some external integrations (notably, adding the non-witness-utxo is complicated for a pruned node): see btcpayserver/btcpayserver#1631.

Adding a warning is a good enough mitigation and avoids users getting locked out of their funds.

Taproot transactions are not affected by this vulnerability.

Add a progress bar to the app's UI during signing (and other long operations)

The device shows a "processing" screen when busy during computation/communication, but no indicator of progress is shown.
The "Processing" screen only appears if the device is busy for too long (more than about 1 second), so it doesn't appear for small transactions.

When signing transactions with many inputs, it would greatly improve the UX if some progress indicator was shown. For example

Signing...
(11/25)

with some appropriate icon (or a spinner, if we can afford it).

Note that there are multiple stages of signing (first all inputs are processed, then all outputs −with user confirmation, then the actual signing happens. We might want to have different progress bars for each phase, and keep the behavior of only showing it if it takes longer than 1 second).

When signing policy wallets, the number of signatures produced is the number of internal placeholders (typically, 1), multiplied by the number of internal inputs (typically, all of them). Care needs to be taken to handle edge cases correctly.

It might be worth adding it to other commands (e.g. showing addresses, registering complex policies), although it's less critical.

Use 64-byte Schnorr signatures by default

When the hashtype is the default one (0), it would be more appropriate to return 64-byte signatures that omit the hashtype.
Currently, we are instead using 0x01 for the hashtype, that is appended to the signature (resulting in 65-byte signatures).

Details in BIP-0341 and BIP-0342.

Once this is fixed, it might need to slightly adjust the fee-estimation logic on Ledger-Live side as well.

Failure to sign with HWI in `experimental` branch

Reported on Discord:

➜ hwi --device-type ledger --chain regtest  signtx cHNidP8BAFICAAAAAcY0GO3kw3WR4DRpjyevAb7hdujw7VJu/c+ri1vFhslFAAAAAAD/////ARD2mQAAAAAAFgAUSBBf5b19pG3O9/HZgTREeBJEmLcAAAAAAAEBH8DYpwAAAAAAFgAUfKscAi6hPq9770PNnBVTCsZ6bM0iBgMOLYv0eQNelPOdulhEVYU53V9ojg5EYm18HTx9uzRcDRipfuaZVAAAgAEAAIAAAACAAAAAAAAAAAAAAA==
{"psbt": "cHNidP8BAFICAAAAAcY0GO3kw3WR4DRpjyevAb7hdujw7VJu/c+ri1vFhslFAAAAAAD/////ARD2mQAAAAAAFgAUSBBf5b19pG3O9/HZgTREeBJEmLcAAAAAAAEBH8DYpwAAAAAAFgAUfKscAi6hPq9770PNnBVTCsZ6bM0iAgMOLYv0eQNelPOdulhEVYU53V9ojg5EYm18HTx9uzRcDUgwRQIhAOCbPasdbkQGPiF5Qo5cJUQ/V/0z1cbE427U4eH6rFFvAiBf7Rnzr9HeiCFhAQQLQT5RIWHvPytxPjYPDdGETf1FNAEiBgMOLYv0eQNelPOdulhEVYU53V9ojg5EYm18HTx9uzRcDRipfuaZVAAAgAEAAIAAAACAAAAAAAAAAAAAAA==", "signed": true}

➜ hwi --device-type ledger --chain regtest  signtx cHNidP8BAFICAAAAAcY0GO3kw3WR4DRpjyevAb7hdujw7VJu/c+ri1vFhslFAAAAAAD/////ARD2mQAAAAAAFgAUSBBf5b19pG3O9/HZgTREeBJEmLcAAAAAAAEBH8DYpwAAAAAAFgAUfKscAi6hPq9770PNnBVTCsZ6bM0iBgMOLYv0eQNelPOdulhEVYU53V9ojg5EYm18HTx9uzRcDRipfuaZVAAAgAEAAIAAAACAAAAAAAAAAAAAAA==
{"error": "('0x6985', 'Error in <BitcoinInsType.SIGN_PSBT: 4> command', '')", "code": -13}
the first run is with older firmware, the second one with the latest experimental
the seed is: neither exist capital focus stumble coil coffee almost then tape husband eager advice win modify unfold rapid magic globe height tone area cactus victory
I'm using the nano S plus

Increase `MAX_BIP32_PATH_STEPS` to 10

In order to better support use cases with longer derivation paths, it would be better to support longer BIP-32 paths for derivations and pubkey export.

To avoid memory problems on Nano S, we have to ascertain that increasing the constant does not lead to an increase in memory usage in critical paths, and in the global variables for UI (affected by MAX_SERIALIZED_BIP32_PATH_LENGTH).
Moreover, double check that the various UX flows are still reasonable when the paths are longer.

UX improvements for Stax

List of possible improvements for Stax UX.

  • Display xpub:
    • It would be nice to have a smaller font for an xpub, and make sure it fits on a single screen.
  • Register policy:
    • Remove the "Cosigner registered", registration only happens at the end; ideally, we could try to make it go to the next/previous cosigner just by going right/left (on all devices, not just for Stax), without having to confirm anything explicitly until the end.
    • Smaller font size (or adaptive based on the size of content) would be great.
    • Word wrapping for long policy names with spaces? Currently it splits at the end of the line, regardless if it makes sense to split there or not. (Lower priority, as this might be tricky)
  • Sign_psbt:
    • Can fit everything on a single screen for simple transactions with just one output?
    • Going to the last screen shows a <-- button, but you can only go back to the previous step (showing the fees), which is awkward. Similarly to cosigners, we might want to allow going back and forth without confirming anything explicitly.

Failed to register simple pkh(@0/**) policy

From wizardsardine/async-hwi#29

This policy does not require registration, but still the registration failed.

Using speculos:

SPECULOS_APPNAME="Bitcoin Test:2.1.0" ./speculos.py apps/nanos-2.1-bitcoin-test-2.1.0-rc.elf --model nanos

and https://github.com/bigspider/HWI/

bigspider-hwi on  wallet-policies [?] is 📦 v2.1.1 via 🐍 v3.9.2 (venv) 
✦ ❯ python3 -m hwi --chain test --device-type ledger getxpub "m/44'/1'/0'"
{"xpub": "tpubDCwYjpDhUdPGP5rS3wgNg13mTrrjBuG8V9VpWbyptX6TRPbNoZVXsoVUSkCjmQ8jJycjuDKBb9eataSymXakTTaGifxR6kmVsfFehH1ZgJT"}

bigspider-hwi on  wallet-policies [?] is 📦 v2.1.1 via 🐍 v3.9.2 (venv) 
✦ ❯ python3 -m hwi --chain test --device-type ledger displayaddress\
       --policy "pkh(@0/**)"\
       --name "Ledger"\
       --keys "[\"[f5acc2fd/44'/1'/0']tpubDCwYjpDhUdPGP5rS3wgNg13mTrrjBuG8V9VpWbyptX6TRPbNoZVXsoVUSkCjmQ8jJycjuDKBb9eataSymXakTTaGifxR6kmVsfFehH1ZgJT\"]"
{"address": "mz5vLWdM1wHVGSmXUkhKVvZbJ2g4epMXSm"}

bigspider-hwi on  wallet-policies [?] is 📦 v2.1.1 via 🐍 v3.9.2 (venv) 
✦ ❯ python3 -m hwi --chain test --device-type ledger registerpolicy\      
       --policy "pkh(@0/**)"\
       --name "Ledger"\
       --keys "[\"[f5acc2fd/44'/1'/0']tpubDCwYjpDhUdPGP5rS3wgNg13mTrrjBuG8V9VpWbyptX6TRPbNoZVXsoVUSkCjmQ8jJycjuDKBb9eataSymXakTTaGifxR6kmVsfFehH1ZgJT\"]" 
{"error": "('0x6a82', 'Error in <BitcoinInsType.REGISTER_WALLET: 2> command', '')", "code": -13}

Fix VSCode integration

The .vscode folder was taken from the boilerplate at the beginning and never updated − and it's broken.

Fixing it (and updating README.md) might help new contributors.

Isn't `ledger-bitcoin` published on npmjs.com?

Hi all, thanks for the wonderful work on bitcoin on Ledger here! 😍

Due to increasing CLA_NOT_SUPPORTED error reports at pocketbitcoin.com I started upgrading our Ledger dependencies. However, even with the latest @ledgerhq/hw-app-btc I still seem to be getting these errors with my Ledger Bitcoin app at 2.1.1. So I found out about this separate repository here.

I noticed there's a completely new client library ledger-bitcoin in here. When it was introduced in #22 it was implied it would be published on npmjs.com. However, this doesn't seem to be the case yet?

Is this new library ready to use yet and available somewhere?

Make a PR to improve HWI's support of the 2.* app

This issue tracks a number of improvements to make to the HWI repository.
ideally, we should do this before the actual app's release, in order to minimize disruption.

Development of the PR in https://github.com/bigspider/HWI/tree/improvements.

The main issue is to make sure that HWI doesn't break with the new app, and never tries the legacy protocol if the app version is >= 2.1.0.

Most urgent:

  • Should add "Bitcoin Legacy" and "Bitcoin Testnet Legacy" as accepted appnames; for them, the legacy protocol should be used regardless of the version.

Other improvements:

  • Port upstream changes from our python library. The 2.1.0 has a new version of the protocol (read here), we should generally switch to that when possible as it simplifies some things. Perhaps we could just use the legacy protocol for any version <2.1.0, and just use the new protocol with version P1=1 for any 2.1.0. This shouldn't break too many things and will allow to simplify some code.
  • These limitations are fixed in more recent versions of the app. There shouldn't be changes to do in the code, just update the docs. (double check)
  • This limitation is removed in the 2.1.0 version of the app
  • sign_message: in HWI it is currently using the legacy protocol. We could switch to the native new protocol of version 2.1.0 (and stick to the current code if version is < 2.1.0)
  • display_multisig_address: not implemented; we could do it, but we would have to register the policy first (like is done when signing). Not great, but better than nothing.
  • Figure out how to let the HWI repo fetch a fresh binary for the bitcoin app they already did

Refactor crypto.c/crypto.h

Currently, crypto.{c,h} is a mix of several functions that use cryptography in some way, but not really cohesive enough.

It would be better to refactor it in several cohesive modules, and add missing unit tests.

Invalid links in README.md

The "Getting Started" link for the toolchain installation is invalid.
The link for the "High level documentation" in invalid.

Ignore SLIP-132

I've noticed by testnet PSBTs are rejected if the extended pubkeys in the global map are not prefixed with tpub.
Is there a reason why the prefix is being used for validation and not just the pubkey / path?

Create a nightly test suite

The running time of the test suite is getting a bit long; one way we could reduce it is to move many of the tests to a nightly test suite.

Other than soving time in PRs, this would have other advantages:

  • some disabled tests that are very slow could be instead executed in the nightly
  • the nightly might catch if some update to the SDK caused problems while there was no opern PR.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.