Skip to content

Enable changing the autoprune block space size in intro dialog #125

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 29, 2021
Merged

Enable changing the autoprune block space size in intro dialog #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62932cc
GUI/Intro: Return actual prune setting from showIfNeeded
luke-jr Apr 20, 2020
f2e5a6b
GUI/Intro: Abstract GUI-to-option into Intro::getPrune
luke-jr Nov 19, 2020
e2dcd95
GUI/Intro: Rework UI flow to let the user set prune size in GBs
luke-jr Apr 21, 2020
2a84c6b
GUI/Intro: Estimate max age of backups that can be restored with pruning
luke-jr Apr 21, 2020
415fb2e
GUI/Intro: Move prune setting below explanation
luke-jr Nov 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/qt/bitcoin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,9 @@ void BitcoinApplication::parameterSetup()
InitParameterInteraction(gArgs);
}

void BitcoinApplication::InitializePruneSetting(bool prune)
void BitcoinApplication::InitPruneSetting(int64_t prune_MiB)
{
// If prune is set, intentionally override existing prune size with
// the default size since this is called when choosing a new datadir.
optionsModel->SetPruneTargetGB(prune ? DEFAULT_PRUNE_TARGET_GB : 0, true);
optionsModel->SetPruneTargetGB(PruneMiBtoGB(prune_MiB), true);
}

void BitcoinApplication::requestInitialize()
Expand Down Expand Up @@ -508,9 +506,9 @@ int GuiMain(int argc, char* argv[])
/// 5. Now that settings and translations are available, ask user for data directory
// User language is set up: pick a data directory
bool did_show_intro = false;
bool prune = false; // Intro dialog prune check box
int64_t prune_MiB = 0; // Intro dialog prune configuration
Copy link
Member

Choose a reason for hiding this comment

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

style piconit: clang-format-diff.py suggests

Suggested change
int64_t prune_MiB = 0; // Intro dialog prune configuration
int64_t prune_MiB = 0; // Intro dialog prune configuration

// Gracefully exit if the user cancels
if (!Intro::showIfNeeded(did_show_intro, prune)) return EXIT_SUCCESS;
if (!Intro::showIfNeeded(did_show_intro, prune_MiB)) return EXIT_SUCCESS;

/// 6. Determine availability of data directory and parse bitcoin.conf
/// - Do not call GetDataDir(true) before this step finishes
Expand Down Expand Up @@ -594,7 +592,7 @@ int GuiMain(int argc, char* argv[])

if (did_show_intro) {
// Store intro dialog settings other than datadir (network specific)
app.InitializePruneSetting(prune);
app.InitPruneSetting(prune_MiB);
}

if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false))
Expand Down
2 changes: 1 addition & 1 deletion src/qt/bitcoin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class BitcoinApplication: public QApplication
/// Create options model
void createOptionsModel(bool resetSettings);
/// Initialize prune setting
void InitializePruneSetting(bool prune);
void InitPruneSetting(int64_t prune_MiB);
/// Create main window
void createWindow(const NetworkStyle *networkStyle);
/// Create splash screen
Expand Down
53 changes: 42 additions & 11 deletions src/qt/forms/intro.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<x>0</x>
<y>0</y>
<width>674</width>
<height>415</height>
<height>447</height>
</rect>
</property>
<property name="windowTitle">
Expand Down Expand Up @@ -210,16 +210,6 @@
</property>
</widget>
</item>
<item>
<widget class="QCheckBox" name="prune">
<property name="toolTip">
<string>Reverting this setting requires re-downloading the entire blockchain. It is faster to download the full chain first and prune it later. Disables some advanced features.</string>
</property>
<property name="text">
<string></string>
</property>
</widget>
</item>
<item>
<widget class="QLabel" name="lblExplanation2">
<property name="text">
Expand All @@ -240,6 +230,47 @@
</property>
</widget>
</item>
<item>
<layout class="QHBoxLayout" name="pruneOptLayout">
<item>
<widget class="QCheckBox" name="prune">
<property name="text">
<string>Limit block chain storage to</string>
</property>
<property name="toolTip">
<string>Reverting this setting requires re-downloading the entire blockchain. It is faster to download the full chain first and prune it later. Disables some advanced features.</string>
</property>
Comment on lines +237 to +242
Copy link
Member

