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

Report progress in ReplayBlocks while rolling forward #13310

Merged
merged 1 commit into from Sep 13, 2018

Conversation

@promag
Copy link
Member

@promag promag commented May 23, 2018

Fixes #13303.

@promag
Copy link
Member Author

@promag promag commented May 23, 2018

Should only report each 5% for instance?

@Empact
Copy link
Member

@Empact Empact commented May 23, 2018

CWallet::ScanForWalletTransactions has an implementation that only reports every 1% 100 blocks.

@promag
Copy link
Member Author

@promag promag commented May 23, 2018

@Empact IMO the receiver is the one that should throttle, because he is the one that knows the resolution. Anyway, looks like ScanForWalletTransactions reports progress each 100 blocks, or am I wrong?

@Empact
Copy link
Member

@Empact Empact commented May 23, 2018

Yep you're right on ScanForWalletTransactions.

@GreatSock
Copy link
Contributor

@GreatSock GreatSock commented May 23, 2018

I think it should report every 1% change. But since changes less than 1% won't be visible, maybe check if the percentage have actually changed before updating. Calling ShowProgress 100 times should be fine right?

@promag
Copy link
Member Author

@promag promag commented May 23, 2018

I don't mind doing something like that...

But since changes less than 1% won't be visible

But how do you know? What if you have a progress bar with, for instance, 200px width? It should report 0.5%.

@GreatSock
Copy link
Contributor

@GreatSock GreatSock commented May 23, 2018

Right now ShowProgress seems to take an integer which means 1% changes won't be visible.

But I agree: ShowProgress should instead take a float(maybe even between 0 and 1) and then check if it should update by itself. This would probably be better since it can then choose to display the progress however it wants.

@promag
Copy link
Member Author

@promag promag commented May 23, 2018

Actually I was thinking in something like ShowProgress(int min, int value, int max).

@GreatSock
Copy link
Contributor

@GreatSock GreatSock commented May 23, 2018

I would personally prefer giving it a float since I feel like it would be more flexible. But then again, why not do both? We could just overload the function.

@jonasschnelli
Copy link
Member

@jonasschnelli jonasschnelli commented May 25, 2018

What is the best test plan to test this?

@DrahtBot DrahtBot closed this Jul 22, 2018
@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Jul 22, 2018

The last travis run for this pull request was 60 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

@DrahtBot DrahtBot reopened this Jul 22, 2018
@jb55
Copy link
Contributor

@jb55 jb55 commented Sep 6, 2018

Anything left to do here?

@laanwj
Copy link
Member

@laanwj laanwj commented Sep 13, 2018

utACK b16ab9a

@laanwj laanwj merged commit b16ab9a into bitcoin:master Sep 13, 2018
2 checks passed
laanwj added a commit that referenced this issue Sep 13, 2018
b16ab9a Report progress in ReplayBlocks while rolling forward (João Barbosa)

Pull request description:

  Fixes #13303.

Tree-SHA512: 9375bda03bd2527018b9d24a25c82fa01a841e41ae2cb5307be61af19e2b759d3a7db76852baba9a286fbcb52f70f427a5ab4375df08215ac439e47e73633e54
@laanwj
Copy link
Member

@laanwj laanwj commented Sep 13, 2018

Anything left to do here?

Not really—I think the most important thing to review here is that divide by zero never happens.

jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Jul 8, 2020
Summary:
b16ab9af07f802cc769c2443df1c637e8e12ab80 Report progress in ReplayBlocks while rolling forward (João Barbosa)

Pull request description:

  Fixes #13303.

---

Backport of Core [[bitcoin/bitcoin#13310 | PR13310]]

Test Plan:
  ninja check-all

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6849
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 27, 2021
…rward

b16ab9a Report progress in ReplayBlocks while rolling forward (João Barbosa)

Pull request description:

  Fixes bitcoin#13303.

Tree-SHA512: 9375bda03bd2527018b9d24a25c82fa01a841e41ae2cb5307be61af19e2b759d3a7db76852baba9a286fbcb52f70f427a5ab4375df08215ac439e47e73633e54
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 28, 2021
…rward

b16ab9a Report progress in ReplayBlocks while rolling forward (João Barbosa)

Pull request description:

  Fixes bitcoin#13303.

Tree-SHA512: 9375bda03bd2527018b9d24a25c82fa01a841e41ae2cb5307be61af19e2b759d3a7db76852baba9a286fbcb52f70f427a5ab4375df08215ac439e47e73633e54
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jun 29, 2021
…rward

b16ab9a Report progress in ReplayBlocks while rolling forward (João Barbosa)

Pull request description:

  Fixes bitcoin#13303.

Tree-SHA512: 9375bda03bd2527018b9d24a25c82fa01a841e41ae2cb5307be61af19e2b759d3a7db76852baba9a286fbcb52f70f427a5ab4375df08215ac439e47e73633e54
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this issue Jul 1, 2021
…rward

b16ab9a Report progress in ReplayBlocks while rolling forward (João Barbosa)

Pull request description:

  Fixes bitcoin#13303.

Tree-SHA512: 9375bda03bd2527018b9d24a25c82fa01a841e41ae2cb5307be61af19e2b759d3a7db76852baba9a286fbcb52f70f427a5ab4375df08215ac439e47e73633e54
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

8 participants