Add Lightning Network support #18179

pull icota wants to merge 24 commits into bitcoin:master from icota:2020-01-core-lightning changing 1240 files +131840 −577
  1. icota commented at 3:51 pm on February 19, 2020: contributor

    Configure with --enable-rust. Here’s how to use:

     0$ ./bitcoin-cli help
     1...
     2 == Lightning ==
     3lnclosechannel "channel_id"
     4lnconnect "node"
     5lncreateinvoice "description" amount
     6lnfundchannel "node_id" amount
     7lngetchannels
     8lngetpeers
     9lnpayinvoice "invoice"
    10...
    

    Connect to ACINQ testnet node:

    0$ ./bitcoin-cli  lnconnect 03933884aaf1d6b108397e5efe5c86bcf2d8ca8d2f700eda99db9214fc2712b134@34.250.234.192:9735
    

    See that we are connected:

    0$ ./bitcoin-cli lngetpeers 
    1[ 
    2  { 
    3    "id": "03933884aaf1d6b108397e5efe5c86bcf2d8ca8d2f700eda99db9214fc2712b134"
    4  } 
    5]
    

    Check if we have enough in our wallet:

    0$ ./bitcoin-cli getbalances 
    1{ 
    2  "mine": { 
    3    "trusted": 0.01000000, 
    4    "untrusted_pending": 0.00000000, 
    5    "immature": 0.00000000 
    6  } 
    7}
    

    Fund a 0.001 tBTC channel with our peer:

     0$ ./bitcoin-cli lnfundchannel 03933884aaf1d6b108397e5efe5c86bcf2d8ca8d2f700eda99db9214fc2712b134 0.001
     1
     2$ ./bitcoin-cli lngetchannels 
     3[ 
     4  { 
     5    "id": "09e43999ea4fc33c0c8aa134727ea17ac1e2ac7e9356b1434fed9bfc8f091e1f", 
     6    "shortid": 0, 
     7    "capacity": 100000, 
     8    "status": "unconfirmed" 
     9  } 
    10]
    

    Wait a couple of blocks and try again:

     0$ ./bitcoin-cli lngetchannels 
     1[ 
     2  { 
     3    "id": "09e43999ea4fc33c0c8aa134727ea17ac1e2ac7e9356b1434fed9bfc8f091e1f", 
     4    "shortid": 1831367457947189249, 
     5    "capacity": 100000, 
     6    "status": "confirmed" 
     7  } 
     8]
     9
    10$ ./bitcoin-cli getbalances 
    11{ 
    12  "mine": { 
    13    "trusted": 0.00896940, 
    14    "untrusted_pending": 0.00000000, 
    15    "immature": 0.00000000 
    16  } 
    17}
    

    Our channel is operational! Immediately go and purchase some beers on yalls.org:

    0$ ./bitcoin-cli lnpayinvoice lntb1500n1p0yxfm6pp5098ss9punvma2xusvdez40dnrnrr7zlf2hpqm5kvnc4g9fjk7pjqdp6g9jxggrjv4skxarfdahzpuyl3kazqar0ypuk7atjyp3k7mrvv43hg6t0dccqzpgxqr23ssp5kjpejv09t6yweaf02x7ywzpdr3z8utpnyw67x9lcvfuf6f6kwdns9qy9qsqx323heyl6e54cmrxn9gnuqsef7juacsl7w7cngvrvg2vft6qtll5f7c07vx004n62uwklnhfwd62tnjzw30gt5ucsz32yz3j6uanvhqqqgpkru
    

    Close the channel:

    0$ ./bitcoin-cli lnclosechannel 09e43999ea4fc33c0c8aa134727ea17ac1e2ac7e9356b1434fed9bfc8f091e1f
    

    Hot on the heels of rust-lightning 0.0.10 release and the LDK announcement I present you CoreLightning.

    This PR is based on #17376 and it adds 4 more crates to the mix: rust-lightning, rust-lightning-invoice, rust-bech32-bitcoin and num-traits. Peers and channels data is in lightning. Code will be impossible to review on GitHub due to submodule mess but for now this is a one-commit PR anyway so take a look there.

    Even though this is nowhere near production ready I’m excited to see it work. Credit to @TheBlueMatt and the rust-bitcoin/lightning team. Truly awesome work.

    For the sake of argument let’s say everyone agrees that Lightning is a good thing and helpes Bitcoin scale. In that case I see built-in CoreLightning as useful in two ways:

    • Makes it easy for the Core install-base to adopt Lightning in one fell swoop. People will be more likely to use and build on top of Lightning if it’s one upgrade away. You can use your existing wallet funds. No separate daemons or needless transactions. Institutional users such as exchanges would have no excuse not leverage Lightning.
    • I feel that Core could be more of a universal client in the future. It’s somewhat industrial nowadays but I’d like 2021 to be the year of Core on the Phone :sunglasses:. I wrote about this a bit here.

    Code is not very robust, it needs a lot of love in regards to making it more “rusty” and figuring how to better manage the C++/Rust FFI boundary. Before I invest any more time in this I’d like to get some guidance and concept (N)ACKs.

  2. Adds a rust library to bitcoin to demonstrate linking and building rust code.
    The demonstration library compiles a rust hello world example and auto-generates
    a header which can be included in C++ code.
    
    Co-Authored-By: Jeremy Rubin <j@rubin.io>
    Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
    Various changes by: Matt Corallo <git@bluematt.me>
    46f4c3e5dd
  3. build: show rust enabled in configure output 8c2ee65588
  4. build: gitignore src/rusty_test 8fd3b4a8fb
  5. Add basic ffi bindings for Rust header download
    Also, break circular dependency with a new helper lib courtesy of
    Cory Fields <cory-nospam-@coryfields.com>.
    8e3f0238a2
  6. Add headers-over-DNS fetcher in Rust 61337eaa84
  7. Limit total memory allocated by all Rust code to 128MB b76d4f9d68
  8. Enable rustc building + testing on xenial Tsan Travis job 4b6e0b800d
  9. Move CNode's FindNextBlocksToDownload state into its own strut
    This starts the process of moving the last bits of
    validation-critical logic out of net_processing - the fallback
    logic of which blocks to download is incredibly critical to
    validation and would likely also be used for any parallel block
    sync systems. Further, assumeutxo will require additional ability
    to select blocks for download, which is a validation-specific
    concept and shouldn't really live in net_processing.
    
    This moves a few responsibilities to the FindNextBlocksToDownload
    callsite instead of passing in a CNodeId, but otherwise has no
    functional changes.
    57ca92ac42
  10. Make FindNextBlocksToDownload a member func on BlockProviderState cd59d277bd
  11. Make FindNextBlocksToDownload not use mapBlockIndex for in-progress
    This makes it more general than just net_processing
    05519da346
  12. Move BlockProvider to validation.{h,cpp} from net_processing 290f8f47fd
  13. Add ffis to access BlockProviderState and hand blocks to C++ e105ea86ec
  14. Add a rust-based backup over-REST block downloader 4e691f9602
  15. Walk pindexBestHeader back to ChainActive().Tip() if it is invalid
    Instead of keeping pindexBestHeader set to the best header we've
    ever seen, reset it back to our validated tip if we find an ancestor
    of it turns out to be invalid. While the name is now a bit confusing,
    this matches much better with how it is used in practice, see below.
    Further, this opens up more use-cases for it in the future, namely
    aggressively searching for new peers in case we have discovered
    (possibly via some covert channel) headers which we do not know to be
    invalid, but which we cannot find block data for.
    
    Places pindexBestHeader is used:
    
     * Various GUI displays of the best header and getblockchaininfo["headers"],
       I don't think changing this is bad, and if anything this is less confusing
       in the presence of an invalid block.
     * IsCurrentForFeeEstimation(): If anything I think ensuring pindexBestHeader
       isn't some crazy invalid chain is better than the alternative, even in the
       case where you are rejecting the current chain due to hardware error (since
       hopefully in that case you won't get any new blocks anyway).
     * ConnectBlock assumevalid checks: We use pindexBestHeader to check that the
       block we're connecting leads to something with nMinimumChainWork (preventing
       a user-set assumevalid from having bogus work) and that the block we're
       connecting leads to pindexBestHeader (I'm not too worried about this one -
       it's nice to "disable" assumevalid if we have a long invalid headers chain,
       but I don't see it as a critical protection).
     * BlockRequestAllowed() uses pindexBestHeader as its target to ensure the
       requested block is within a month of the "current chain". I don't think this
       is a meaningful difference, if we're rejecting the current tip we're
       trivially fingerprintable anyway, and if the chain really does have a bunch
       of invalid crap near the tip, using the best not-invalid header is likely a
       better criteria.
     * ProcessGetBlockData uses pindexBestHeader as the "current chain" definition
       of whether a block request is "historical" for the purpose of bandwidth
       limiting. Similarly, I don't see why this is a meaningful change.
     * We use pindexBestHeader for requesting missing headers on receipt of a
       headers/compact block message or block inv as well as for initial getheaders.
       I think this is definitely wrong, using the best not-invalid header for such
       requests is much better.
     * We use pindexBestHeader to define the "current chain" for deciding when
       we're close to done with initial headers sync. I don't think this is a
       meaningful change.
     * We use pindexBestHeader to decide if initial headers sync has timed out. If
       we're rejecting the chain due to hardware error this may result in
       additional cases where we ban a peer, but this is already true, so I think
       its fine.
    06f1de0406
  16. (XXX: Use subtree merge instead): bump to latest upstream libsecp 99eaf1fe09
  17. (XXX: subtree me) add libc as a dep ca3ddddc09
  18. (XXX: subtree me) Add rust-bitcoin and deps 6bbb67abdd
  19. Build the new rusty deps into the rusty lib
    Note that we have to use LTO to work around the missing der_lax
    symbol in rust-secp256k1.
    See https://github.com/rust-bitcoin/rust-secp256k1/issues/168
    87c014d0b3
  20. Add additional FFIs from rust to support a full-fledged P2P client 640deb2332
  21. Add a FastRandomContext wrapper in the Rust FFI bridge 3f0598325e
  22. Add FFIs to Rust Bridge to check outbound connection nonces af2ea1e18d
  23. Add P2P Client d0be125006
  24. Add headers tree in test bridge and test locator response generation 33287e2a3f
  25. Add CoreLightning
    CoreLightning is a rusty interface to Lightning Network
    It uses the rust-lightning library
    6693089385
  26. DrahtBot added the label Build system on Feb 19, 2020
  27. DrahtBot added the label Docs on Feb 19, 2020
  28. DrahtBot added the label P2P on Feb 19, 2020
  29. DrahtBot added the label RPC/REST/ZMQ on Feb 19, 2020
  30. DrahtBot added the label Tests on Feb 19, 2020
  31. DrahtBot added the label Validation on Feb 19, 2020
  32. jonasschnelli commented at 5:34 pm on February 19, 2020: contributor

    While this is an impressive piece of work, I don’t think it makes sense to add layer 2 functionality to this project. Using the same repository, the same codebase, will automatically make reviews harder and will increase the risk of introducing critical bugs that impact all areas of the software.

    Instead of clutching everything together, we should try to achieve process separation (wallet / node / validation / GUI / etc.).

    What are the benefits of this over a separated Bitcoin-Core & c-lightning (or any other ln daemon)? If the user interface should be improved, one could developer a new type of a bitcoin-cli (a command-line client) that combines the calls to Bitcoin Core and a lightning daemon.

  33. ariard commented at 8:03 pm on February 19, 2020: member

    Thanks @icota for this cool PoC, that’s really impressive but I do share Jonas opinion here. Better would be to spend time on process separation/interface cleaning/reusable library than pouring more stuff in one repository which would slow down development process of every component involved. It would be great for bitcoin ecosystem to reuse Core components like wallet utxo-management or fee-bumping if we turn them more library-like a la rust-lightning and avoid a lot of reinventing the wheel again and again. It’s true they aren’t easy piece of software to get right.

    Enforcing a specific runtime on users is a hazardous move given the huge diversity of use-cases. A mobile wallet is easier to build as one monolithic process whereas exchange maybe interested to have a micro-services approach for high-availability. And there is also resources-constrained, embedded devices where you’re not going to run a full bitcoin stack on it. Running a full-node is already bandwidth-expensive in a lot of places, not even mentionning mobile data plan. So IMO modularity is a better way to serve a broad, diverse base of users/requirements.

  34. luke-jr commented at 9:45 pm on February 19, 2020: member

    What are the benefits of this over a separated Bitcoin-Core & c-lightning (or any other ln daemon)?

    Presumably one hypothetical benefit is not having a separate wallet.

  35. JeremyRubin commented at 10:46 pm on February 19, 2020: contributor

    Assume you were to rebase this on top of @ryanofsky’s process isolated wallet, could you keep all of the rust integrated stuff in the wallet process? Can the rust ln networking stuff be running a separate process from most of the wallet stuff?

    I think it would be a pretty decent design that you can offer a wallet module compiled with or without lightning, and then the “core node” software (which should have a minimum amount of stuff) can be just about validation.

  36. luke-jr commented at 0:09 am on February 20, 2020: member
    Can you split the rust-lightning code out of this, at least? Check for it in configure and dynamic link to its library…
  37. laanwj commented at 8:29 am on February 20, 2020: member

    I’m very divided about this. I’s impressive work, but also, I don’t want this project to compete with c-lightning, lnd et al. It’s their own specialty, they do what they do well, and maintaining the core validation layer is enough work for us. There shouldn’t be more going through the bottleneck of bitcoin core review. (also there’s the more abstract but not less important concern of layer 2 being permissionless-should “we” choose lightning as “winner”, should we integrate every possible, mature, layer 2 then? I’ve always been in favor of limiting the scope of this project, not extending it)

    I’m really sorry to say this after you did all this work.

  38. laanwj commented at 9:12 am on February 20, 2020: member

    Presumably one hypothetical benefit is not having a separate wallet.

    Presumably, if people want to pursue advanced mobile wallet functionality (I heard about Bluetooth integrations, or device power management, NFC and now L2 integration)—this is all very cool, but I think we need to limit the scope of this project, these are so far from block/transaction validation, the critical mission of this project. So bitcoin core’s wallet would have to split to a separate repository with different maintainers.

    (it can still be distributed together with the node, so from the perspective of an end-user not much would change, but avoids maintenance headaches, and the risks of complex, highly coupled systems, like we don’t want some wallet commit adding some shiny new feature to accidentally break validation)

  39. laanwj removed the label Build system on Feb 20, 2020
  40. laanwj removed the label Docs on Feb 20, 2020
  41. laanwj removed the label P2P on Feb 20, 2020
  42. laanwj removed the label Tests on Feb 20, 2020
  43. laanwj removed the label Validation on Feb 20, 2020
  44. laanwj added the label Wallet on Feb 20, 2020
  45. laanwj added the label Feature on Feb 20, 2020
  46. icota commented at 9:32 am on February 20, 2020: contributor

    Thank you all for the feedback, I really appreciate it.

    What are the benefits of this over a separated Bitcoin-Core & c-lightning (or any other ln daemon)? If the user interface should be improved, one could developer a new type of a bitcoin-cli (a command-line client) that combines the calls to Bitcoin Core and a lightning daemon. @jonasschnelli To open a channel in your scenario we are doing two additional steps (newaddr and sendtoaddress). Built-in CoreLightning approach saves the user time and money since we are broadcasting only one transaction instead of two. Huge UX win in my opinion.

    Enforcing a specific runtime on users is a hazardous move given the huge diversity of use-cases. A mobile wallet is easier to build as one monolithic process whereas exchange maybe interested to have a micro-services approach for high-availability. @ariard I absolutely agree with this. I don’t believe in a one-size-fits-all approach. I am familiar with @ryanofsky and your work on process separation. But crucially multiprocess is optional at compile time and one can still build a monolithic binary.

    I say crucially because, on mobile, monolithic is the only way to go. Multiple processes and IPC pose huge challenges on Android and this situation is not likely to improve (iOS is completely locked down). Take a look at this c-lightning issue and the following comments.

    Long story short mobile OSes want to manage the lifecycle of the apps. Spawning processes and various IPC goes against that grain.

    Assume you were to rebase this on top of @ryanofsky’s process isolated wallet, could you keep all of the rust integrated stuff in the wallet process? @JeremyRubin I’m not entirely sure. Is tx broadcast and notifying of new blocks part of the wallet process? Right now division of labour is like this: Rust code is dealing with Lightning only and Core does everything else.

    Having said all that I realise now that I got carried away a little bit. I was following work on #16834, #17376, #16762 and I lost sight that while fetching headers and blocks is a core (heh) competency of Core - Lightning is probably not.

    But before I close this I’d like to take the opportunity to propose a way forward.

    What about having CoreLightning as a separate library in its own repository? This way at configure time one could decide to link against it and the only thing needed on the Core side would be the rusty interface to stuff lightning needs like signing and broadcasting tx, fee estimation…

    Can you split the rust-lightning code out of this, at least? Check for it in configure and dynamic link to its library… @luke-jr It would have to look for much more than rust-lightning so I think the above approach is more appropriate.

    I’m really sorry to say this after you did all this work. @laanwj I really appreciate the sentiment but don’t be sorry. This was a great learning experience for me and I learned a ton. I would even go so far to say I had fun. :open_mouth:

  47. fanquake commented at 9:38 am on February 20, 2020: member

    I agree with @laanwj and most of the other commentors here. These comments:

    Presumably, if people want to pursue advanced mobile wallet functionality (I heard about Bluetooth integrations, or device power management, NFC and now L2 integration)—this is all very cool, but I think we need to limit the scope of this project, these are so far from block/transaction validation, the critical mission of this project. So bitcoin core’s wallet would have to split to a separate repository with different maintainers.

    came from a discussion me and @laanwj had after I took at look at @icota’s “mobile roadmap”.

  48. Sjors commented at 3:55 pm on February 20, 2020: member
    Can you make this repository a submodule? If that works, then hopefully you only need some modest changes in this repo, e.g. a way to insert RPC methods, or helper classes like BlockProviderState. That way you get to have a monolithic app for mobile.
  49. luke-jr commented at 5:32 pm on February 20, 2020: member

    @luke-jr It would have to look for much more than rust-lightning so I think the above approach is more appropriate.

    It’s not. Besides, what else would it need to look for? If rust-lightning itself has dependencies, consumers of rust-lightning shouldn’t need to care…

  50. ariard commented at 6:55 pm on February 20, 2020: member

    I say crucially because, on mobile, monolithic is the only way to go. Multiple processes and IPC pose huge challenges on Android and this situation is not likely to improve (iOS is completely locked down). Take a look at this c-lightning issue and the following comments.

    IMO the way to go is first to make Core wallet running as a standalone process, then when it’s well-done you can start to think about runtime integration with other bitcoin protocol stack like LN or Coinjoin feature-gated behind compilation flags. But anyway it should wait for core wallet being in its own repository, to avoid pouring any more burden maintenance/review on this project.

  51. icota commented at 7:06 pm on February 20, 2020: contributor

    Besides, what else would it need to look for? If rust-lightning itself has dependencies, consumers of rust-lightning shouldn’t need to care…

    rust-lightning-invoice is another dependency (of this PR). But okay, two dependencies are not a big deal.

    Can you make this repository a submodule? If that works, then hopefully you only need some modest changes in this repo, e.g. a way to insert RPC methods, or helper classes like BlockProviderState. That way you get to have a monolithic app for mobile.

    That is one possiblity. I’m leaning more toward librarising bitcoind in some way, in addition to subverting RPC. Helper classes are probably not needed, I’d guess RPC already has everything required.

    “Core Mobile” with its flashy UI links against a hypothetical “libbitcoind” an does an internal RPC dance. Power management, other mobile garbage and lightning is in that separate repository. Disruption and maintenance burden to this repository is kept to minimum. Everybody wins.

    Following a conversation with @fanquake today I will open an issue in the coming days where we can brainstorm this and other approaches to mobile and lightning.

    I’m closing this ill-concieved PR. :closed_lock_with_key:

  52. icota closed this on Feb 20, 2020

  53. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 10:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me