[qt] OptionsDialog: add prune setting #13043

Sjors · 2018-04-20T13:22:24Z

The default suggested value is 2 GB. Minimum is 1 GB (550 MB rounded up).

When the user toggles this setting, a strong warning appears that undoing requires re-downloading the chain:

Tooltip points out that actual disk usage can be higher. It's a bit vague on the "advanced features", because I'm assuming anyone who needs to use -rescan and -txindex will read the documentation, and a more detailed text would needlessly confuse everyone else.

The UI uses gigabytes for readability and easy of use. There is also no manual pruning UI (prune=1). The user will have to use bitcoin.conf for those things.

Fixes #6461. When combined with #13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit bitcoin.conf. However I don't think that's an essential prerequisite.

randolf

Adding this feature to the GUI is a great idea.

laanwj · 2018-04-23T10:58:19Z

Concept ACK, will test and review. THanks a lot for adding this.

jonasschnelli · 2018-04-23T11:56:23Z

Nice! Concept ACK.
Thanks for adding this
One thought: why not adding this (additionally) to the intro screen (including showing the approximate disk usage reduction)?

laanwj · 2018-04-23T13:27:20Z

The default suggested value is 20 GB; ~1 month of worst case data. Minimum is 1 GB (550 MB rounded up).

What would be the motivation to not to default to the minimum?

Tested OK:

Enabled the checkbox (without changing the default of 20GB), restarted the client, next run it prunes:

2018-04-23T13:27:38Z init message: Pruning blockstore...
2018-04-23T13:28:20Z Prune: UnlinkPrunedFiles deleted blk/rev (00000)
...
2018-04-23T13:28:20Z Prune: UnlinkPrunedFiles deleted blk/rev (00783)

Re-enabling the checkbox and restarting causes the client to re-download the block chain, as expected, after the following dialog and clicking OK:

Sjors · 2018-04-25T10:48:57Z

why not adding this (additionally) to the intro screen (including showing the approximate disk usage reduction)?

That's what I had in mind as well, but I figured it can go in a separate PR.

What would be the motivation to not to default to the minimum?

To be on the safe side in case of a very large reorg, e.g. if some contentious user activated soft fork gets majority cumulative difficulty after a few weeks. If people think it's overkill, I'm happy to set it to 1 GB. If I remember correctly the limit of 550 MB was decided before SegWit, so maybe 2 GB would be a better default?

laanwj · 2018-04-25T14:08:57Z

To be on the safe side in case of a very large reorg, e.g. if some contentious user activated soft fork gets majority cumulative difficulty after a few weeks. If people think it's overkill, I'm happy to set it to 1 GB. If I remember correctly the limit of 550 MB was decided before SegWit, so maybe 2 GB would be a better default?

Oh, right, good point. From what I remember, the minimum was meant to be 550 blocks, reserving 1MiB per block, so 550MiB. This is no longer a good assumption with segwit blocks so having a higher default makes sense.

Also: NODE_NETWORK_LIMITED (BIP159) requires 288 blocks.

laanwj · 2018-04-25T14:10:20Z

@jonasschnelli can you trigger a build for this PR? Would be useful to have so people can test, I think.

jonasschnelli · 2018-04-26T06:43:08Z

Gitian build: https://bitcoin.jonasschnelli.ch/build/584

luke-jr · 2018-04-26T07:03:26Z

This adds a new QSettings prune option, distinct from the conf file. But the option cannot be set separately for GUI vs bitcoind, so this makes no sense...

jonasschnelli · 2018-04-26T07:22:59Z

Oh, right, good point. From what I remember, the minimum was meant to be 550 blocks, reserving 1MiB per block, so 550MiB. This is no longer a good assumption with segwit blocks so having a higher default makes sense.

Also: NODE_NETWORK_LIMITED (BIP159) requires 288 blocks.

The -prune=<value> is a "target" and there is a pruning protection of block files that contain blocks higher in the chain then the MIN_BLOCKS_TO_KEEP constant (288 blocks).
AFAIK it is impossible with the current code to prune blocks height then the 288 limit.

But, I agree that that the prune min target of 550 is misleading because it seems like that 550 can no longer be reached.

I also think it should be labeled as "target" (something we want to reach but not sure we can).

harding · 2018-04-26T15:23:25Z

Tested it a little bit. No problems found but here are two things I found unintuitive:

There doesn't seem to be any consistency on this screen for where to hover to see the tooltip. I've draw highlight boxes on a screenshot approximately where hovering shows me a tooltip. This seems like a somewhat pre-existing inconsistency only made slightly worse by this PR. (Debian 9, QT5 used)
The message that a restart is needed before the settings takes effect disappears after 10-15 seconds, which seems weird (but is probably pre-existing behavior).

Everything else seemed to work as expected on this previously-pruned node after restart.

harding · 2018-04-26T16:07:38Z

An additional suggestion: if the tooltip isn't already overfull, it might be worth mentioning that the software remains a full node even after limiting block storage, as I find it's a common misconception for people to think that pruned nodes aren't fully validating nodes. Perhaps saying something like:

Disables some advanced features but all blocks will still be fully validated. Reverting this setting requires re-downloading the entire blockchain. Actual disk usage may be somewhat higher.

(5 extra words; 31 extra characters)

mess110 · 2018-04-26T17:13:53Z

Tested ACK a475493

Should the word prune appear in the label? Maybe Prune block storage to

nit: if I click the Limit block storage to text, nothing happens, but if I click the other texts in the options dialog the corresponding checkbox is ticked.

nit: it is also the only option in the screen missing a alt+random-key shortcut. I can press alt+s to tick the first checkbox. However, this is the only element in this screen which has 2 things which would need a shortcut: the text and the input box. To solve this, you could do something similar to Connect through SOCKS5 proxy from Network tab.

tested on ubuntu qt5

maflcko · 2018-04-30T02:17:56Z

src/qt/optionsdialog.cpp

@@ -36,8 +36,17 @@ OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) :
    /* Main elements init */
    ui->databaseCache->setMinimum(nMinDbCache);
    ui->databaseCache->setMaximum(nMaxDbCache);
+    static const uint64_t GB = 1024 * 1024 * 1024;


note that GB is 1000^3 and 1024^3 is GiB

GB=1024^3 came first, although admittedly we have such precedent already.

I'll rename the variable to GiB, to be consistent with prune which also uses MiB.

luke-jr

Needs to not add a QSettings prune option

Sjors · 2018-04-30T11:13:23Z

@luke-jr wrote:

This adds a new QSettings prune option, distinct from the conf file. But the option cannot be set separately for GUI vs bitcoind, so this makes no sense...

That is what I was referring to in the description:

When combined with #13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit bitcoin.conf.

It's why I created #12833 (let QT write to config file, based on your #11082) and alternative #13029 (interpret absence of prune= setting in a way that using QSettings for this isn't a big deal).

@harding @mess110 I'll look into the tooltip range and click area. It should probably be broader. Will also look at your suggested text improvement.

@jonasschnelli thanks for the build

Sjors · 2018-05-15T10:47:57Z

Rebased, and:

reduced default to 2 GiB
fixed tooltip range and click area
included the suggested tooltip text improvement
replaced the word "limit" with "prune"
added alt+b keyboard shortcut, though I can't test that on macOS. Once checked, tab can be used to reached all thee input fields on that screen.

I don't see any way to address "Needs to not add a QSettings prune option" other than #12833 or #13029, so unless there is a better idea out there this PR will likely be stuck for a while.

@jonasschnelli:

I also think it should be labeled as "target" (something we want to reach but not sure we can).

Bit of a cop-out, but I think the tooltip "Actual disk usage may be somewhat higher" encapsulates that.

harding · 2018-05-16T17:41:36Z

