Skip to content

fix: early EHF and buried EHF are indistinguish #6434

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

Merged
merged 6 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 48 additions & 18 deletions src/evo/mnhftx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

static const std::string MNEHF_REQUESTID_PREFIX = "mnhf";
static const std::string DB_SIGNALS = "mnhf_s";
static const std::string DB_SIGNALS_v2 = "mnhf_s2";

uint256 MNHFTxPayload::GetRequestId() const
{
Expand Down Expand Up @@ -57,34 +58,33 @@ CMNHFManager::Signals CMNHFManager::GetSignalsStage(const CBlockIndex* const pin
{
if (!DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return {};

Signals signals = GetForBlock(pindexPrev);
Signals signals_tmp = GetForBlock(pindexPrev);

if (pindexPrev == nullptr) return {};
const int height = pindexPrev->nHeight + 1;
for (auto it = signals.begin(); it != signals.end(); ) {
bool found{false};
const auto signal_pindex = pindexPrev->GetAncestor(it->second);

Signals signals_ret;

for (auto signal : signals_tmp) {
bool expired{false};
const auto signal_pindex = pindexPrev->GetAncestor(signal.second);
assert(signal_pindex != nullptr);
const int64_t signal_time = signal_pindex->GetMedianTimePast();
for (int index = 0; index < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++index) {
const auto& deployment = Params().GetConsensus().vDeployments[index];
if (deployment.bit != it->first) continue;
if (deployment.bit != signal.first) continue;
if (signal_time < deployment.nStartTime) {
// new deployment is using the same bit as the old one
LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is expired at height=%d\n", it->first, it->second, height);
it = signals.erase(it);
} else {
++it;
LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is expired at height=%d\n",
signal.first, signal.second, height);
expired = true;
}
found = true;
break;
}
if (!found) {
// no deployment means we buried it and aren't using the same bit (yet)
LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is not known at height=%d\n", it->first, it->second, height);
it = signals.erase(it);
if (!expired) {
signals_ret.insert(signal);
}
}
return signals;
return signals_ret;
}

bool MNHFTx::Verify(const llmq::CQuorumManager& qman, const uint256& quorumHash, const uint256& requestId, const uint256& msgHash, TxValidationState& state) const
Expand Down Expand Up @@ -287,6 +287,9 @@ CMNHFManager::Signals CMNHFManager::GetForBlock(const CBlockIndex* pindex)
const Consensus::Params& consensusParams{Params().GetConsensus()};
while (!to_calculate.empty()) {
const CBlockIndex* pindex_top{to_calculate.top()};
if (pindex_top->nHeight % 1000 == 0) {
LogPrintf("re-index EHF signals at block %d\n", pindex_top->nHeight);
}
CBlock block;
if (!ReadBlockFromDisk(block, pindex_top, consensusParams)) {
throw std::runtime_error("failed-getehfforblock-read");
Expand Down Expand Up @@ -328,11 +331,19 @@ std::optional<CMNHFManager::Signals> CMNHFManager::GetFromCache(const CBlockInde
return signals;
}
}
if (m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) {
if (m_evoDb.Read(std::make_pair(DB_SIGNALS_v2, blockHash), signals)) {
LOCK(cs_cache);
mnhfCache.insert(blockHash, signals);
return signals;
}
if (!DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR)) {
// before mn_rr activation we are safe
if (m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) {
LOCK(cs_cache);
mnhfCache.insert(blockHash, signals);
return signals;
}
}
return std::nullopt;
}

Expand All @@ -346,7 +357,7 @@ void CMNHFManager::AddToCache(const Signals& signals, const CBlockIndex* const p
}
if (!DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return;

m_evoDb.Write(std::make_pair(DB_SIGNALS, blockHash), signals);
m_evoDb.Write(std::make_pair(DB_SIGNALS_v2, blockHash), signals);
}

void CMNHFManager::AddSignal(const CBlockIndex* const pindex, int bit)
Expand All @@ -364,6 +375,25 @@ void CMNHFManager::ConnectManagers(gsl::not_null<ChainstateManager*> chainman, g
m_qman = qman;
}

bool CMNHFManager::ForceSignalDBUpdate()
{
// force ehf signals db update
auto dbTx = m_evoDb.BeginTransaction();

const bool last_legacy = bls::bls_legacy_scheme.load();
bls::bls_legacy_scheme.store(false);
GetSignalsStage(m_chainman->ActiveChainstate().m_chain.Tip());
bls::bls_legacy_scheme.store(last_legacy);

dbTx->Commit();
// flush it to disk
if (!m_evoDb.CommitRootTransaction()) {
LogPrintf("CMNHFManager::%s -- failed to commit to evoDB\n", __func__);
return false;
}
Comment on lines +383 to +393
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we lock cs_main during all this to avoid chain advancing between bls::bls_legacy_scheme.load and bls::bls_legacy_scheme.store?

Copy link
Collaborator Author

@knst knst Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's already locked in init.cpp

            try {
                LOCK(cs_main);
...
                if (!node.mnhf_manager->ForceSignalDBUpdate()) {
                    strLoadError = _("Error upgrading evo database for EHF");
                    break;
                }

maybe annotation should be add? but that's a very specific method that is supposed to be called from initialization code only, not for regular usage, doesn't seems as necessary

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's please add annotations

return true;
}

std::string MNHFTx::ToString() const
{
return strprintf("MNHFTx(versionBit=%d, quorumHash=%s, sig=%s)",
Expand Down
2 changes: 2 additions & 0 deletions src/evo/mnhftx.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ class CMNHFManager : public AbstractEHFManager
*/
void DisconnectManagers() { m_chainman = nullptr; m_qman = nullptr; };

bool ForceSignalDBUpdate();

private:
void AddToCache(const Signals& signals, const CBlockIndex* const pindex);

Expand Down
4 changes: 4 additions & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
strLoadError = _("Error upgrading evo database");
break;
}
if (!node.mnhf_manager->ForceSignalDBUpdate()) {
strLoadError = _("Error upgrading evo database for EHF");
break;
}

for (CChainState* chainstate : chainman.GetAll()) {
if (!is_coinsview_empty(chainstate)) {
Expand Down
Loading