Choose a reason for hiding this comment

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

Qt Designer insists on changing properties order:

Suggested change
<property name="text">
<string>Limit block chain storage to</string>
</property>
<property name="toolTip">
<string>Reverting this setting requires re-downloading the entire blockchain. It is faster to download the full chain first and prune it later. Disables some advanced features.</string>
</property>
<property name="toolTip">
<string>Reverting this setting requires re-downloading the entire blockchain. It is faster to download the full chain first and prune it later. Disables some advanced features.</string>
</property>
<property name="text">
<string>Limit block chain storage to</string>
</property>

</widget>
</item>
<item>
<widget class="QSpinBox" name="pruneGB">
<property name="suffix">
<string> GB</string>
</property>
</widget>
</item>
<item>
<widget class="QLabel" name="lblPruneSuffix">
<property name="buddy">
<cstring>pruneGB</cstring>
</property>
</widget>
</item>
<item>
<spacer name="horizontalSpacer_2">
<property name="orientation">
<enum>Qt::Horizontal</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>40</width>
<height>20</height>
</size>
</property>
</spacer>
</item>
</layout>
</item>
<item>
<spacer name="verticalSpacer">
<property name="orientation">
Expand Down
31 changes: 28 additions & 3 deletions src/qt/intro.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <interfaces/node.h>
#include <util/system.h>
#include <validation.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

77f3f2d

It's unfortunate to include this just because of MIN_DISK_SPACE_FOR_BLOCK_FILES.


#include <QFileDialog>
#include <QSettings>
Expand Down Expand Up @@ -139,17 +140,26 @@ Intro::Intro(QWidget *parent, int64_t blockchain_size_gb, int64_t chain_state_si
);
ui->lblExplanation2->setText(ui->lblExplanation2->text().arg(PACKAGE_NAME));

const int min_prune_target_GB = std::ceil(MIN_DISK_SPACE_FOR_BLOCK_FILES / 1e9);
Copy link
Member

Choose a reason for hiding this comment

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

nit: why here and after not use

gui/src/qt/guiconstants.h

Lines 52 to 53 in fa2a5b8

/* One gigabyte (GB) in bytes */
static constexpr uint64_t GB_BYTES{1000000000};

?

ui->pruneGB->setRange(min_prune_target_GB, std::numeric_limits<int>::max());
if (gArgs.GetArg("-prune", 0) > 1) { // -prune=1 means enabled, above that it's a size in MiB
ui->prune->setChecked(true);
ui->prune->setEnabled(false);
}
ui->prune->setText(tr("Discard blocks after verification, except most recent %1 GB (prune)").arg(m_prune_target_gb));
ui->pruneGB->setValue(m_prune_target_gb);
ui->pruneGB->setToolTip(ui->prune->toolTip());
ui->lblPruneSuffix->setToolTip(ui->prune->toolTip());
UpdatePruneLabels(ui->prune->isChecked());

connect(ui->prune, &QCheckBox::toggled, [this](bool prune_checked) {
UpdatePruneLabels(prune_checked);
UpdateFreeSpaceLabel();
});
connect(ui->pruneGB, QOverload<int>::of(&QSpinBox::valueChanged), [this](int prune_GB) {
Copy link
Member

Choose a reason for hiding this comment

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

The QOverload helper class is required for C++11 code. Having C++17, the qOverload should be used.

Copy link

Choose a reason for hiding this comment

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

IIRC qOverload function template does not work with Visual Studio 2017 due to implementation limitations, and I see that compiler mentioned in https://github.com/bitcoin-core/gui/blob/master/build_msvc/README.md .

Copy link
Member

Choose a reason for hiding this comment

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

IIRC qOverload function template does not work with Visual Studio 2017 due to implementation limitations

Oh, I was not aware of it. I'd like to know more about this pitfall. Any proof/link/test is appreciated :)

Copy link

Choose a reason for hiding this comment

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

@hebasto see https://bugreports.qt.io/browse/QTBUG-61667?focusedCommentId=462231&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-462231

Copy link
Member

Choose a reason for hiding this comment

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

@Talkless

Thanks! Since bitcoin/bitcoin#21811 is merged, it isn't a problem, is it?

Btw, could you look at #257?

Copy link

Choose a reason for hiding this comment

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

It seem so, yes. Maybe I should try building on some older GCC to make sure other compiler does not have similar limitations (though I doubt it).

I will take a look at 257 at some time.

m_prune_target_gb = prune_GB;
UpdatePruneLabels(ui->prune->isChecked());
UpdateFreeSpaceLabel();
});

