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

Prevent multiple calls to ExtractDestination #7825

Merged
merged 1 commit into from Jun 2, 2016

Conversation

Projects
None yet
7 participants
@pedrobranco
Contributor

pedrobranco commented Apr 6, 2016

Trivial performance improvement.

It consists in preventing multiple calls to ExtractDestination in rpc call listunspent. For an large UTXO set the improvement are more notable.

This PR moves the computation of multiple ExtractDestination to a single call, and also reorganizes the code of building the JSON result of each UTXO.

@laanwj laanwj added the Wallet label Apr 6, 2016

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Apr 7, 2016

Contributor

utACK 4a7c2f4

Contributor

dcousens commented Apr 7, 2016

utACK 4a7c2f4

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 9, 2016

Member

Concept ACK

Member

laanwj commented Apr 9, 2016

Concept ACK

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 22, 2016

Contributor

ACK 4a7c2f4

To get more people looking at this, please provide some interesting numbers ;-))

Contributor

paveljanik commented Apr 22, 2016

ACK 4a7c2f4

To get more people looking at this, please provide some interesting numbers ;-))

@pedrobranco

This comment has been minimized.

Show comment
Hide comment
@pedrobranco

pedrobranco May 3, 2016

Contributor

The performance improvement isn't much significant on typical wallets. The main advantage is the reuse of the function ExtractDestination( and the organization of listunspent call.

Contributor

pedrobranco commented May 3, 2016

The performance improvement isn't much significant on typical wallets. The main advantage is the reuse of the function ExtractDestination( and the organization of listunspent call.

@NicolasDorier

This comment has been minimized.

Show comment
Hide comment
@NicolasDorier

NicolasDorier May 5, 2016

Member

I would add this condition so it is equivalent to before:

if(!fValidAddress && setAddress.size() > 0)
    continue;
Member

NicolasDorier commented May 5, 2016

I would add this condition so it is equivalent to before:

if(!fValidAddress && setAddress.size() > 0)
    continue;
@pedrobranco

This comment has been minimized.

Show comment
Hide comment
@pedrobranco

pedrobranco May 5, 2016

Contributor

@NicolasDorier You are right. Will fix.

Contributor

pedrobranco commented May 5, 2016

@NicolasDorier You are right. Will fix.

@NicolasDorier

This comment has been minimized.

Show comment
Hide comment
@NicolasDorier

NicolasDorier May 5, 2016

Member

utACK 83153e6 (nit: redeemScript does not appear in the help message.)

Member

NicolasDorier commented May 5, 2016

utACK 83153e6 (nit: redeemScript does not appear in the help message.)

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Jun 2, 2016

Member

utACK 0bf6f30

Member

sipa commented Jun 2, 2016

utACK 0bf6f30

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 2, 2016

Member

Core-Review utACK 0bf6f30

Member

jonasschnelli commented Jun 2, 2016

Core-Review utACK 0bf6f30

@sipa sipa merged commit 0bf6f30 into bitcoin:master Jun 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

sipa added a commit that referenced this pull request Jun 2, 2016

Merge #7825: Prevent multiple calls to ExtractDestination
0bf6f30 Prevent multiple calls to ExtractDestination (Pedro Branco)

@pedrobranco pedrobranco deleted the uphold:enhancement/prevent-multiple-calls-extractdestination branch Jun 3, 2016

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #7825: Prevent multiple calls to ExtractDestination
0bf6f30 Prevent multiple calls to ExtractDestination (Pedro Branco)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #7825: Prevent multiple calls to ExtractDestination
0bf6f30 Prevent multiple calls to ExtractDestination (Pedro Branco)

codablock added a commit to codablock/dash that referenced this pull request Dec 22, 2017

Merge #7825: Prevent multiple calls to ExtractDestination
0bf6f30 Prevent multiple calls to ExtractDestination (Pedro Branco)

@str4d str4d referenced this pull request Apr 23, 2018

Merged

Bech32 encoding support #3202

zkbot added a commit to zcash/zcash that referenced this pull request Apr 23, 2018

Auto merge of #3202 - str4d:3058-sapling-bech32, r=<try>
Bech32 encoding support and t-addr encoding refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`
- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
- bitcoin/bitcoin#11167
  - Only the first three commits (the fourth commit depends on #2390, later ones are SegWit-specific).

Part of #3058.

zkbot added a commit to zcash/zcash that referenced this pull request Apr 24, 2018

Auto merge of #3202 - str4d:3058-sapling-bech32, r=<try>
Bech32 encoding support and t-addr encoding refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`
- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
- bitcoin/bitcoin#11167
  - Only the first three commits (the fourth commit depends on #2390, later ones are SegWit-specific).
- bitcoin/bitcoin#8578
- bitcoin/bitcoin#11372
  - Only the first commit (the rest block on refactoring to remove `CZCEncoding`)

Part of #3058.

zkbot added a commit to zcash/zcash that referenced this pull request Apr 25, 2018

Auto merge of #3202 - str4d:3058-sapling-bech32, r=<try>
Bech32 encoding support and t-addr encoding refactor

Includes code cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`
- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
- bitcoin/bitcoin#11167
  - Only the first three commits (the fourth commit depends on #2390, later ones are SegWit-specific).
- bitcoin/bitcoin#8578
- bitcoin/bitcoin#11372
  - Only the first three commits (the fourth commit depends on #2390)

Part of #3058.

zkbot added a commit to zcash/zcash that referenced this pull request Apr 25, 2018

Auto merge of #3202 - str4d:3058-sapling-bech32, r=<try>
Bech32 encoding support and address encoding refactor

Includes code cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`
- bitcoin/bitcoin#11117
- bitcoin/bitcoin#11259
- bitcoin/bitcoin#11167
  - Only the first three commits (the fourth commit depends on #2390, later ones are SegWit-specific).
- bitcoin/bitcoin#8578
- bitcoin/bitcoin#11372
  - Only the first three commits (the fourth commit depends on #2390)

Part of #3058.

@str4d str4d referenced this pull request Apr 26, 2018

Merged

Upstream encoding cleanups #3213

zkbot added a commit to zcash/zcash that referenced this pull request May 1, 2018

Auto merge of #3213 - str4d:3202-precursor, r=<try>
Upstream encoding cleanups

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`

Precursor to #3202.

zkbot added a commit to zcash/zcash that referenced this pull request May 1, 2018

Auto merge of #3213 - str4d:3202-precursor, r=<try>
Upstream encoding cleanups

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`

Precursor to #3202.

zkbot added a commit to zcash/zcash that referenced this pull request May 1, 2018

Auto merge of #3213 - str4d:3202-precursor, r=str4d
Upstream encoding cleanups

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7922
- bitcoin/bitcoin#7825
- bitcoin/bitcoin#8317
- bitcoin/bitcoin#9804
  - Only the commit that changed `base58.cpp`

Precursor to #3202.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment