Skip to content

Commit

Permalink
fix: Avoid using GetAdjustedTime() where adjusted time is not reall…
Browse files Browse the repository at this point in the history
…y needed or can be harmful (#5631)

## Issue being fixed or feature implemented
`GetAdjustedTime()` can be manipulated by our peers, we should avoid
using it for our internal data structures/logic.

## What was done?
Use `GetTime<T>()` instead, fix some includes while at it.

## How Has This Been Tested?
run tests, run a node

## Breaking Changes
should be none

## 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)_
  • Loading branch information
UdjinM6 authored Oct 23, 2023
1 parent faba796 commit d4e8aa7
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 32 deletions.
3 changes: 2 additions & 1 deletion src/evo/mnauth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <net.h>
#include <net_processing.h>
#include <netmessagemaker.h>
#include <util/time.h>
#include <validation.h>

void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
Expand Down Expand Up @@ -123,7 +124,7 @@ void CMNAuth::ProcessMessage(CNode& peer, PeerManager& peerman, CConnman& connma
}

if (!peer.fInbound) {
mmetaman->GetMetaInfo(mnauth.proRegTxHash)->SetLastOutboundSuccess(GetAdjustedTime());
mmetaman->GetMetaInfo(mnauth.proRegTxHash)->SetLastOutboundSuccess(GetTime<std::chrono::seconds>().count());
if (peer.m_masternode_probe_connection) {
LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- Masternode probe successful for %s, disconnecting. peer=%d\n",
mnauth.proRegTxHash.ToString(), peer.GetId());
Expand Down
8 changes: 4 additions & 4 deletions src/governance/classes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
#include <llmq/utils.h>
#include <primitives/transaction.h>
#include <script/standard.h>
#include <timedata.h>
#include <util/strencodings.h>
#include <validation.h>
#include <util/moneystr.h>
#include <util/strencodings.h>
#include <util/time.h>
#include <util/underlying.h>
#include <validation.h>

#include <univalue.h>

Expand Down Expand Up @@ -186,7 +186,7 @@ void CGovernanceTriggerManager::CleanAndRemove()
if (pObj) {
strDataAsPlainString = pObj->GetDataAsPlainString();
// mark corresponding object for deletion
pObj->PrepareDeletion(GetAdjustedTime());
pObj->PrepareDeletion(GetTime<std::chrono::seconds>().count());
}
LogPrint(BCLog::GOBJECT, "CGovernanceTriggerManager::CleanAndRemove -- Removing trigger object %s\n", strDataAsPlainString);
// delete the trigger
Expand Down
15 changes: 9 additions & 6 deletions src/governance/governance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <protocol.h>
#include <shutdown.h>
#include <spork.h>
#include <util/time.h>
#include <validation.h>

std::unique_ptr<CGovernanceManager> governance;
Expand Down Expand Up @@ -314,7 +315,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, CConnman

if (govobj.GetObjectType() == GovernanceObject::TRIGGER && !triggerman.AddNewTrigger(nHash)) {
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString());
objpair.first->second.PrepareDeletion(GetAdjustedTime());
objpair.first->second.PrepareDeletion(GetTime<std::chrono::seconds>().count());
return;
}

Expand Down Expand Up @@ -359,7 +360,7 @@ void CGovernanceManager::UpdateCachesAndClean()
triggerman.CleanAndRemove();

auto it = mapObjects.begin();
int64_t nNow = GetAdjustedTime();
int64_t nNow = GetTime<std::chrono::seconds>().count();

while (it != mapObjects.end()) {
CGovernanceObject* pObj = &((*it).second);
Expand Down Expand Up @@ -600,7 +601,7 @@ std::optional<CSuperblock> CGovernanceManager::CreateSuperblockCandidate(int nHe
int nNextSuperblock;

CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock);
auto SBEpochTime = static_cast<int64_t>(count_seconds(GetTime<std::chrono::seconds>()) + (nNextSuperblock - nHeight) * 2.62 * 60);
auto SBEpochTime = static_cast<int64_t>(GetTime<std::chrono::seconds>().count() + (nNextSuperblock - nHeight) * 2.62 * 60);
auto governanceBudget = CSuperblock::GetPaymentsLimit(nNextSuperblock);

CAmount budgetAllocated{};
Expand Down Expand Up @@ -1046,7 +1047,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
ostr << "CGovernanceManager::ProcessVote -- Unknown parent object " << nHashGovobj.ToString()
<< ", MN outpoint = " << vote.GetMasternodeOutpoint().ToStringShort();
exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_WARNING);
if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, GetAdjustedTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME))) {
if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, GetTime<std::chrono::seconds>().count() + GOVERNANCE_ORPHAN_EXPIRATION_TIME))) {
LEAVE_CRITICAL_SECTION(cs)
RequestGovernanceObject(pfrom, nHashGovobj, connman);
LogPrint(BCLog::GOBJECT, "%s\n", ostr.str());
Expand Down Expand Up @@ -1317,6 +1318,8 @@ void CGovernanceManager::AddCachedTriggers()
{
LOCK(cs);

int64_t nNow = GetTime<std::chrono::seconds>().count();

for (auto& objpair : mapObjects) {
CGovernanceObject& govobj = objpair.second;

Expand All @@ -1325,7 +1328,7 @@ void CGovernanceManager::AddCachedTriggers()
}

if (!triggerman.AddNewTrigger(govobj.GetHash())) {
govobj.PrepareDeletion(GetAdjustedTime());
govobj.PrepareDeletion(nNow);
}
}
}
Expand Down Expand Up @@ -1463,7 +1466,7 @@ void CGovernanceManager::CleanOrphanObjects()
LOCK(cs);
const vote_cmm_t::list_t& items = cmmapOrphanVotes.GetItemList();

