Skip to content

Commit

Permalink
fix: off-by-one in the way we use v19 activation helpers (#5431)
Browse files Browse the repository at this point in the history
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)_
  • Loading branch information
UdjinM6 authored and PastaPastaPasta committed Jun 13, 2023
1 parent f391d7c commit 225398d
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 17 deletions.
7 changes: 1 addition & 6 deletions src/evo/deterministicmns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,13 +643,8 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
oldList = GetListForBlock(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",
Expand Down
4 changes: 2 additions & 2 deletions src/evo/specialtxman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
nTimeMerkle += nTime5 - nTime4;
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.
// V19 activated just activated, so we must switch to the new rules here.
bls::bls_legacy_scheme.store(false);
Expand All @@ -175,7 +175,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);
Expand Down
2 changes: 1 addition & 1 deletion src/governance/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion src/governance/vote.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
19 changes: 12 additions & 7 deletions src/llmq/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,16 +659,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);

LOCK(cs_llmq_vbc);
if (VersionBitsState(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache) != ThresholdState::ACTIVE) {
return nullptr;
}
int nHeight = VersionBitsStateSinceHeight(pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V19, llmq_versionbitscache);
return pindex->GetAncestor(nHeight);
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 IsInstantSendLLMQTypeShared()
Expand Down
1 change: 1 addition & 0 deletions src/llmq/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/// Returns the state of `-llmq-data-recovery`
Expand Down

0 comments on commit 225398d

Please sign in to comment.