Lightly tested cbede7d (but did not review code). Updated tooltip and tooltip range LGTM, thanks!

mess110 · 2018-05-16T22:43:39Z

Tested ACK cbede7d

ken2812221 · 2018-06-07T18:44:04Z

src/qt/optionsdialog.cpp

@@ -266,6 +280,11 @@ void OptionsDialog::on_hideTrayIcon_stateChanged(int fState)
    }
 }

+void OptionsDialog::togglePruneWarning(bool enabled)
+{
+    ui->pruneWarning->setVisible(!ui->pruneWarning->isVisible());


enabled is unused

^ This is up for grabs

ken2812221 · 2018-06-07T18:49:15Z

Concept ACK

laanwj · 2018-06-11T12:21:11Z

utACK cbede7d

cbede7d [qt] OptionsDialog: add prune setting (Sjors Provoost) Pull request description: The default suggested value is 2 GB. Minimum is 1 GB (550 MB rounded up). When the user toggles this setting, a strong warning appears that undoing requires re-downloading the chain: <img width="478" alt="schermafbeelding 2018-05-15 om 12 35 24" src="https://user-images.githubusercontent.com/10217/40051858-7939cc20-583c-11e8-9120-327a75376732.png"> Tooltip points out that actual disk usage can be higher. It's a bit vague on the "advanced features", because I'm assuming anyone who needs to use `-rescan` and `-txindex` will read the documentation, and a more detailed text would needlessly confuse everyone else. <img width="450" alt="schermafbeelding 2018-05-15 om 12 33 51" src="https://user-images.githubusercontent.com/10217/40051791-49d6156a-583c-11e8-97b9-7de6dfd8c481.png"> The UI uses gigabytes for readability and easy of use. There is also no manual pruning UI (`prune=1`). The user will have to use `bitcoin.conf` for those things. Fixes #6461. When combined with #13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit `bitcoin.conf`. However I don't think that's an essential prerequisite. Tree-SHA512: e17aff276d7235fbd40796adb6431d430620788a753ee13bc064abd35d2edc4280a3d3cddc18e42b4e00edff13ed18fd4f2a966c6f0b43b689afd13673e0c4bf

luke-jr · 2018-06-12T20:17:58Z

Why was this merged with QSettings?

Sjors · 2018-06-13T09:39:51Z

#12833 gets rid of QSettings almost completely. Adding this one setting to the migration there is trivial

If #11082 makes it into 0.17 then I can remove the QSettings stuff in #12833 as if it was never there.

If it takes longer, then the only gotcha is that users need to put prune=1 in bitcoin.conf if you want to use bitcoind.

jonasschnelli · 2018-06-13T09:49:47Z

Post merge tested ACK

Summary: cbede7d [qt] OptionsDialog: add prune setting (Sjors Provoost) Pull request description: The default suggested value is 2 GB. Minimum is 1 GB (550 MB rounded up). When the user toggles this setting, a strong warning appears that undoing requires re-downloading the chain: <img width="478" alt="schermafbeelding 2018-05-15 om 12 35 24" src="https://user-images.githubusercontent.com/10217/40051858-7939cc20-583c-11e8-9120-327a75376732.png"> Tooltip points out that actual disk usage can be higher. It's a bit vague on the "advanced features", because I'm assuming anyone who needs to use `-rescan` and `-txindex` will read the documentation, and a more detailed text would needlessly confuse everyone else. <img width="450" alt="schermafbeelding 2018-05-15 om 12 33 51" src="https://user-images.githubusercontent.com/10217/40051791-49d6156a-583c-11e8-97b9-7de6dfd8c481.png"> The UI uses gigabytes for readability and easy of use. There is also no manual pruning UI (`prune=1`). The user will have to use `bitcoin.conf` for those things. Fixes #6461. When combined with #13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit `bitcoin.conf`. However I don't think that's an essential prerequisite. Tree-SHA512: e17aff276d7235fbd40796adb6431d430620788a753ee13bc064abd35d2edc4280a3d3cddc18e42b4e00edff13ed18fd4f2a966c6f0b43b689afd13673e0c4bf Backport of Core PR13043 bitcoin/bitcoin#13043 Test Plan: make check bitcoin-qt Settings -> Options -> Main tab {F4079935} Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4271

cbede7d [qt] OptionsDialog: add prune setting (Sjors Provoost) Pull request description: The default suggested value is 2 GB. Minimum is 1 GB (550 MB rounded up). When the user toggles this setting, a strong warning appears that undoing requires re-downloading the chain: <img width="478" alt="schermafbeelding 2018-05-15 om 12 35 24" src="https://user-images.githubusercontent.com/10217/40051858-7939cc20-583c-11e8-9120-327a75376732.png"> Tooltip points out that actual disk usage can be higher. It's a bit vague on the "advanced features", because I'm assuming anyone who needs to use `-rescan` and `-txindex` will read the documentation, and a more detailed text would needlessly confuse everyone else. <img width="450" alt="schermafbeelding 2018-05-15 om 12 33 51" src="https://user-images.githubusercontent.com/10217/40051791-49d6156a-583c-11e8-97b9-7de6dfd8c481.png"> The UI uses gigabytes for readability and easy of use. There is also no manual pruning UI (`prune=1`). The user will have to use `bitcoin.conf` for those things. Fixes bitcoin#6461. When combined with bitcoin#13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit `bitcoin.conf`. However I don't think that's an essential prerequisite. Tree-SHA512: e17aff276d7235fbd40796adb6431d430620788a753ee13bc064abd35d2edc4280a3d3cddc18e42b4e00edff13ed18fd4f2a966c6f0b43b689afd13673e0c4bf

Summary: cbede7dbfde83d53ef38d257e9940af5f163b03c [qt] OptionsDialog: add prune setting (Sjors Provoost) Pull request description: The default suggested value is 2 GB. Minimum is 1 GB (550 MB rounded up). When the user toggles this setting, a strong warning appears that undoing requires re-downloading the chain: <img width="478" alt="schermafbeelding 2018-05-15 om 12 35 24" src="https://user-images.githubusercontent.com/10217/40051858-7939cc20-583c-11e8-9120-327a75376732.png"> Tooltip points out that actual disk usage can be higher. It's a bit vague on the "advanced features", because I'm assuming anyone who needs to use `-rescan` and `-txindex` will read the documentation, and a more detailed text would needlessly confuse everyone else. <img width="450" alt="schermafbeelding 2018-05-15 om 12 33 51" src="https://user-images.githubusercontent.com/10217/40051791-49d6156a-583c-11e8-97b9-7de6dfd8c481.png"> The UI uses gigabytes for readability and easy of use. There is also no manual pruning UI (`prune=1`). The user will have to use `bitcoin.conf` for those things. Fixes #6461. When combined with #13029 the user, after pruning their node, can safely reset settings and/or use bitcoind without having to edit `bitcoin.conf`. However I don't think that's an essential prerequisite. Tree-SHA512: e17aff276d7235fbd40796adb6431d430620788a753ee13bc064abd35d2edc4280a3d3cddc18e42b4e00edff13ed18fd4f2a966c6f0b43b689afd13673e0c4bf Backport of Core PR13043 bitcoin/bitcoin#13043 Test Plan: make check bitcoin-qt Settings -> Options -> Main tab {F4079935} Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D4271

fanquake added the GUI label Apr 20, 2018

Sjors mentioned this pull request Apr 20, 2018

Exposing prune option in GUI #6461

Closed

randolf approved these changes Apr 20, 2018

View reviewed changes

Sjors mentioned this pull request Apr 25, 2018

Interpret absense of prune= as prune=1 if there are pruned blocks #13029

Closed

maflcko reviewed Apr 30, 2018

View reviewed changes

luke-jr suggested changes Apr 30, 2018

View reviewed changes

[qt] OptionsDialog: add prune setting

cbede7d

Sjors force-pushed the 2018/04/qt-prune branch from a475493 to cbede7d Compare May 15, 2018 10:47

ken2812221 reviewed Jun 7, 2018

View reviewed changes

laanwj merged commit cbede7d into bitcoin:master Jun 11, 2018

Sjors deleted the 2018/04/qt-prune branch June 11, 2018 13:08

Bushstar mentioned this pull request Jun 12, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#343

Merged

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 13, 2021

Merge bitcoin#13043: [qt] OptionsDialog: add prune setting

1541e44

xdustinface pushed a commit to xdustinface/dash that referenced this pull request Apr 13, 2021

Merge bitcoin#13043: [qt] OptionsDialog: add prune setting

e5915e4

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[qt] OptionsDialog: add prune setting #13043

[qt] OptionsDialog: add prune setting #13043

Sjors commented Apr 20, 2018 •

edited

Loading

randolf left a comment

laanwj commented Apr 23, 2018

jonasschnelli commented Apr 23, 2018

laanwj commented Apr 23, 2018 •

edited

Loading

Sjors commented Apr 25, 2018

laanwj commented Apr 25, 2018 •

edited

Loading

laanwj commented Apr 25, 2018

jonasschnelli commented Apr 26, 2018

luke-jr commented Apr 26, 2018

jonasschnelli commented Apr 26, 2018

harding commented Apr 26, 2018

harding commented Apr 26, 2018

mess110 commented Apr 26, 2018 •

edited

Loading

maflcko Apr 30, 2018

luke-jr Apr 30, 2018

Sjors May 15, 2018

luke-jr left a comment

Sjors commented Apr 30, 2018

Sjors commented May 15, 2018

harding commented May 16, 2018

mess110 commented May 16, 2018

ken2812221 Jun 7, 2018

maflcko Jun 11, 2018

ken2812221 commented Jun 7, 2018

laanwj commented Jun 11, 2018

luke-jr commented Jun 12, 2018

Sjors commented Jun 13, 2018

jonasschnelli commented Jun 13, 2018

[qt] OptionsDialog: add prune setting #13043

[qt] OptionsDialog: add prune setting #13043

Conversation

Sjors commented Apr 20, 2018 • edited Loading

randolf left a comment

Choose a reason for hiding this comment

laanwj commented Apr 23, 2018

jonasschnelli commented Apr 23, 2018

laanwj commented Apr 23, 2018 • edited Loading

Sjors commented Apr 25, 2018

laanwj commented Apr 25, 2018 • edited Loading

laanwj commented Apr 25, 2018

jonasschnelli commented Apr 26, 2018

luke-jr commented Apr 26, 2018

jonasschnelli commented Apr 26, 2018

harding commented Apr 26, 2018

harding commented Apr 26, 2018

mess110 commented Apr 26, 2018 • edited Loading

maflcko Apr 30, 2018

Choose a reason for hiding this comment

luke-jr Apr 30, 2018

Choose a reason for hiding this comment

Sjors May 15, 2018

Choose a reason for hiding this comment

luke-jr left a comment

Choose a reason for hiding this comment

Sjors commented Apr 30, 2018

Sjors commented May 15, 2018

harding commented May 16, 2018

mess110 commented May 16, 2018

ken2812221 Jun 7, 2018

Choose a reason for hiding this comment

maflcko Jun 11, 2018

Choose a reason for hiding this comment

ken2812221 commented Jun 7, 2018

laanwj commented Jun 11, 2018

luke-jr commented Jun 12, 2018

Sjors commented Jun 13, 2018

jonasschnelli commented Jun 13, 2018

Sjors commented Apr 20, 2018 •

edited

Loading

laanwj commented Apr 23, 2018 •

edited

Loading

laanwj commented Apr 25, 2018 •

edited

Loading

mess110 commented Apr 26, 2018 •

edited

Loading