diff --git a/src/avalanche/processor.h b/src/avalanche/processor.h --- a/src/avalanche/processor.h +++ b/src/avalanche/processor.h @@ -33,8 +33,6 @@ class PeerManager; struct bilingual_str; -using NodePeerManager = PeerManager; - /** * Finalization score. */ @@ -188,7 +186,6 @@ class Processor { CConnman *connman; - NodePeerManager *nodePeerManager; std::chrono::milliseconds queryTimeoutDuration; /** @@ -250,16 +247,15 @@ std::unique_ptr chainNotificationsHandler; Processor(interfaces::Chain &chain, CConnman *connmanIn, - NodePeerManager *nodePeerManagerIn, std::unique_ptr peerDataIn, CKey sessionKeyIn); public: ~Processor(); - static std::unique_ptr - MakeProcessor(const ArgsManager &argsman, interfaces::Chain &chain, - CConnman *connman, NodePeerManager *nodePeerManager, - bilingual_str &error); + static std::unique_ptr MakeProcessor(const ArgsManager &argsman, + interfaces::Chain &chain, + CConnman *connman, + bilingual_str &error); void setQueryTimeoutDuration(std::chrono::milliseconds d) { queryTimeoutDuration = d; @@ -272,7 +268,8 @@ // TODO: Refactor the API to remove the dependency on avalanche/protocol.h void sendResponse(CNode *pfrom, Response response) const; bool registerVotes(NodeId nodeid, const Response &response, - std::vector &updates); + std::vector &updates, int &banscore, + std::string &error); template auto withPeerManager(Callable &&func) const { LOCK(cs_peerManager); diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -8,8 +8,7 @@ #include #include #include -#include // For DecodeSecret -#include // For ::PeerManager +#include // For DecodeSecret #include #include #include @@ -219,11 +218,9 @@ }; Processor::Processor(interfaces::Chain &chain, CConnman *connmanIn, - NodePeerManager *nodePeerManagerIn, std::unique_ptr peerDataIn, CKey sessionKeyIn) - : connman(connmanIn), nodePeerManager(nodePeerManagerIn), - queryTimeoutDuration(AVALANCHE_DEFAULT_QUERY_TIMEOUT), round(0), - peerManager(std::make_unique()), + : connman(connmanIn), queryTimeoutDuration(AVALANCHE_DEFAULT_QUERY_TIMEOUT), + round(0), peerManager(std::make_unique()), peerData(std::move(peerDataIn)), sessionKey(std::move(sessionKeyIn)) { // Make sure we get notified of chain state changes. chainNotificationsHandler = @@ -235,10 +232,10 @@ stopEventLoop(); } -std::unique_ptr -Processor::MakeProcessor(const ArgsManager &argsman, interfaces::Chain &chain, - CConnman *connman, NodePeerManager *nodePeerManager, - bilingual_str &error) { +std::unique_ptr Processor::MakeProcessor(const ArgsManager &argsman, + interfaces::Chain &chain, + CConnman *connman, + bilingual_str &error) { std::unique_ptr peerData; CKey masterKey; CKey sessionKey; @@ -330,9 +327,8 @@ } // We can't use std::make_unique with a private constructor - return std::unique_ptr( - new Processor(chain, connman, nodePeerManager, std::move(peerData), - std::move(sessionKey))); + return std::unique_ptr(new Processor( + chain, connman, std::move(peerData), std::move(sessionKey))); } bool Processor::addBlockToReconcile(const CBlockIndex *pindex) { @@ -410,7 +406,8 @@ } bool Processor::registerVotes(NodeId nodeid, const Response &response, - std::vector &updates) { + std::vector &updates, int &banscore, + std::string &error) { { // Save the time at which we can query again. LOCK(cs_peerManager); @@ -430,7 +427,8 @@ auto w = queries.getWriteView(); auto it = w->find(std::make_tuple(nodeid, response.getRound())); if (it == w.end()) { - nodePeerManager->Misbehaving(nodeid, 2, "unexpected-ava-response"); + banscore = 2; + error = "unexpected-ava-response"; return false; } @@ -442,14 +440,15 @@ const std::vector &votes = response.GetVotes(); size_t size = invs.size(); if (votes.size() != size) { - nodePeerManager->Misbehaving(nodeid, 100, "invalid-ava-response-size"); + banscore = 100; + error = "invalid-ava-response-size"; return false; } for (size_t i = 0; i < size; i++) { if (invs[i].hash != votes[i].GetHash()) { - nodePeerManager->Misbehaving(nodeid, 100, - "invalid-ava-response-content"); + banscore = 100; + error = "invalid-ava-response-content"; return false; } } diff --git a/src/avalanche/test/processor_tests.cpp b/src/avalanche/test/processor_tests.cpp --- a/src/avalanche/test/processor_tests.cpp +++ b/src/avalanche/test/processor_tests.cpp @@ -84,8 +84,7 @@ // Get the processor ready. bilingual_str error; m_processor = Processor::MakeProcessor(*m_node.args, *m_node.chain, - m_node.connman.get(), - m_node.peerman.get(), error); + m_node.connman.get(), error); BOOST_CHECK(m_processor); // The master private key we delegate to. @@ -168,6 +167,14 @@ } uint64_t getRound() const { return AvalancheTest::getRound(*m_processor); } + + bool registerVotes(NodeId nodeid, const avalanche::Response &response, + std::vector &updates) { + int banscore; + std::string error; + return m_processor->registerVotes(nodeid, response, updates, banscore, + error); + } }; } // namespace @@ -336,7 +343,7 @@ auto registerNewVote = [&](const Response &resp) { runEventLoop(); auto nodeid = avanodes[nextNodeIndex++ % avanodes.size()]->GetId(); - BOOST_CHECK(m_processor->registerVotes(nodeid, resp, updates)); + BOOST_CHECK(registerVotes(nodeid, resp, updates)); }; // Let's vote for this block a few times. @@ -495,8 +502,8 @@ uint64_t round = getRound(); runEventLoop(); - BOOST_CHECK(m_processor->registerVotes( - avanodes[0]->GetId(), {round, 0, {Vote(0, blockHashA)}}, updates)); + BOOST_CHECK(registerVotes(avanodes[0]->GetId(), + {round, 0, {Vote(0, blockHashA)}}, updates)); BOOST_CHECK_EQUAL(updates.size(), 0); // Start voting on block B after one vote. @@ -515,7 +522,7 @@ for (int i = 0; i < 4; i++) { NodeId nodeid = getSuitableNodeToQuery(); runEventLoop(); - BOOST_CHECK(m_processor->registerVotes(nodeid, next(resp), updates)); + BOOST_CHECK(registerVotes(nodeid, next(resp), updates)); BOOST_CHECK_EQUAL(updates.size(), 0); } @@ -523,7 +530,7 @@ for (int i = 0; i < AVALANCHE_FINALIZATION_SCORE; i++) { NodeId nodeid = getSuitableNodeToQuery(); runEventLoop(); - BOOST_CHECK(m_processor->registerVotes(nodeid, next(resp), updates)); + BOOST_CHECK(registerVotes(nodeid, next(resp), updates)); BOOST_CHECK_EQUAL(updates.size(), 0); } @@ -537,7 +544,7 @@ BOOST_CHECK(firstNodeid != secondNodeid); // Next vote will finalize block A. - BOOST_CHECK(m_processor->registerVotes(firstNodeid, next(resp), updates)); + BOOST_CHECK(registerVotes(firstNodeid, next(resp), updates)); BOOST_CHECK_EQUAL(updates.size(), 1); BOOST_CHECK(updates[0].getBlockIndex() == pindexA); BOOST_CHECK_EQUAL(updates[0].getStatus(), BlockUpdate::Status::Finalized); @@ -550,7 +557,7 @@ BOOST_CHECK(invs[0].hash == blockHashB); // Next vote will finalize block B. - BOOST_CHECK(m_processor->registerVotes(secondNodeid, resp, updates)); + BOOST_CHECK(registerVotes(secondNodeid, resp, updates)); BOOST_CHECK_EQUAL(updates.size(), 1); BOOST_CHECK(updates[0].getBlockIndex() == pindexB); BOOST_CHECK_EQUAL(updates[0].getStatus(), BlockUpdate::Status::Finalized); @@ -600,16 +607,26 @@ // Respond to the request. Response resp = {round, 0, {Vote(0, blockHash)}}; - BOOST_CHECK(m_processor->registerVotes(avanodeid, resp, updates)); + BOOST_CHECK(registerVotes(avanodeid, resp, updates)); BOOST_CHECK_EQUAL(updates.size(), 0); // Now that avanode fullfilled his request, it is added back to the list of // queriable nodes. BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); + auto checkRegisterVotesError = [&](NodeId nodeid, + const avalanche::Response &response, + const std::string &expectedError) { + int banscore; + std::string error; + BOOST_CHECK(!m_processor->registerVotes(nodeid, response, updates, + banscore, error)); + BOOST_CHECK_EQUAL(error, expectedError); + BOOST_CHECK_EQUAL(updates.size(), 0); + }; + // Sending a response when not polled fails. - BOOST_CHECK(!m_processor->registerVotes(avanodeid, next(resp), updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, next(resp), "unexpected-ava-response"); // Trigger a poll on avanode. round = getRound(); @@ -620,22 +637,19 @@ // 1. Too many results. resp = {round, 0, {Vote(0, blockHash), Vote(0, blockHash)}}; runEventLoop(); - BOOST_CHECK(!m_processor->registerVotes(avanodeid, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, resp, "invalid-ava-response-size"); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); // 2. Not enough results. resp = {getRound(), 0, {}}; runEventLoop(); - BOOST_CHECK(!m_processor->registerVotes(avanodeid, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, resp, "invalid-ava-response-size"); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); // 3. Do not match the poll. resp = {getRound(), 0, {Vote()}}; runEventLoop(); - BOOST_CHECK(!m_processor->registerVotes(avanodeid, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, resp, "invalid-ava-response-content"); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); // 4. Invalid round count. Request is not discarded. @@ -643,22 +657,19 @@ runEventLoop(); resp = {queryRound + 1, 0, {Vote()}}; - BOOST_CHECK(!m_processor->registerVotes(avanodeid, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, resp, "unexpected-ava-response"); resp = {queryRound - 1, 0, {Vote()}}; - BOOST_CHECK(!m_processor->registerVotes(avanodeid, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, resp, "unexpected-ava-response"); // 5. Making request for invalid nodes do not work. Request is not // discarded. resp = {queryRound, 0, {Vote(0, blockHash)}}; - BOOST_CHECK(!m_processor->registerVotes(avanodeid + 1234, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid + 1234, resp, "unexpected-ava-response"); // Proper response gets processed and avanode is available again. resp = {queryRound, 0, {Vote(0, blockHash)}}; - BOOST_CHECK(m_processor->registerVotes(avanodeid, resp, updates)); + BOOST_CHECK(registerVotes(avanodeid, resp, updates)); BOOST_CHECK_EQUAL(updates.size(), 0); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); @@ -674,14 +685,13 @@ resp = {getRound(), 0, {Vote(0, blockHash), Vote(0, blockHash2)}}; runEventLoop(); - BOOST_CHECK(!m_processor->registerVotes(avanodeid, resp, updates)); - BOOST_CHECK_EQUAL(updates.size(), 0); + checkRegisterVotesError(avanodeid, resp, "invalid-ava-response-content"); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); // But they are accepted in order. resp = {getRound(), 0, {Vote(0, blockHash2), Vote(0, blockHash)}}; runEventLoop(); - BOOST_CHECK(m_processor->registerVotes(avanodeid, resp, updates)); + BOOST_CHECK(registerVotes(avanodeid, resp, updates)); BOOST_CHECK_EQUAL(updates.size(), 0); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); @@ -689,7 +699,7 @@ pindex2->nStatus = pindex2->nStatus.withFailed(); resp = {getRound(), 0, {Vote(0, blockHash)}}; runEventLoop(); - BOOST_CHECK(m_processor->registerVotes(avanodeid, resp, updates)); + BOOST_CHECK(registerVotes(avanodeid, resp, updates)); BOOST_CHECK_EQUAL(updates.size(), 0); BOOST_CHECK_EQUAL(getSuitableNodeToQuery(), avanodeid); } @@ -726,7 +736,7 @@ std::this_thread::sleep_for(std::chrono::milliseconds(1)); runEventLoop(); - bool ret = m_processor->registerVotes(avanodeid, next(resp), updates); + bool ret = registerVotes(avanodeid, next(resp), updates); if (std::chrono::steady_clock::now() > start + queryTimeDuration) { // We waited for too long, bail. Because we can't know for sure when // previous steps ran, ret is not deterministic and we do not check @@ -742,8 +752,7 @@ runEventLoop(); std::this_thread::sleep_for(queryTimeDuration); runEventLoop(); - BOOST_CHECK( - !m_processor->registerVotes(avanodeid, next(resp), updates)); + BOOST_CHECK(!registerVotes(avanodeid, next(resp), updates)); } } @@ -795,7 +804,7 @@ // Send one response, now we can poll again. auto it = node_round_map.begin(); Response resp = {it->second, 0, {Vote(0, blockHash)}}; - BOOST_CHECK(m_processor->registerVotes(it->first, resp, updates)); + BOOST_CHECK(registerVotes(it->first, resp, updates)); node_round_map.erase(it); invs = getInvsForNextPoll(); @@ -831,8 +840,7 @@ // Check that all nodes can vote. for (size_t i = 0; i < avanodes.size(); i++) { runEventLoop(); - BOOST_CHECK(m_processor->registerVotes(avanodes[i]->GetId(), next(resp), - updates)); + BOOST_CHECK(registerVotes(avanodes[i]->GetId(), next(resp), updates)); } // Generate a query for every single node. @@ -861,13 +869,13 @@ continue; } - BOOST_CHECK(m_processor->registerVotes( - nodeid, {r, 0, {Vote(0, blockHash)}}, updates)); + BOOST_CHECK( + registerVotes(nodeid, {r, 0, {Vote(0, blockHash)}}, updates)); BOOST_CHECK_EQUAL(m_processor->getConfidence(pindex), confidence); } - BOOST_CHECK(m_processor->registerVotes( - firstNodeId, {round, 0, {Vote(0, blockHash)}}, updates)); + BOOST_CHECK( + registerVotes(firstNodeId, {round, 0, {Vote(0, blockHash)}}, updates)); BOOST_CHECK_EQUAL(m_processor->getConfidence(pindex), confidence + 1); } @@ -925,8 +933,7 @@ std::chrono::steady_clock::now() + std::chrono::milliseconds(100); std::vector updates; - m_processor->registerVotes(nodeid, {queryRound, 100, {Vote(0, blockHash)}}, - updates); + registerVotes(nodeid, {queryRound, 100, {Vote(0, blockHash)}}, updates); for (int i = 0; i < 10000; i++) { // We make sure that we do not get a request before queryTime. UninterruptibleSleep(std::chrono::milliseconds(1)); diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -2483,8 +2483,7 @@ // Step 6.5 (I guess ?): Initialize Avalanche. bilingual_str avalancheError; g_avalanche = avalanche::Processor::MakeProcessor( - args, *node.chain, node.connman.get(), node.peerman.get(), - avalancheError); + args, *node.chain, node.connman.get(), avalancheError); if (!g_avalanche) { InitError(avalancheError); return false; diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4359,7 +4359,11 @@ } std::vector updates; - if (!g_avalanche->registerVotes(pfrom.GetId(), response, updates)) { + int banscore; + std::string error; + if (!g_avalanche->registerVotes(pfrom.GetId(), response, updates, + banscore, error)) { + Misbehaving(pfrom, banscore, error); return; } diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -23,7 +23,6 @@ "wallet/rpcwallet -> wallet/wallet -> wallet/rpcwallet" "wallet/wallet -> wallet/walletdb -> wallet/wallet" "avalanche/processor -> validation -> avalanche/processor" - "avalanche/processor -> net_processing -> avalanche/processor" "chainparams -> protocol -> chainparams" "chainparamsbase -> util/system -> chainparamsbase" "minerfund -> validation -> minerfund"