fix: off-by-one in the way we use v19 activation helpers (#5431)

## Issue being fixed or feature implemented
Some conditions won't trigger when reorging exactly from the forkpoint

## What was done?
pls see individual commits, tl;dr: you can't get correct results with
`GetAncestor` cause the answer is in the future

## How Has This Been Tested?
reorg to 850000 and back on testnet
```
invalidateblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
reconsiderblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
```
this fails on develop and work with this patch

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
This commit is contained in:
UdjinM6 2023-06-13 17:24:19 +03:00 committed by GitHub
parent f5ba5f5606
commit aa91946e20
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 18 additions and 20 deletions

View File

@ -611,13 +611,8 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
oldList = GetListForBlockInternal(pindex->pprev);
diff = oldList.BuildDiff(newList);
// NOTE: The block next to the activation is the one that is using new rules.
// If the fork was activated at pindex->prev block then current one is the first one
// using new rules. Save mn list snapsot for this block.
bool v19_just_activated = pindex->pprev == llmq::utils::V19ActivationIndex(pindex);
m_evoDb.Write(std::make_pair(DB_LIST_DIFF, newList.GetBlockHash()), diff);
if ((nHeight % DISK_SNAPSHOT_PERIOD) == 0 || oldList.GetHeight() == -1 || v19_just_activated) {
if ((nHeight % DISK_SNAPSHOT_PERIOD) == 0 || oldList.GetHeight() == -1) {
m_evoDb.Write(std::make_pair(DB_LIST_SNAPSHOT, newList.GetBlockHash()), newList);
mnListsCache.emplace(newList.GetBlockHash(), newList);
LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n",

View File

@ -169,7 +169,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
nTimeCbTxCL += nTime6 - nTime5;
LogPrint(BCLog::BENCHMARK, " - CheckCbTxBestChainlock: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeCbTxCL * 0.000001);
if (llmq::utils::V19ActivationIndex(pindex) == pindex) {
if (llmq::utils::V19ActivationHeight(pindex) == pindex->nHeight + 1) {
// NOTE: The block next to the activation is the one that is using new rules.
// V19 activated just activated, so we must switch to the new rules here.
bls::bls_legacy_scheme.store(false);
@ -190,7 +190,7 @@ bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq:
auto bls_legacy_scheme = bls::bls_legacy_scheme.load();
try {
if (llmq::utils::V19ActivationIndex(pindex) == pindex) {
if (llmq::utils::V19ActivationHeight(pindex) == pindex->nHeight + 1) {
// NOTE: The block next to the activation is the one that is using new rules.
// Removing the activation block here, so we must switch back to the old rules.
bls::bls_legacy_scheme.store(true);

View File

@ -316,7 +316,7 @@ bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const
{
CBLSSignature sig;
const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->nTime;
bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime;
sig.SetByteVector(vchSig, is_bls_legacy_scheme);
if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) {
LogPrintf("CGovernanceObject::CheckSignature -- VerifyInsecure() failed\n");

View File

@ -238,7 +238,7 @@ bool CGovernanceVote::CheckSignature(const CBLSPublicKey& pubKey) const
{
CBLSSignature sig;
const auto pindex = llmq::utils::V19ActivationIndex(::ChainActive().Tip());
bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->nTime;
bool is_bls_legacy_scheme = pindex == nullptr || nTime < pindex->pprev->nTime;
sig.SetByteVector(vchSig, is_bls_legacy_scheme);
if (!sig.VerifyInsecure(pubKey, GetSignatureHash(), is_bls_legacy_scheme)) {
LogPrintf("CGovernanceVote::CheckSignature -- VerifyInsecure() failed\n");

View File

@ -712,19 +712,21 @@ bool IsV19Active(const CBlockIndex* pindex)
return VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) == ThresholdState::ACTIVE;
}
const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex)
const int V19ActivationHeight(const CBlockIndex* pindex)
{
assert(pindex);
auto opt_height = [&pindex]() -> std::optional<int> {
LOCK(cs_llmq_vbc);
if (VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) != ThresholdState::ACTIVE) {
return std::nullopt;
}
return {VersionBitsStateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache)};
}();
if (opt_height == std::nullopt) return nullptr;
return pindex->GetAncestor(*opt_height);
LOCK(cs_llmq_vbc);
if (VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) != ThresholdState::ACTIVE) {
return -1;
}
return VersionBitsStateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache);
}
const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex)
{
assert(pindex);
return pindex->GetAncestor(V19ActivationHeight(pindex));
}
bool IsV20Active(const CBlockIndex* pindex)

View File

@ -84,6 +84,7 @@ Consensus::LLMQType GetInstantSendLLMQType(const CQuorumManager& qman, const CBl
Consensus::LLMQType GetInstantSendLLMQType(bool deterministic);
bool IsDIP0024Active(const CBlockIndex* pindex);
bool IsV19Active(const CBlockIndex* pindex);
const int V19ActivationHeight(const CBlockIndex* pindex);
const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex);
bool IsV20Active(const CBlockIndex* pindex);