This is a known old bug, which was never truly fixed, due to it being harmless (and discovered after the last block-value reduction).

The function GetBlockValue(int nHeight) returns the block-reward value for a block at a given height (based on a "halving" schedule hardcoded within the function).
The issue is that it was being called passing the height of the previous block.
At some point, one particular call (in CreateCoinStake) was changed to use the (correct) current block height, while leaving the wrong one in all other places (including masternode payments/budgets and validation code).

This made no real difference with PIVX, as the change was introduced before the last milestone (which increased the block value from 4.5 to 5, rather than reducing it), and, after that, the block value never changed anymore (so it has always been GetBlockValue(N) == GetBlockValue(N-1)).
But it was an issue for all PIVX forks that had different schedules (or that introduced drastic changes to the masternode/staker-rewards proportions).

In #967 @CaveSpectre11 gives a complete explanation of the problem (already discussed on a wrong fix attempt in #814), and proposes to change only the call in CreateCoinStake, aligning it to the (wrong) height (mostly for the sake of consistency and for the benefit of future PIVX forks, as it has no effect on PIVX chain, at his current height).

This PR, instead, proposes the opposite solution. Align all other calls, so that the correct height is passed (pindex->nHeight during validation, and pindexPrev->nHeight + 1 when creating a new block).
This makes the code consistent with the actual block values on chain.
For example, the first-6-masternodes-premine was not in the genesis block, but at block 1.
Therefore we should have had 60001 * COIN returned by GetBlockValue(1), not by GetBlockValue(0).

• main-net https://explorer.pivx.link/block/000005504fa4a6766e854b2a2c3f21cd276fd7305b84f416241fd4431acbd12d
• testnet https://testnet.pivx.link/block/0000081796e8bf7f8047cc84681ceecb64da781e11961da844d0f8b7c13690a5

Thus, to align it with the values on chain (and prevent issues during initial sync) the schedule defined inside GetBlockValue is "shifted ahead" by one block.

This also refactors GetBlockValue to make it more readable.

Tested Resyncing from scratch on both testnet and mainnet.

Based on top of

• [Refactoring] Budget, round 2: review locks #1844

just to avoid merge conflicts with the refactoring of CBudgetManager::FillBlockPayee.
Only the last two commits are new here.