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

RPC: Backport scantxoutset from BitCoin Core v0.17.2 #2780

Merged
merged 3 commits into from
Nov 20, 2022

Conversation

PeterL73
Copy link

@PeterL73 PeterL73 commented Nov 5, 2022

Issue being fixed or feature implemented

As I didn't like the idea of backporting partial functionality of an upstream feature I looked at how scantxoutset and descriptors where implemented in BitCoin Core. Thanks to @tecnovert in #2778 I was able to easily locate the introduction point of scantxoutset and descriptors into BitCoin Core. As there were only a few files involved I tried to backport the complete functionality from BitCoin Core v0.17.2.

What was done

  • added script/descriptor.cpp and script/descriptor.h from BitCoin Core
  • removed non PIVX compatible code from script/descriptor.cpp
    • changed #include <util.h> to #include <util/system.h>
    • removed wpkh, wsh support
  • added scantxoutset related code to rpc/blockchain.cpp and rpc/client.cpp
  • made some minor adjustments in backported code in rpc/blockchain.cpp
    • changed HexStr call used for outputting scriptPubKey
    • removed P2WPKH references from the help description
    • changed xpub/xprv to DRKV/DRKP as prefix for extended keys in the help description
  • added SigningProvider related code to script/sign.cpp and script/sign.h
  • added the functional test rpc_scantxoutset.py from BitCoin Core
    • changed the addresses and keys to fit PIVX requirements
    • changed the first of the "non HD unspent outputs" tests from 0.002 to 0.007, because PIVX does not have P2SH_SEGWIT or BECH32 addresses only LEGACY addresses are used in this test
    • dashpay@4127918

How Has This Been Tested

This was tested using pivx-cli and pivx-qt, by scanning for unspent transaction output with different descriptors and with the functional test.

Breaking Changes

- commented out non PIVX code in descriptor.cpp
- added scantxoutset related code to rpc/blockchain.cpp and rpc/client.cpp
- made some minor adjustments in BitCoins code (call to HexStr and help description) in rpc/blockchain.cpp
- added SigningProvider related code to script/sign.cpp and script/sign.h
- changed the addresses and keys to fit PIVX requirments
- changed the first non HD unspent outputs test from 0.002 to 0.007, because PIVX does not have P2SH_SEGWIT and BECH32 addresses, only LEGACY are tested here
@Fuzzbawls
Copy link
Collaborator

@tecnovert, since this supersedes your PR and is intended to suit your particular use case / implementation, would you please give it a review / approval?

@tecnovert
Copy link

tecnovert commented Nov 14, 2022

tACK fe55859

I've compiled this branch and verified that the functional test rpc_scantxoutset passes.
I also checked that the Basicswap PIVX tests pass using this branch.

Copy link
Member

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK fe55859

Compiled and compared to bitcoin:bitcoin#12196 and bitcoin:bitcoin#13697
Tested and runs as expected.

@Fuzzbawls Fuzzbawls added the Needs Backport Placeholder tag for anything needing a backport to prior version branches label Nov 18, 2022
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.

ACK fe55859

@Fuzzbawls Fuzzbawls merged commit 8cc2181 into PIVX-Project:master Nov 20, 2022
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 22, 2022
- commented out non PIVX code in descriptor.cpp
- added scantxoutset related code to rpc/blockchain.cpp and rpc/client.cpp
- made some minor adjustments in BitCoins code (call to HexStr and help description) in rpc/blockchain.cpp
- added SigningProvider related code to script/sign.cpp and script/sign.h

Github-Pull: PIVX-Project#2780
Rebased-From: c5ba307
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 22, 2022
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Nov 22, 2022
- changed the addresses and keys to fit PIVX requirments
- changed the first non HD unspent outputs test from 0.002 to 0.007, because PIVX does not have P2SH_SEGWIT and BECH32 addresses, only LEGACY are tested here

Github-Pull: PIVX-Project#2780
Rebased-From: fe55859
@Fuzzbawls Fuzzbawls mentioned this pull request Nov 22, 2022
@Fuzzbawls Fuzzbawls added Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes and removed Needs Backport Placeholder tag for anything needing a backport to prior version branches Needs Release Notes Placeholder tag for anything needing mention in the "Notable Changes" section of release notes labels Dec 11, 2022
Fuzzbawls added a commit that referenced this pull request Dec 12, 2022
c0f6055 [Consensus] Bump v5.5 activation height for mainnet (Fuzzbawls)
200b908 [Consensus] Set v5.5 activation height for mainnet (Fuzzbawls)
dc31cef Add functional test for scantxoutset (PeterL73)
b452fb6 Remove commented out code (PeterL73)
3fc2e6b Backport scantxoutset and descriptors from BitCoin Core v0.17.2 (PeterL73)
8f5f52d Remove semicolon and add "verbose" back to vRPCConvertParams. (tecnovert)
ba310d1 Test for hex strings and remove stray newline from rpc_named_arguments. (tecnovert)
2b244ba Fix missing help text and add backwards compatibility. (tecnovert)
cce6951 rpc: Backport getblock verbosity (tecnovert)
fd719a2 add include_delegated to vRPCConvertParams (PeterL73)
d285af0 use block height from caller (PeterL73)

Pull request description:

  Backports the following PRs to the `5.5` branch:
  #2772
  #2774
  #2779
  #2780
  #2781
  #2784

Top commit has no ACKs.

Tree-SHA512: b0858e8ce7a39a1a3399fb2cf3eb4c4e77ecd526d9c8ca38fab0dc1ca6d108c962a581c8e481d8ac584a35b8a88f58d9d887166a35aec8fdfb66e8d57232dfa8
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.

4 participants