int64_t nNow = GetAdjustedTime();
int64_t nNow = GetTime<std::chrono::seconds>().count();

auto it = items.begin();
while (it != items.end()) {
Expand Down
4 changes: 3 additions & 1 deletion src/governance/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <masternode/sync.h>
#include <messagesigner.h>
#include <net.h>
#include <timedata.h>
#include <util/time.h>
#include <validation.h>
#include <validationinterface.h>

Expand Down Expand Up @@ -734,7 +736,7 @@ void CGovernanceObject::UpdateSentinelVariables()
if ((GetAbsoluteYesCount(VOTE_SIGNAL_DELETE) >= nAbsDeleteReq) && !fCachedDelete) {
fCachedDelete = true;
if (nDeletionTime == 0) {
nDeletionTime = GetAdjustedTime();
nDeletionTime = GetTime<std::chrono::seconds>().count();
}
}
if (GetAbsoluteYesCount(VOTE_SIGNAL_ENDORSED) >= nAbsVoteReq) fCachedEndorsed = true;
Expand Down
7 changes: 4 additions & 3 deletions src/llmq/chainlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <txmempool.h>
#include <ui_interface.h>
#include <util/thread.h>
#include <util/time.h>
#include <validation.h>

namespace llmq
Expand Down Expand Up @@ -319,7 +320,7 @@ void CChainLocksHandler::TrySignChainTip()
LOCK(cs);
auto it = txFirstSeenTime.find(txid);
if (it != txFirstSeenTime.end()) {
txAge = GetAdjustedTime() - it->second;
txAge = GetTime<std::chrono::seconds>().count() - it->second;
}
}

Expand Down Expand Up @@ -381,7 +382,7 @@ void CChainLocksHandler::BlockConnected(const std::shared_ptr<const CBlock>& pbl
}
auto& txids = *it->second;

int64_t curTime = GetAdjustedTime();
int64_t curTime = GetTime<std::chrono::seconds>().count();

