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

Base work for the Sapling signatureHash #1666

Merged
merged 10 commits into from
Aug 5, 2020

Conversation

furszy
Copy link

@furszy furszy commented Jun 6, 2020

Base work for the new transaction digest algorithm for signature verification on PIVX Sapling transactions.

Essentially, an implementation of BIP143 + few more good commits that found down the rabbit hole.

Back ports:

Next step over this area (need 1553 merged to be able to push it) is the further specialization of BIP143 into our custom implementation of ZIP143 (with a different digest algorithm definition using our tx data and hash personalization).

@furszy furszy self-assigned this Jun 6, 2020
@random-zebra random-zebra added this to the 4.2.0 milestone Jun 8, 2020
@furszy furszy modified the milestones: 4.2.0, 5.0.0 Jun 9, 2020
@furszy furszy modified the milestones: 4.2.0, 5.0.0 Jun 29, 2020
@furszy furszy force-pushed the 2020_pip143 branch 2 times, most recently from c647a16 to 1b3742a Compare July 9, 2020 22:45
random-zebra
random-zebra previously approved these changes Jul 22, 2020
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK 3ae1342

@furszy furszy requested a review from Fuzzbawls August 2, 2020 08:21
@Fuzzbawls
Copy link
Collaborator

needs a rebase as #1700 being merged introduced new code in src/test/main_tests.cpp that is not compatible with the changes here:

test/main_tests.cpp:36:80: error: too few arguments to function call, expected 4, have 3
    DummySignatureCreator(nullptr).CreateSig(vchSig, pubKey.GetID(), scriptCode);
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                   ^
./script/sign.h:60:5: note: 'CreateSig' declared here
    bool CreateSig(std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const;
    ^
1 error generated.

furszy added 10 commits August 2, 2020 18:21
Coming from btc@bd0f41387783ee91623d7fac15e89e57db37df82
This is a preparation for BIP143 support.

-- Edited for PIVX merge --

Coming from btc@0ef1dd3e11dd573b6e443852ef0c72e34093ac68
Removing deprecated accounts checks and the hardcoded serialization.
Coming from btc@d2c5d044d00ec805957ab246a7863d83ca075805
Coming from btc@7ef8f3c072a8750c72a3a1cdc727b5c1d173bac8
Coming from btc@ab48c5e72156b34300db4a6521cb3c9969be3937
Coming from btc@35fe0393f216aa6020fc929272118eade5628636
@furszy
Copy link
Author

furszy commented Aug 2, 2020

Good, rebased + updated to latest master.
Single line change, ready for review.

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

utACK d1d15c8

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK d1d15c8 and merging...

@random-zebra random-zebra merged commit 5a09215 into PIVX-Project:master Aug 5, 2020
furszy added a commit that referenced this pull request Aug 21, 2020
…values

e02edd3 Cleaning isminetype::ISMINE_SPENDABLE_STAKEABLE. (furszy)
1bf9dc4 wallet: unify GetUnspentCredit with GetCredit (furszy)
9f2ae47 wallet: remove currently unused UpdateAmount method. (furszy)
bef69d3 wallet: add cachable amounts for caching credit/debit values (furszy)
8f2a156 Make IsMine stop distinguishing solvable/unsolvable (furszy)
e541593 Make coincontrol use IsSolvable to determine solvability (furszy)
e1a0cfb Back port of IsSolvable to check scripts solvability. (furszy)

Pull request description:

  Built on top of #1755 and #1666 .

  The goal of this PR has been to improve the `CWalletTx` to be able to add shielded cached balances in a much cleaner way.

  Including the following changes:

  * Adapted IsSolvable function from 0c8ea63 (without the witness check. Only used to check regular outputs to be able to decouple the output solvability from `ismine.h` enum)

  * Solvability decoupled from `ismine.h`. Back ported 6d714c3 and 4e91820 from bitcoin#13142.

  * And lastly, adapted bitcoin#15780 beauty cleanup to our sources.

ACKs for top commit:
  random-zebra:
    utACK e02edd3

Tree-SHA512: 3c54397585f2ce1a47a5a86292216b5beab6cbf1e4ad6c289a1daa958cf4c535855bac1e13a7de290bd8d374abef3da7a909b11ee0f5016af31e90f642b448d5
@random-zebra random-zebra modified the milestones: 5.0.0, 4.3.0 Sep 10, 2020
@furszy furszy modified the milestones: 4.3.0, 5.0.0 Sep 23, 2020
@furszy furszy deleted the 2020_pip143 branch November 29, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants