Skip to content

Conversation

@str4d
Copy link
Contributor

@str4d str4d commented Dec 19, 2019

Cherry-picked from the following upstream PRs:

sipa and others added 10 commits December 18, 2019 18:59
Before this, if someone imported a scriptPubKey directly (in hex form) using
importaddress, outputs sending to it would be treated as change, as the
corresponding CTxDestination was not added to the address book.

Fix this by trying to detect scriptPubKeys that are in fact convertible to a
CTxDestination and add them anyway. Add a warning to the RPC help to warn
against importing raw non-standard scripts.
Zcash: Excluding unit tests for SegWit logic.
Does not test watch-only addresses.

Zcash: Excluding SegWit tests and isInvalid code (which is part of the
SegWit support structure).
Zcash: Excludes changes to importwallet (missing bitcoin/bitcoin#11667)
and ProcessImport (missing bitcoin/bitcoin#7551).
Only IsMine's internal code needs this, as part of a recursion into P2SH and P2WSH
scripts. The exposed functions always operate on actual scriptPubKeys and not on
redeemScripts or witness scripts.

Zcash: Rewrote this commit so that it instead adds an internal
SigVersion argument (which we didn't have because it was added to
support SegWit logic).

Co-authored-by: Jack Grigg <[email protected]>
This will allow us to have the consensus code and IsMine code diverge.
Inside IsMine we care about the distinction between scriptPubKey execution
and P2SH redeemScript execution. The consensus code does not care about this
distinction, and thus SigVersion does not have a field for P2SH. As the IsMine
code will care, it uses a separate enum with more fields.
Such outputs can still be watched, and signed for, but they aren't treated as valid payments.
That means they won't cause transactions to appear in listtransactions, their outputs to be
shown under listunspent, or affect balances.
@str4d str4d added A-wallet Area: Wallet C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase. C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Dec 19, 2019
@str4d str4d added this to the v2.1.1 milestone Dec 19, 2019
@ebfull
Copy link
Contributor

ebfull commented Dec 19, 2019

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Dec 19, 2019

📌 Commit 82eefce has been approved by ebfull

@zkbot
Copy link
Contributor

zkbot commented Dec 19, 2019

⌛ Testing commit 82eefce with merge 900ed45...

zkbot added a commit that referenced this pull request Dec 19, 2019
Bitcoin wallet PRs 3

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7687
- bitcoin/bitcoin#11116
  - Excludes SegWit tests
  - Excludes `isInvalid` code (which is part of the SegWit support structure)
- bitcoin/bitcoin#13002
  - Excludes changes to `importwallet` (missing bitcoin/bitcoin#11667)
  - Excludes changes to `ProcessImport` (missing bitcoin/bitcoin#7551)
  - Third commit was rewritten to add an internal SigVersion argument (which we didn't have because it was added to support SegWit logic).
@zkbot
Copy link
Contributor

zkbot commented Dec 19, 2019

☀️ Test successful - pr-merge
Approved by: ebfull
Pushing 900ed45 to master...

@zkbot zkbot merged commit 82eefce into zcash:master Dec 19, 2019
@str4d str4d deleted the 2074-wallet-3 branch December 19, 2019 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-wallet Area: Wallet C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-upstream-port Category: Changes that are ported from the Bitcoin Core codebase.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants