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

Some conditions won't trigger when reorging exactly from the forkpoint

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

reorg to 850000 and back on testnet
```
invalidateblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
reconsiderblock 0000003eddb94218e7a3f41b2ac6e26143f8a748b50cd26e86bdbbab9ebe50aa
```
this fails on develop and work with this patch

n/a

- [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 pasta
parent f391d7cac3
commit 225398dafc
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
6 changed files with 18 additions and 17 deletions

View File

@ -643,13 +643,8 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
oldList = GetListForBlock(pindex->pprev); oldList = GetListForBlock(pindex->pprev);
diff = oldList.BuildDiff(newList); 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); 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); m_evoDb.Write(std::make_pair(DB_LIST_SNAPSHOT, newList.GetBlockHash()), newList);
mnListsCache.emplace(newList.GetBlockHash(), newList); mnListsCache.emplace(newList.GetBlockHash(), newList);
LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n", LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n",

View File

@ -154,7 +154,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
nTimeMerkle += nTime5 - nTime4; nTimeMerkle += nTime5 - nTime4;
LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 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. // 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. // V19 activated just activated, so we must switch to the new rules here.
bls::bls_legacy_scheme.store(false); bls::bls_legacy_scheme.store(false);
@ -175,7 +175,7 @@ bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq:
auto bls_legacy_scheme = bls::bls_legacy_scheme.load(); auto bls_legacy_scheme = bls::bls_legacy_scheme.load();
try { 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. // 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. // Removing the activation block here, so we must switch back to the old rules.
bls::bls_legacy_scheme.store(true); bls::bls_legacy_scheme.store(true);

View File

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

View File

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

View File

@ -659,16 +659,21 @@ bool IsV19Active(const CBlockIndex* pindex)
return VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) == ThresholdState::ACTIVE; 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); assert(pindex);
LOCK(cs_llmq_vbc); LOCK(cs_llmq_vbc);
if (VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) != ThresholdState::ACTIVE) { if (VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) != ThresholdState::ACTIVE) {
return nullptr; return -1;
} }
int nHeight = VersionBitsStateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache); return VersionBitsStateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache);
return pindex->GetAncestor(nHeight); }
const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex)
{
assert(pindex);
return pindex->GetAncestor(V19ActivationHeight(pindex));
} }
bool IsInstantSendLLMQTypeShared() bool IsInstantSendLLMQTypeShared()

View File

@ -84,6 +84,7 @@ Consensus::LLMQType GetInstantSendLLMQType(const CQuorumManager& qman, const CBl
Consensus::LLMQType GetInstantSendLLMQType(bool deterministic); Consensus::LLMQType GetInstantSendLLMQType(bool deterministic);
bool IsDIP0024Active(const CBlockIndex* pindex); bool IsDIP0024Active(const CBlockIndex* pindex);
bool IsV19Active(const CBlockIndex* pindex); bool IsV19Active(const CBlockIndex* pindex);
const int V19ActivationHeight(const CBlockIndex* pindex);
const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex); const CBlockIndex* V19ActivationIndex(const CBlockIndex* pindex);
/// Returns the state of `-llmq-data-recovery` /// Returns the state of `-llmq-data-recovery`