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

Delegate Bitcoin Core's private key management to Eclair #2613

Merged
merged 8 commits into from
Sep 21, 2023

Commits on Sep 18, 2023

  1. Make Eclair manage bitcoin core's wallet private keys

    We create an empty watch-only wallet and import public descriptors generated by Eclair.
    Bitcoin Core can fund transaction and manage utxos, but can no longer sign transactions.
    
    * Check that spent amounts and utxos are consistent before we sign a PSBT
    
    PSBT utxo fields include the amount that are being spent by the PSBT inputs, but there is a "fee attack"
    where using amounts that are lower than what is actually spent may make us sign a tx that spends much more
    in fees than we think.
    
    * Check that non-segwit uxto has been provided and inputs are signed with SIGHASH_ALL
    
    * Verify that Bitcoin Core's fee match what we specified
    
    When we call Bitcoin Core's `fundrawtransaction` RPC method, we check that the fee that we pay match the fee rate that we requested.
    The fee is computed using the utxo information that Bitcoin Core adds to our PSBT before we sign it.
    
    We can safely used this information because if Bitcoin Core lies about the value of the inputs that we're spending then the signature we
    produce will also not be valid (it commits to the value being spent).
    
    When we're adding wallet inputs to "bump" the fees of a parent transaction we need to take the whole package into account when we verify the
    actual fee rate, which is why some internal methods were modified to return the package weight that was used as reference when `fundrawtransaction`
    was called.
    
    * Check that fundrawtransaction does not add more than 1 change output
    
    * Validate addresses and keys generated by bitcoin core
    
    When eclair manages private keys, make sure that we can re-compute addresses and keys
    generated by bitcoin core.
    
    * Add a separate configuration file for Eclair's onchain signer
    
    Eclair's onchain signer now has its own `eclair-signer.conf` configuration file in HOCON format.
    It includes BIP39 mnemonic codes and passphrase, a wallet name and a timestamp.
    
    When an `eclair-signer.conf` file is found, Eclair's API will return descriptors that can be imported into an
    empty watch-only Bitcoin Wallet.
    When wallet name in `eclair-signer.conf` matches the name of the Bitcoin Core wallet defined in `eclair.conf`
    (`eclair.bitcoind.wallet`), Eclair will bypass Bitcoin Core and sign onchain transactions directly.
    sstone committed Sep 18, 2023
    Configuration menu
    Copy the full SHA
    04c9306 View commit details
    Browse the repository at this point in the history
  2. Create eclair-backed wallet automatically on startup

    When eclair starts, if it is configured to manage bitcoin core's onchain key, and the configured wallet does not exist yet,
    and eclair descriptor's timestamps are less then 2 hours old, eclair will automatically create the configured wallet with the
    appropriate options and import its descriptors.
    sstone committed Sep 18, 2023
    Configuration menu
    Copy the full SHA
    9f64641 View commit details
    Browse the repository at this point in the history
  3. Address review comments

    * Fix typos, use more explicit method names, ...
    
    * Use hardcoded ports in tests instead of "first available port"
    
    * User-friendly error message when eclair-baked wallet creation fails at startup
    
    * Simplify ReplaceableTxFunder
    
    * Replace EitherKt with Scala's Either type
    
    * Update signPsbt() to return Try
    
    * Skip validation of local non-change outputs:
    
    Local non-change outputs send to an external address, for splicing out funds for example.
    We assume that these inputs are trusted i.e have been created by a trusted API call and our local
    onchain key manager will not validate them during the signing process.
    
    * Do not track our wallet inputs/outputs
    
    It is currently easy to identify the wallet inputs/outputs that we added to fund/bump transactions, we don't need to explicitly track them.
    
    * Document why we use a separate, specific file for the onchain key manager
    
    Using a new signer section is eclair.conf would be simpler but "leaks" because it becomes available everywhere
    in the code through the actor system's settings instead of being contained to where it is actually needed
    and could potentially be exposed through a bug that "exports" the configuration (through logs, ....)
    though this is highly unlikely.
    sstone committed Sep 18, 2023
    Configuration menu
    Copy the full SHA
    e28c3f3 View commit details
    Browse the repository at this point in the history
  4. Additional changes to delegate bitcoin core keys to eclair (#2726)

    Refactor the `BitcoinCoreClient` and `LocalOnChainKeyManager` to:
    
    - rely less on exceptions
    - use more idiomatic scala (reduce dependency on kotlin types)
    - provide more detailed logs
    
    We also simplify the `useEclairSigner` field in `BitcoinCoreClient`.
    The complexity of handling the case where there was an on-chain key
    manager but for a different wallet than the one configured isn't
    something that should be used, so it wasn't worth supporting.
    
    Some checks were inconsistent and are now unified:
    
    - checking the exact `scriptPubKey` in our outputs in TODO and TODO
    - we allow using `fundTransaction` with a tx that already includes a
      change output (which may happen when RBF-ing a transaction)
    - `getP2wpkhPubkeyHashForChange` didn't verify the returned key
    
    We completely separate the two cases in `signPsbt`, because otherwise
    in the non eclair-backed case, we were calling bitcoind's `processpsbt`
    twice for no good reason, which is bad for performance.
    
    We also decouple the `OnChainKeyManager` from the `BitcoinCoreClient`.
    This lets users keep running their eclair node with a bitcoin client that
    owns the private key while configuring the on-chain key manager for a
    future bitcoin client that will leverage this on-chain key manager.
    
    Users can use the eclair APIs to get the master xpub and descriptors to
    properly configure their next bitcoin core node, and switch to it once it
    has synchronized the descriptors.
    t-bast authored and sstone committed Sep 18, 2023
    Configuration menu
    Copy the full SHA
    eff698a View commit details
    Browse the repository at this point in the history

Commits on Sep 20, 2023

  1. Address review comments

    Rename `wallet` field to `walletName`
    Improve comments related to signature workflow and verification of wallet inputs/outputs
    Remove `descriptorChecksum` method: it's provided by bitcoin-kmp and does not need to be exposed
    sstone committed Sep 20, 2023
    Configuration menu
    Copy the full SHA
    0f47b52 View commit details
    Browse the repository at this point in the history
  2. Simplify replaceable tx funding (#2745)

    * Refactor `dummySignedCommitTx`
    
    We only need the weight of the signed commit tx, it was error-prone to
    provide what looks like a signed commit tx.
    
    * Simplify replaceable tx funding
    
    We were previously signing twice (with makes a call to `bitcoind`),
    just to get the final weights and adjust the change outputs. This was
    unnecessary, as we can adjust the weights before adding inputs.
    
    We were also duplicating the checks where we verify that `bitcoind` is
    malicious. We only need to check that once, during the final signing step.
    t-bast authored Sep 20, 2023
    Configuration menu
    Copy the full SHA
    7cd8b14 View commit details
    Browse the repository at this point in the history

Commits on Sep 21, 2023

  1. Remove code to automatically create eclair-managed wallet (#2746)

    Remove code to automatically create eclair-managed wallet
    
    This code made the onchain key manager more complex, and for older wallets (when nodes are moved to a new machine for example) we need
    to provide a manual process for creating a new empty wallet and importing descriptors generated by Eclair.
    
    => It is simpler to always use this manual process.
    sstone authored Sep 21, 2023
    Configuration menu
    Copy the full SHA
    bf6f240 View commit details
    Browse the repository at this point in the history
  2. Document how we setup an optional onchain key manager

    Depending upon the presence of an `eclair-signer.conf` file in Eclair's data directory, and the names of the wallet set in `eclair-signer.conf` and `eclair.conf`, we can have:
    - no onchain key manager (which is the default)
    - an onchain key manager that is used to generate descriptors through our API but that is not active. This is how you create a new watch-only Bitcoin wallet to be used by Eclair
    - an onchain key manager that is used to sign bitcoin wallet transactions
    sstone committed Sep 21, 2023
    Configuration menu
    Copy the full SHA
    d3ac588 View commit details
    Browse the repository at this point in the history