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

Merge #16226 #4640

Merged
merged 1 commit into from
Apr 3, 2022
Merged

Merge #16226 #4640

merged 1 commit into from
Apr 3, 2022

Conversation

vijaydasmp
Copy link

e61de63 Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2 Move ismine to wallet module (Andrew Chow)

Pull request description:

IsMine isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves IsMine into the wallet module and for it to take a CWallet instead of CKeyStore. The test that used IsMine is also moved to the wallet tests.

This is first prerequisites for the wallet structure changes.

ACKs for commit e61de6:
MarcoFalke:
re-ACK e61de63 (only change is rebase with git auto-merge)
meshcollider:
Very light code review ACK bitcoin@e61de63

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341

@vijaydasmp vijaydasmp changed the title Merge #16226: Move ismine to the wallet module Merge #16226 #15639 Dec 28, 2021
@vijaydasmp vijaydasmp changed the title Merge #16226 #15639 Merge #16226 Dec 29, 2021
@vijaydasmp vijaydasmp marked this pull request as ready for review December 29, 2021 11:02
@vijaydasmp vijaydasmp marked this pull request as draft December 29, 2021 11:03
@vijaydasmp
Copy link
Author

Temporary interface_zmq failure

#include <script/script.h>

Copy link
Author

Choose a reason for hiding this comment

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

we dont have #include <script/sign.h> in line 10 (as in upstream) So two lines are deleted

@github-actions
Copy link

github-actions bot commented Jan 3, 2022

This pull request has conflicts, please rebase.

@vijaydasmp
Copy link
Author

feature_llmq_data_recovery test case failure is intermittent

@github-actions
Copy link

This pull request has conflicts, please rebase.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

pls see 5352236

@vijaydasmp
Copy link
Author

pls see 5352236

Thanks @UdjinM6

@vijaydasmp vijaydasmp marked this pull request as draft March 31, 2022 04:09
@vijaydasmp vijaydasmp marked this pull request as ready for review March 31, 2022 06:21
@vijaydasmp vijaydasmp requested a review from UdjinM6 March 31, 2022 06:22
@UdjinM6
Copy link

UdjinM6 commented Mar 31, 2022

still missing some parts from 5352236 in src/wallet/test/ismine_tests.cpp

e61de63 Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2 Move ismine to wallet module (Andrew Chow)

Pull request description:

  `IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.

  This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.

ACKs for commit e61de6:
  MarcoFalke:
    re-ACK e61de63 (only change is rebase with git auto-merge)
  meshcollider:
    Very light code review ACK bitcoin@e61de63

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341
#include <script/script.h>

#include <wallet/wallet.h>

Copy link
Author

Choose a reason for hiding this comment

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

Already a line break is here, do we need to add another?

@vijaydasmp
Copy link
Author

still missing some parts from 5352236 in `src/wallet/test/ismine_tests

still missing some parts from 5352236 in src/wallet/test/ismine_tests.cpp

Somehow I had missed it, now fixed, please have a look

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 77e8428 into dashpay:develop Apr 3, 2022
@UdjinM6 UdjinM6 added this to the 18 milestone Apr 3, 2022
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Dec 1, 2023
e61de63 Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2 Move ismine to wallet module (Andrew Chow)

Pull request description:

  `IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.

  This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.

ACKs for commit e61de6:
  MarcoFalke:
    re-ACK e61de63 (only change is rebase with git auto-merge)
  meshcollider:
    Very light code review ACK bitcoin@e61de63

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341

Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
gades pushed a commit to piratecash/pirate that referenced this pull request Dec 9, 2023
e61de63 Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2 Move ismine to wallet module (Andrew Chow)

Pull request description:

  `IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.

  This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.

ACKs for commit e61de6:
  MarcoFalke:
    re-ACK e61de63 (only change is rebase with git auto-merge)
  meshcollider:
    Very light code review ACK bitcoin@e61de63

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341

Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants