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

GUI: fix model overlay header sync #15091

Merged
merged 2 commits into from Feb 6, 2019

Conversation

Projects
None yet
5 participants
@jonasschnelli
Copy link
Member

commented Jan 3, 2019

Update the block and header tip is constraint to have a minimal distance of 250ms between updates... which can lead to miss the last header update.

The modal overlay then assumes we are still in header sync and the view get stuck in "syncing headers,..." (while it's actually syncing blocks).

This removes the 250ms minimal delta for header updates as well as it fixes the correct display of how header updates should update the labels.

@jonasschnelli jonasschnelli added the GUI label Jan 3, 2019

@hebasto

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

Concept ACK.

Show resolved Hide resolved src/qt/modaloverlay.cpp Outdated

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2019/01/qt_fix_modal branch Jan 4, 2019

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

This removes the 250ms minimal delta for header updates as well as it fixes the correct display of how header updates should update the labels.

The delta is there to prevent a flood of GUI update messages. I don't removing it is a good solution, we should take care that one update makes it per 250ms though.

Show resolved Hide resolved src/qt/clientmodel.cpp Outdated
@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

The delta is there to prevent a flood of GUI update messages. I don't removing it is a good solution, we should take care that one update makes it per 250ms though.

I agree. This PR only removes the delay-update for header updates... which IMO should not result in GUI flood. AFAIK the GUI flood only gets problematic on chain tip updates.

jonasschnelli added some commits Jan 3, 2019

@jonasschnelli jonasschnelli force-pushed the jonasschnelli:2019/01/qt_fix_modal branch to e8db6b8 Jan 4, 2019

@hebasto

This comment has been minimized.

Copy link
Member

commented Jan 6, 2019

tACK e8db6b8 (Linux Mint 19.2).

@jonasschnelli jonasschnelli added this to the 0.18.0 milestone Jan 31, 2019

ui->expectedTimeLeft->setText(tr("Unknown..."));
}
}

void ModalOverlay::UpdateHeaderSyncLabel() {
int est_headers_left = bestHeaderDate.secsTo(QDateTime::currentDateTime()) / Params().GetConsensus().nPowTargetSpacing;
ui->numberOfBlocksLeft->setText(tr("Unknown. Syncing Headers (%1, %2%)...").arg(bestHeaderHeight).arg(QString::number(100.0 / (bestHeaderHeight + est_headers_left) * bestHeaderHeight, 'f', 1)));

This comment has been minimized.

Copy link
@promag

promag Jan 31, 2019

Member

nit, could use overloaded QString::arg http://doc.qt.io/qt-5/qstring.html#arg-8 instead of QString::number. Also, not really sure if it makes a difference but expression could be 100.0 * a / b.

@promag

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

utACK e8db6b8.

@laanwj laanwj merged commit e8db6b8 into bitcoin:master Feb 6, 2019

2 checks passed

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

laanwj added a commit that referenced this pull request Feb 6, 2019

Merge #15091: GUI: fix model overlay header sync
e8db6b8 Qt: Fix update headers-count (Jonas Schnelli)
7bb45e4 Qt: update header count regardless of update delay (Jonas Schnelli)

Pull request description:

  Update the block and header tip is constraint to have a minimal distance of 250ms between updates... which can lead to miss the last header update.

  The modal overlay then assumes we are still in header sync and the view get stuck in "syncing headers,..." (while it's actually syncing blocks).

  This removes the 250ms minimal delta for header updates as well as it fixes the correct display of how header updates should update the labels.

Tree-SHA512: 57608dac822b135cd604fc6ba1c80f25c0202a6e20bb140362026615d4bf243ef4fcc254a11bad36419c554a222a2f4947438d4ce44aa14041d1874751643d68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.