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

1. 
   Sjors commented at 2:27 pm on April 19, 2018: member

   bool fPruneMode is replaced by an enum class to include an UNKNOWN state.

   fHavePruned is used to set fPruneNode if argument is not explictly set.

2. Sjors force-pushed on Apr 19, 2018
3. 
   Sjors commented at 2:34 pm on April 19, 2018: member

   I may have been a bit overzealous with assert(fPruneMode != PruneMode::UNKNOWN.

   fHavePruned is unknown during ParameterInteraction methods, because it is loaded from block index db, the location of which could be set by a parameter. It might be more elegant to split the ParameterInteraction methods into before and after databases have been loaded. That might also help to avoid various remaining instances of gArgs.GetArg("-prune".

4. Sjors renamed this:
   Interpret absense of prune= as prune=1 if there are pruned blocks
   WIP: Interpret absense of prune= as prune=1 if there are pruned blocks
   on Apr 20, 2018
5. Sjors force-pushed on Apr 20, 2018
6. Sjors renamed this:
   WIP: Interpret absense of prune= as prune=1 if there are pruned blocks
   Interpret absense of prune= as prune=1 if there are pruned blocks
   on Apr 20, 2018
7. 
   laanwj commented at 7:04 am on April 24, 2018: member

   I’m not entirely sure about this. Guessing values for options according to context has the drawback that the system becomes less transparent, harder to troubleshoot.

   Also it’s good to avoid IsArgSet as much as possible as it’s not possible to re-set an argument back to unset state, even by clearing it.

   It also seems a large code change for what feels like a small argument handling change.

   And if we do this, should we do the same for txindex?

8. 
   jonasschnelli commented at 8:12 am on April 24, 2018: contributor

   @Sjors: can you elaborate a little bit the use case this solves?

   Agree with @laanwj that other arguments would have the same problem (if we would agree it is a problem). It seems that this adds unnecessary complexity. I think the current behaviour of refusing to run pruned blockchains where prune= is not set is correct.

9. 
   Sjors commented at 10:39 am on April 25, 2018: member

   The problem I’m trying to solve here is being able to set prune= in QT (#13043) without causing errors if the user later launches bitcoind, or if the user resets their QT settings (at which point they’d get locked out).

   Another approach could be to reinterpret prune=0 to mean “you can’t prune now, but it’s OK if things were pruned in the past (fHavePruned=true)”.

   A similar problem exists for txindex, but that’s solved by #13033, which no longer wipes the transaction index if you launch with txindex=0.

10. 
    MarcoFalke commented at 6:39 pm on May 9, 2018: member

    Needs rebase if still relevant
11. Sjors force-pushed on May 15, 2018
12. 
    Sjors commented at 11:08 am on May 15, 2018: member

    Rebased.

    I might make a separate PR with a different approach:

    1. interpret prune=0 as not allowing manual pruning, but not failing if chain is already pruned
    2. if -txindex or -rescan are set, check if chain was actually pruned and fail if so

    I don’t think that approach would require much less code, but it is perhaps more future proof, once txindex and rescan work (better) with pruned nodes.

13. 
    MarcoFalke commented at 2:00 pm on May 30, 2018: member

    Needs rebase due to merge of #13341
14. Interpret absense of prune= as prune=1 if there are pruned blocks

    bool fPruneMode is replaced by an enum class to include an UNKNOWN state.
    fHavePruned is used to set fPruneNode if argument is not explictly set.

    fa90f1f662
15. Sjors force-pushed on May 30, 2018
16. 
    Sjors commented at 3:59 pm on May 30, 2018: member

    Rebased
17. laanwj referenced this in commit 6e249e4678 on Jun 11, 2018
18. 
    Sjors commented at 10:03 am on July 6, 2018: member

    Closing this because the rw_config approach in #11082 seems to have sufficient support.
19. Sjors closed this on Jul 6, 2018

    ---

20. Sjors deleted the branch on Jul 6, 2018
21. PastaPastaPasta referenced this in commit fdfbd01954 on May 11, 2020
22. DrahtBot locked this on Sep 8, 2021

Contributors
Sjors laanwj jonasschnelli MarcoFalke