bitcoin-wallet tool: Drop libbitcoin_server.a dependency #15639

1. 
   ryanofsky commented at 5:32 am on March 22, 2019: member

   Dropping the bitcoin-wallet dependency on libbitcoin_server.a ensures wallet code can’t access node global state, avoiding bugs like #15557 (review)
2. ryanofsky force-pushed on Mar 22, 2019
3. fanquake added the label Wallet on Mar 22, 2019
4. ryanofsky force-pushed on Mar 22, 2019
5. 
   DrahtBot commented at 7:25 am on March 22, 2019: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #15750 ([rpc] Remove the addresses field from the getaddressinfo return object by jnewbery)
   • #15719 (Drop Chain::requestMempoolTransactions method by ryanofsky)
   • #15557 (Enhance bumpfee to include inputs when targeting a feerate by instagibbs)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

6. ryanofsky force-pushed on Mar 22, 2019
7. ryanofsky force-pushed on Mar 22, 2019
8. 
   practicalswift commented at 10:09 am on March 22, 2019: contributor

   Concept ACK

   Decoupling is good.

   Somewhat related: #15612 (“Reduce the number of globals used”)

9. 
   l2a5b1 commented at 12:41 pm on March 25, 2019: contributor

   I just left feedback in #15638 pertaining to the newly introduced circular dependencies. I agree with @practicalswift that decoupling is good, but I hope we can do this without introducing new circular dependencies.
10. ryanofsky referenced this in commit 9742ff8d79 on Mar 30, 2019
11. DrahtBot added the label Needs rebase on Apr 1, 2019
12. ryanofsky force-pushed on Apr 8, 2019
13. DrahtBot removed the label Needs rebase on Apr 8, 2019
14. DrahtBot added the label Needs rebase on Apr 9, 2019
15. ryanofsky force-pushed on Apr 10, 2019
16. DrahtBot removed the label Needs rebase on Apr 10, 2019
17. bitcoin-wallet tool: Drop MakeChain calls

    Pass null Chain interface pointer to CWallet. This is needed to drop
    libbitcoin_server dependency and avoid linking node code.

    fbc6bb8e83
18. Remove access to node globals from wallet-linked code

    Remove last few instances of accesses to node global variables from wallet
    code. Also remove accesses to node globals from code in policy/policy.cpp that
    isn't actually called by wallet code, but does get linked into wallet code.
    
    This is the last change needed to allow bitcoin-wallet tool to be linked
    without depending on libbitcoin_server.a, to ensure wallet code doesn't access
    node global state and avoid bugs like
    https://github.com/bitcoin/bitcoin/pull/15557#discussion_r267735431

    b874747b51
19. bitcoin-wallet tool: Drop libbitcoin_server.a dependency

    This ensures wallet code doesn't access node global state, avoiding bugs like
    https://github.com/bitcoin/bitcoin/pull/15557#discussion_r267735431

    78a2fb55c9
20. laanwj referenced this in commit 6a135fbe5b on Apr 10, 2019
21. ryanofsky force-pushed on Apr 10, 2019
22. 
    ryanofsky commented at 2:33 pm on April 10, 2019: member

    I just left feedback in #15638 pertaining to the newly introduced circular dependencies

    Thanks, this was addressed some time ago in the other PR.

23. jgarzik approved
24. 
    jgarzik commented at 2:50 pm on April 10, 2019: contributor

    tested ACK.

    Cosmetic question/comment: is the proj moving back towards underscores in variables like old school Unix (dust_relay_fee), or sticking with C++ style lower-cased first letter, and no underscores?

25. 
    jnewbery commented at 3:13 pm on April 10, 2019: member

    utACK 78a2fb55c97fbc26f7b74c5b1fb999a2aff8ce88. Nice work, Russ.
26. 
    MarcoFalke commented at 3:47 pm on April 10, 2019: member

    utACK 78a2fb5
27. 
    meshcollider commented at 9:28 am on April 11, 2019: contributor

    utACK https://github.com/bitcoin/bitcoin/pull/15639/commits/78a2fb55c97fbc26f7b74c5b1fb999a2aff8ce88
28. meshcollider merged this on Apr 11, 2019
29. meshcollider closed this on Apr 11, 2019

    ---

30. 
    ryanofsky commented at 9:37 am on April 11, 2019: member

    re: #15639#pullrequestreview-225022833

    Cosmetic question/comment: is the proj moving back towards underscores in variables like old school Unix

    Pretty much yes. It’s documented in the developer notes: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-general

31. deadalnix referenced this in commit ae40c8eaf2 on May 22, 2020
32. deadalnix referenced this in commit ed7b9d6689 on May 26, 2020
33. deadalnix referenced this in commit 103cee9a7f on May 27, 2020
34. deadalnix referenced this in commit a7a55fb3ac on May 27, 2020
35. DrahtBot locked this on Dec 16, 2021

Contributors
ryanofsky DrahtBot practicalswift l2a5b1 jgarzik jnewbery MarcoFalke meshcollider

Labels
Wallet