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

fix CDSNotificationInterface::UpdatedBlockTip signature #1562

Merged
merged 1 commit into from
Aug 4, 2017

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Aug 4, 2017

Fix it to match the one in CValidationInterface.

Thanks @chaeplin for reporting/digging/testing 👍

@UdjinM6 UdjinM6 added the bug label Aug 4, 2017
@UdjinM6 UdjinM6 added this to the 12.2 milestone Aug 4, 2017
@@ -16,7 +16,7 @@ class CDSNotificationInterface : public CValidationInterface

protected:
// CValidationInterface
void UpdatedBlockTip(const CBlockIndex *pindex);
void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload);
Copy link

Choose a reason for hiding this comment

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

May be we should use override keyword to prevent such errors in the future?

Copy link
Author

Choose a reason for hiding this comment

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

Good idea 👍 I'll include this in the next one to keep cleanup/refactoring separate from bug fixing.

@UdjinM6
Copy link
Author

UdjinM6 commented Aug 4, 2017

@OlegGirko
Some weird error about CConman https://travis-ci.org/dashpay/dash/jobs/260976461
Similar job in my own repo finished successfully https://travis-ci.org/UdjinM6/dash/jobs/260976270 so I guess it's unrelated to this PR.

to match the one in CValidationInterface
@UdjinM6
Copy link
Author

UdjinM6 commented Aug 4, 2017

Pushed to trigger build restart

@OlegGirko
Copy link

Wow, this was an oversight from my side to not update signature of UpdatedBlockTip() method of all classes inheriting from CValidationInterface when submitting PR #1556.

Thank you @chaeplin for finding this bug and thank you @UdjinM6 for fixing it!

urACK of course!

@UdjinM6 UdjinM6 merged commit 87707c0 into dashpay:v0.12.2.x Aug 4, 2017
@UdjinM6 UdjinM6 deleted the fixUpdatedBlockTip branch November 26, 2020 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants