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

Staking effectively disabled if no block for 60 minutes #957

Open
CaveSpectre11 opened this issue May 26, 2021 · 4 comments
Open

Staking effectively disabled if no block for 60 minutes #957

CaveSpectre11 opened this issue May 26, 2021 · 4 comments

Comments

@CaveSpectre11
Copy link
Collaborator

The problem code is in miner.cpp:

            if (!gArgs.GetBoolArg("-genoverride", false) && (GetAdjustedTime() - nTimeLastBlock > 60*60 || IsInitialBlockDownload() || !g_connman->GetNodeCount(CConnman::NumConnections::CONNECTIONS_ALL) || nHeight < Params().HeightPoSStart())) {
                MilliSleep(5000);
                continue;
            }

In commit 5b7a6a6 things started to go down this path; that results in the staking effectively being turned off if there's no block in 3600 seconds. The intent of genoverride here is to allow another mined block to unstick the chain.

genoverride is a developer tool; but the basic thing is that if there's a significant drop in staking power, and we hit the limit on mined blocks in a row; we should still wait for a stake, for however long it takes, without needing a gen-overridden mined block to come.

@Zannick
Copy link
Collaborator

Zannick commented Jul 25, 2021

Would there be any negative effect from just removing this time check? GetAdjustedTime() - nTimeLastBlock > 60*60

@CaveSpectre11
Copy link
Collaborator Author

Would there be any negative effect from just removing this time check? GetAdjustedTime() - nTimeLastBlock > 60*60

The part that just logically throws me for a loop is that what it -should- allow is: If there's been too many consecutive mining blocks so the next block -has- to be a staking block, but there's not been a block for 6 minutes, it re-allows mining for another block.

@Zannick
Copy link
Collaborator

Zannick commented Aug 8, 2021

The part that just logically throws me for a loop is that what it -should- allow is: If there's been too many consecutive mining blocks so the next block -has- to be a staking block, but there's not been a block for 6 minutes, it re-allows mining for another block.

That's going to be a consensus change, because we're not turning off mining but rejecting the pow block:

veil/src/validation.cpp

Lines 4667 to 4671 in 27d0b70

if (!isNewBlock) {
if (pindexPrev->nHeight >= Params().ConsecutivePoWHeight() && !CheckConsecutivePoW(block, pindexPrev)) {
return state.DoS(100, false, REJECT_INVALID, "bad-pow", false, strprintf("too many consecutive pow blocks"));
}
}

The intent of genoverride here is to allow another mined block to unstick the chain.

I don't think that works, then, because this is only in staking code: the effect is only to allow the user of genoverride to create a stake after 3600 seconds. This would only be a change on the staking side, unless I missed a consensus check somewhere where we reject stake blocks after 3600 seconds (I don't think we do or we would have failed to revive testnet as much as we have).

Also, oops, 3600 seconds = 60 minutes, I don't know how I misread that before. That's a lot less worrisome than I initially thought. :)

@Zannick Zannick changed the title Staking effectively disabled if no block for 6 minutes Staking effectively disabled if no block for 60 minutes Aug 8, 2021
@seanPhill
Copy link
Collaborator

Aha. 60 minutes gels with our experience of testnets getting stalled. I don't really mind so long as we remember to include some PoW when getting things running.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants