fix: Do not use nHeight when trying to identify the very first/initial snapshot (#5538)

## Issue being fixed or feature implemented

`-1` will only mean `not initialized` from now

Should fix crashes like
https://gitlab.com/dashpay/dash/-/jobs/4893359925
This fix applied on top of #5525
https://gitlab.com/UdjinM6/dash/-/pipelines/972154075

## What was done?
Introduce and use `m_initial_snapshot_index` instead of re-using
`nHeight`. Added a couple of asserts to make sure:
1. we never create mn lists with `nHeight` set to `-1` _explicitly_ (but
it's ok for ctor with no params to do so)
2. we never set `nHeight` to `-1` for an existing mn list
3. we never try to get a height for a non-initialized list
4. `GetListForBlockInternal` never returns non-initialized mn lists

## How Has This Been Tested?
Run tests, run regtest/testnet wallets.

## Breaking Changes
We never stored snapshots with `nHeight == -1`, should be no breaking
changes I think.

## 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-08-22 00:46:57 +03:00 committed by GitHub
parent 84f2c29ccf
commit 4896809295
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 6 deletions

View File

@ -607,17 +607,13 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
return true;
}
if (newList.GetHeight() == -1) {
newList.SetHeight(nHeight);
}
newList.SetBlockHash(pindex->GetBlockHash());
oldList = GetListForBlockInternal(pindex->pprev);
diff = oldList.BuildDiff(newList);
m_evoDb.Write(std::make_pair(DB_LIST_DIFF, newList.GetBlockHash()), diff);
if ((nHeight % DISK_SNAPSHOT_PERIOD) == 0 || oldList.GetHeight() == -1) {
if ((nHeight % DISK_SNAPSHOT_PERIOD) == 0 || pindex->pprev == m_initial_snapshot_index) {
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",
@ -1051,8 +1047,11 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(const CBlo
CDeterministicMNListDiff diff;
if (!m_evoDb.Read(std::make_pair(DB_LIST_DIFF, pindex->GetBlockHash()), diff)) {
// no snapshot and no diff on disk means that it's the initial snapshot
snapshot = CDeterministicMNList(pindex->GetBlockHash(), -1, 0);
m_initial_snapshot_index = pindex;
snapshot = CDeterministicMNList(pindex->GetBlockHash(), pindex->nHeight, 0);
mnListsCache.emplace(pindex->GetBlockHash(), snapshot);
LogPrintf("CDeterministicMNManager::%s -- initial snapshot. blockHash=%s nHeight=%d\n",
__func__, snapshot.GetBlockHash().ToString(), snapshot.GetHeight());
break;
}
@ -1088,6 +1087,7 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(const CBlo
}
}
assert(snapshot.GetHeight() != -1);
return snapshot;
}

View File

@ -187,6 +187,7 @@ public:
nHeight(_height),
nTotalRegisteredCount(_totalRegisteredCount)
{
assert(nHeight >= 0);
}
template <typename Stream, typename Operation>
@ -299,10 +300,12 @@ public:
}
[[nodiscard]] int GetHeight() const
{
assert(nHeight >= 0);
return nHeight;
}
void SetHeight(int _height)
{
assert(_height >= 0);
nHeight = _height;
}
[[nodiscard]] uint32_t GetTotalRegisteredCount() const
@ -586,6 +589,7 @@ private:
std::unordered_map<uint256, CDeterministicMNList, StaticSaltedHasher> mnListsCache GUARDED_BY(cs);
std::unordered_map<uint256, CDeterministicMNListDiff, StaticSaltedHasher> mnListDiffsCache GUARDED_BY(cs);
const CBlockIndex* tipIndex GUARDED_BY(cs) {nullptr};
const CBlockIndex* m_initial_snapshot_index GUARDED_BY(cs) {nullptr};
public:
explicit CDeterministicMNManager(CEvoDB& evoDb, CConnman& _connman) :