startThread();
}
Expand Down Expand Up @@ -182,7 +192,17 @@ void Intro::setDataDirectory(const QString &dataDir)
}
}

bool Intro::showIfNeeded(bool& did_show_intro, bool& prune)
int64_t Intro::getPruneMiB() const
{
switch (ui->prune->checkState()) {
case Qt::Checked:
return PruneGBtoMiB(m_prune_target_gb);
case Qt::Unchecked: default:
return 0;
Comment on lines +200 to +201
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
case Qt::Unchecked: default:
return 0;
case Qt::Unchecked:
[[fallthrough]];
default:
return 0;

}
}

bool Intro::showIfNeeded(bool& did_show_intro, int64_t& prune_MiB)
{
did_show_intro = false;

Expand Down Expand Up @@ -233,7 +253,7 @@ bool Intro::showIfNeeded(bool& did_show_intro, bool& prune)
}

// Additional preferences:
prune = intro.ui->prune->isChecked();
prune_MiB = intro.getPruneMiB();

settings.setValue("strDataDir", dataDir);
settings.setValue("fReset", false);
Expand Down Expand Up @@ -361,6 +381,11 @@ void Intro::UpdatePruneLabels(bool prune_checked)
storageRequiresMsg = tr("Approximately %1 GB of data will be stored in this directory.");
}
ui->lblExplanation3->setVisible(prune_checked);
ui->pruneGB->setEnabled(prune_checked);
static constexpr uint64_t nPowTargetSpacing = 10 * 60; // from chainparams, which we don't have at this stage
static constexpr uint32_t expected_block_data_size = 2250000; // includes undo data
Comment on lines +385 to +386
Copy link
Member

Choose a reason for hiding this comment

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

style piconit: clang-format-diff.py suggests

Suggested change
static constexpr uint64_t nPowTargetSpacing = 10 * 60; // from chainparams, which we don't have at this stage
static constexpr uint32_t expected_block_data_size = 2250000; // includes undo data
static constexpr uint64_t nPowTargetSpacing = 10 * 60; // from chainparams, which we don't have at this stage
static constexpr uint32_t expected_block_data_size = 2250000; // includes undo data

const uint64_t expected_backup_days = m_prune_target_gb * 1e9 / (uint64_t(expected_block_data_size) * 86400 / nPowTargetSpacing);
ui->lblPruneSuffix->setText(tr("(sufficient to restore backups %n day(s) old)", "block chain pruning", expected_backup_days));
ui->sizeWarningLabel->setText(
tr("%1 will download and store a copy of the Bitcoin block chain.").arg(PACKAGE_NAME) + " " +
storageRequiresMsg.arg(m_required_space_gb) + " " +
Expand Down
5 changes: 3 additions & 2 deletions src/qt/intro.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Intro : public QDialog

QString getDataDirectory();
void setDataDirectory(const QString &dataDir);
int64_t getPruneMiB() const;
Copy link
Member

Choose a reason for hiding this comment

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

Could be private?


/**
* Determine data directory. Let the user choose if the current one doesn't exist.
Expand All @@ -47,7 +48,7 @@ class Intro : public QDialog
* @note do NOT call global GetDataDir() before calling this function, this
* will cause the wrong path to be cached.
*/
static bool showIfNeeded(bool& did_show_intro, bool& prune);
static bool showIfNeeded(bool& did_show_intro, int64_t& prune_MiB);

Q_SIGNALS:
void requestCheck();
Expand All @@ -72,7 +73,7 @@ private Q_SLOTS:
//! Total required space (in GB) depending on user choice (prune or not prune).
int64_t m_required_space_gb{0};
uint64_t m_bytes_available{0};
const int64_t m_prune_target_gb;
int64_t m_prune_target_gb;

void startThread();
void checkPath(const QString &dataDir);
Expand Down