for (const auto& tx : pblock->vtx) {
if (tx->IsCoinBase() || tx->vin.empty()) {
Expand Down Expand Up @@ -456,7 +457,7 @@ bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid) const
LOCK(cs);
auto it = txFirstSeenTime.find(txid);
if (it != txFirstSeenTime.end()) {
txAge = GetAdjustedTime() - it->second;
txAge = GetTime<std::chrono::seconds>().count() - it->second;
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/llmq/quorums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <netmessagemaker.h>
#include <univalue.h>
#include <util/irange.h>
#include <util/time.h>
#include <util/underlying.h>
#include <validation.h>

Expand Down Expand Up @@ -895,7 +896,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
break;
}

if ((GetAdjustedTime() - nTimeLastSuccess) > nRequestTimeout) {
if ((GetTime<std::chrono::seconds>().count() - nTimeLastSuccess) > nRequestTimeout) {
if (nTries >= vecMemberHashes.size()) {
printLog("All tried but failed");
break;
Expand All @@ -913,7 +914,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
}
// Sleep a bit depending on the start offset to balance out multiple requests to same masternode
quorumThreadInterrupt.sleep_for(std::chrono::milliseconds(nMyStartOffset * 100));
nTimeLastSuccess = GetAdjustedTime();
nTimeLastSuccess = GetTime<std::chrono::seconds>().count();
connman.AddPendingMasternode(*pCurrentMemberHash);
printLog("Connect");
}
Expand All @@ -926,7 +927,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(const CQuorumCPtr pQuorum, co
}

if (RequestQuorumData(pNode, pQuorum->qc->llmqType, pQuorum->m_quorum_base_block_index, nDataMask, proTxHash)) {
nTimeLastSuccess = GetAdjustedTime();
nTimeLastSuccess = GetTime<std::chrono::seconds>().count();
printLog("Requested");
} else {
LOCK(cs_data_requests);
Expand Down
9 changes: 5 additions & 4 deletions src/llmq/signing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <netmessagemaker.h>
#include <scheduler.h>
#include <util/irange.h>
#include <util/time.h>
#include <util/underlying.h>
#include <validation.h>

Expand Down Expand Up @@ -322,7 +323,7 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
{
CDBBatch batch(*db);

uint32_t curTime = GetAdjustedTime();
uint32_t curTime = GetTime<std::chrono::seconds>().count();

// we put these close to each other to leverage leveldb's key compaction
// this way, the second key can be used for fast HasRecoveredSig checks while the first key stores the recSig
Expand Down Expand Up @@ -410,7 +411,7 @@ void CRecoveredSigsDb::CleanupOldRecoveredSigs(int64_t maxAge)
std::unique_ptr<CDBIterator> pcursor(db->NewIterator());

auto start = std::make_tuple(std::string("rs_t"), (uint32_t)0, (Consensus::LLMQType)0, uint256());
uint32_t endTime = (uint32_t)(GetAdjustedTime() - maxAge);
uint32_t endTime = (uint32_t)(GetTime<std::chrono::seconds>().count() - maxAge);
pcursor->Seek(start);

std::vector<std::pair<Consensus::LLMQType, uint256>> toDelete;
Expand Down Expand Up @@ -474,7 +475,7 @@ bool CRecoveredSigsDb::GetVoteForId(Consensus::LLMQType llmqType, const uint256&
void CRecoveredSigsDb::WriteVoteForId(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
{
auto k1 = std::make_tuple(std::string("rs_v"), llmqType, id);
auto k2 = std::make_tuple(std::string("rs_vt"), (uint32_t)htobe32(GetAdjustedTime()), llmqType, id);
auto k2 = std::make_tuple(std::string("rs_vt"), (uint32_t)htobe32(GetTime<std::chrono::seconds>().count()), llmqType, id);

CDBBatch batch(*db);
batch.Write(k1, msgHash);
Expand All @@ -488,7 +489,7 @@ void CRecoveredSigsDb::CleanupOldVotes(int64_t maxAge)
std::unique_ptr<CDBIterator> pcursor(db->NewIterator());

auto start = std::make_tuple(std::string("rs_vt"), (uint32_t)0, (Consensus::LLMQType)0, uint256());
uint32_t endTime = (uint32_t)(GetAdjustedTime() - maxAge);
uint32_t endTime = (uint32_t)(GetTime<std::chrono::seconds>().count() - maxAge);
pcursor->Seek(start);

CDBBatch batch(*db);
Expand Down
9 changes: 5 additions & 4 deletions src/llmq/signing_shares.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <spork.h>
#include <util/irange.h>
#include <util/thread.h>
#include <util/time.h>
#include <util/underlying.h>

#include <cxxtimer.hpp>
Expand Down Expand Up @@ -716,7 +717,7 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CConnma
}

// Update the time we've seen the last sigShare
timeSeenForSessions[sigShare.GetSignHash()] = GetAdjustedTime();
timeSeenForSessions[sigShare.GetSignHash()] = GetTime<std::chrono::seconds>().count();

if (!quorumNodes.empty()) {
// don't announce and wait for other nodes to request this share and directly send it to them
Expand Down Expand Up @@ -822,7 +823,7 @@ void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map<NodeId, std
{
AssertLockHeld(cs);

int64_t now = GetAdjustedTime();
int64_t now = GetTime<std::chrono::seconds>().count();
const size_t maxRequestsForNode = 32;

// avoid requesting from same nodes all the time
Expand Down Expand Up @@ -1240,7 +1241,7 @@ CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionI

void CSigSharesManager::Cleanup()
{
int64_t now = GetAdjustedTime();
int64_t now = GetTime<std::chrono::seconds>().count();
if (now - lastCleanupTime < 5) {
return;
}
Expand Down Expand Up @@ -1364,7 +1365,7 @@ void CSigSharesManager::Cleanup()
nodeStates.erase(nodeId);
}

lastCleanupTime = GetAdjustedTime();
lastCleanupTime = GetTime<std::chrono::seconds>().count();
}

void CSigSharesManager::RemoveSigSharesForSession(const uint256& signHash)
Expand Down
4 changes: 2 additions & 2 deletions src/llmq/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
#include <net.h>
#include <random.h>
#include <spork.h>
#include <timedata.h>
#include <util/irange.h>
#include <util/ranges.h>
#include <util/time.h>
#include <util/underlying.h>
#include <validation.h>
#include <versionbits.h>
Expand Down Expand Up @@ -939,7 +939,7 @@ void AddQuorumProbeConnections(const Consensus::LLMQParams& llmqParams, const CB
}

auto members = GetAllQuorumMembers(llmqParams.type, pQuorumBaseBlockIndex);
auto curTime = GetAdjustedTime();
auto curTime = GetTime<std::chrono::seconds>().count();

std::set<uint256> probeConnections;
for (const auto& dmn : members) {
Expand Down
4 changes: 2 additions & 2 deletions src/masternode/meta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <masternode/meta.h>

#include <flat-database.h>
#include <timedata.h>
#include <util/time.h>

#include <sstream>

Expand Down Expand Up @@ -34,7 +34,7 @@ UniValue CMasternodeMetaInfo::ToJson() const
{
UniValue ret(UniValue::VOBJ);

auto now = GetAdjustedTime();
auto now = GetTime<std::chrono::seconds>().count();

ret.pushKV("lastDSQ", nLastDsq);
ret.pushKV("mixingTxCount", nMixingTxCount);
Expand Down
3 changes: 2 additions & 1 deletion src/masternode/sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <shutdown.h>
#include <ui_interface.h>
#include <validation.h>
#include <util/time.h>
#include <util/translation.h>

class CMasternodeSync;
Expand Down Expand Up @@ -335,7 +336,7 @@ void CMasternodeSync::NotifyHeaderTip(const CBlockIndex *pindexNew, bool fInitia
void CMasternodeSync::UpdatedBlockTip(const CBlockIndex *pindexNew, bool fInitialDownload)
{
LogPrint(BCLog::MNSYNC, "CMasternodeSync::UpdatedBlockTip -- pindexNew->nHeight: %d fInitialDownload=%d\n", pindexNew->nHeight, fInitialDownload);
nTimeLastUpdateBlockTip = GetAdjustedTime();
nTimeLastUpdateBlockTip = GetTime<std::chrono::seconds>().count();

CBlockIndex* pindexTip = WITH_LOCK(cs_main, return pindexBestHeader);

Expand Down
3 changes: 2 additions & 1 deletion src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <util/sock.h>
#include <util/strencodings.h>
#include <util/thread.h>
#include <util/time.h>
#include <util/translation.h>
#include <validation.h>

Expand Down Expand Up @@ -2567,7 +2568,7 @@ void CConnman::ThreadOpenMasternodeConnections()
if (interruptNet)
return;

int64_t nANow = GetAdjustedTime();
int64_t nANow = GetTime<std::chrono::seconds>().count();
constexpr const auto &_func_ = __func__;

// NOTE: Process only one pending masternode at a time
Expand Down

0 comments on commit d4e8aa7

Please sign in to comment.