refactor, wallet: Nuke coincontrol circular dependency #17518

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:20191119-coincontrol-dep changing 5 files +8 −4

hebasto commented at 5:48 PM on November 19, 2019: member

This PR gets rid of wallet/coincontrol -> wallet/wallet -> wallet/coincontrol circular dependency.
fanquake added the label Refactoring on Nov 19, 2019
fanquake added the label Wallet on Nov 19, 2019
practicalswift commented at 5:50 PM on November 19, 2019: contributor

Concept ACK

Very nice :)
Sjors commented at 6:30 PM on November 19, 2019: member

ACK 80303fa2b9a8eaaa00d8c3ddf665a5026675508d. Tested on macOS 10.15.1
DrahtBot commented at 10:08 PM on November 19, 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:
- #17211 (Allow fundrawtransaction and walletcreatefundedpsbt to take external inputs by achow101)
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.
in src/wallet/init.cpp:14 in 80303fa2b9 outdated
```
  10 | @@ -11,6 +11,7 @@
  11 |  #include <util/moneystr.h>
  12 |  #include <util/system.h>
  13 |  #include <util/translation.h>
  14 | +#include <wallet/coincontrol.h> // for DEFAULT_AVOIDPARTIALSPENDS
```
promag commented at 10:16 PM on November 21, 2019:

While here also add #include <ui_interface.h> because of InitError.

nit, does the comment really matter?

hebasto commented at 7:01 PM on November 22, 2019:

Done.
in src/wallet/coincontrol.h:19 in 80303fa2b9 outdated
```
  15 |  
  16 |  const int DEFAULT_MIN_DEPTH = 0;
  17 |  const int DEFAULT_MAX_DEPTH = 9999999;
  18 |  
  19 | +//! Default for -avoidpartialspends
  20 | +static constexpr bool DEFAULT_AVOIDPARTIALSPENDS = false;
```
promag commented at 10:18 PM on November 21, 2019:

I've tried to move these to a new header wallet/constants.h - which would allow to avoid #include <wallet/wallet.h> in some places - but suddenly it was already a lot of moved code.. so I think this is fine.

hebasto commented at 6:23 PM on November 22, 2019:

I've tried to move these to a new header...

That was my first attempt too ;)
promag commented at 10:27 PM on November 21, 2019: member

ACK 80303fa2b9a8eaaa00d8c3ddf665a5026675508d.
hebasto force-pushed on Nov 22, 2019
hebasto commented at 7:04 PM on November 22, 2019: member

@promag @Sjors

Thank you for your reviews. @promag's comments have been addressed.

Would you mind re-reviewing?
DrahtBot added the label Needs rebase on Nov 22, 2019
refactor: Nuke coincontrol circular dependency 3ed5e6819a
hebasto force-pushed on Nov 23, 2019
hebasto commented at 7:43 AM on November 23, 2019: member

Rebased after #16944 has been merged.
DrahtBot removed the label Needs rebase on Nov 23, 2019
Sjors commented at 11:19 AM on November 23, 2019: member

re-ACK 3ed5e6819a50434449d92cb96b9d8d141e8c7d2b
meshcollider commented at 5:23 AM on November 24, 2019: contributor

utACK 3ed5e6819a50434449d92cb96b9d8d141e8c7d2b
meshcollider referenced this in commit 9cbd87d8ee on Nov 24, 2019
meshcollider merged this on Nov 24, 2019
meshcollider closed this on Nov 24, 2019
hebasto deleted the branch on Nov 24, 2019
sidhujag referenced this in commit 0dd0c6eb48 on Nov 24, 2019
MarkLTZ referenced this in commit a93e7e185d on Nov 29, 2019
jasonbcox referenced this in commit 532d58920b on Oct 1, 2020
sidhujag referenced this in commit 97a74a714d on Nov 10, 2020
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

refactor, wallet: Nuke coincontrol circular dependency #17518

Conflicts