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

refactor, wallet: Nuke coincontrol circular dependency #17518

Merged
merged 1 commit into from Nov 24, 2019

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Nov 19, 2019

This PR gets rid of wallet/coincontrol -> wallet/wallet -> wallet/coincontrol circular dependency.

@practicalswift
Copy link
Contributor

Concept ACK

Very nice :)

@Sjors
Copy link
Member

Sjors commented Nov 19, 2019

ACK 80303fa. Tested on macOS 10.15.1

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 19, 2019

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.

Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 80303fa.

@@ -11,6 +11,7 @@
#include <util/moneystr.h>
#include <util/system.h>
#include <util/translation.h>
#include <wallet/coincontrol.h> // for DEFAULT_AVOIDPARTIALSPENDS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

nit, does the comment really matter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


const int DEFAULT_MIN_DEPTH = 0;
const int DEFAULT_MAX_DEPTH = 9999999;

//! Default for -avoidpartialspends
static constexpr bool DEFAULT_AVOIDPARTIALSPENDS = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@hebasto hebasto Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

That was my first attempt too ;)

@hebasto
Copy link
Member Author

hebasto commented Nov 22, 2019

@promag @Sjors

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

Would you mind re-reviewing?

@hebasto
Copy link
Member Author

hebasto commented Nov 23, 2019

Rebased after #16944 has been merged.

@Sjors
Copy link
Member

Sjors commented Nov 23, 2019

re-ACK 3ed5e68

Copy link
Contributor

@meshcollider meshcollider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 3ed5e68

meshcollider added a commit that referenced this pull request Nov 24, 2019
3ed5e68 refactor: Nuke coincontrol circular dependency (Hennadii Stepanov)

Pull request description:

  This PR gets rid of `wallet/coincontrol` -> `wallet/wallet` -> `wallet/coincontrol` circular dependency.

ACKs for top commit:
  Sjors:
    re-ACK 3ed5e68
  meshcollider:
    utACK 3ed5e68

Tree-SHA512: 7fbceb74a9cd04157170df158d2deb979cf397df818376b478224d2423f1d8504a8688e3a9b8fc527da73e4a34ab6bc4a98be0cc2937e102a063ab2ac553e86d
@meshcollider meshcollider merged commit 3ed5e68 into bitcoin:master Nov 24, 2019
@hebasto hebasto deleted the 20191119-coincontrol-dep branch November 24, 2019 05:40
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 24, 2019
…ndency

3ed5e68 refactor: Nuke coincontrol circular dependency (Hennadii Stepanov)

Pull request description:

  This PR gets rid of `wallet/coincontrol` -> `wallet/wallet` -> `wallet/coincontrol` circular dependency.

ACKs for top commit:
  Sjors:
    re-ACK 3ed5e68
  meshcollider:
    utACK 3ed5e68

Tree-SHA512: 7fbceb74a9cd04157170df158d2deb979cf397df818376b478224d2423f1d8504a8688e3a9b8fc527da73e4a34ab6bc4a98be0cc2937e102a063ab2ac553e86d
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Nov 29, 2019
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 1, 2020
Summary:
3ed5e6819a50434449d92cb96b9d8d141e8c7d2b refactor: Nuke coincontrol circular dependency (Hennadii Stepanov)

Pull request description:

  This PR gets rid of `wallet/coincontrol` -> `wallet/wallet` -> `wallet/coincontrol` circular dependency.

---

Backport of Core [[bitcoin/bitcoin#17518 | PR17518]]

Test Plan:
  ninja check check-functional

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D7694
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
…ndency

3ed5e68 refactor: Nuke coincontrol circular dependency (Hennadii Stepanov)

Pull request description:

  This PR gets rid of `wallet/coincontrol` -> `wallet/wallet` -> `wallet/coincontrol` circular dependency.

ACKs for top commit:
  Sjors:
    re-ACK 3ed5e68
  meshcollider:
    utACK 3ed5e68

Tree-SHA512: 7fbceb74a9cd04157170df158d2deb979cf397df818376b478224d2423f1d8504a8688e3a9b8fc527da73e4a34ab6bc4a98be0cc2937e102a063ab2ac553e86d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants