diff --git a/src/avalanche/peermanager.h b/src/avalanche/peermanager.h --- a/src/avalanche/peermanager.h +++ b/src/avalanche/peermanager.h @@ -177,8 +177,10 @@ public: class ConflictingProofHandler { public: - virtual void onConflictingProof(const std::shared_ptr &proof, - bool accepted) {} + virtual void + onConflictingProof(const std::shared_ptr &proof, + const std::unordered_set &conflictingPeerIds, + bool accepted) {} virtual ~ConflictingProofHandler() {} }; diff --git a/src/avalanche/peermanager.cpp b/src/avalanche/peermanager.cpp --- a/src/avalanche/peermanager.cpp +++ b/src/avalanche/peermanager.cpp @@ -338,7 +338,7 @@ } for (auto &cb : conflictingProofHandlers) { - cb->onConflictingProof(proof, accepted); + cb->onConflictingProof(proof, conflicting_peerids, accepted); } return peers.end(); diff --git a/src/avalanche/processor.h b/src/avalanche/processor.h --- a/src/avalanche/processor.h +++ b/src/avalanche/processor.h @@ -25,6 +25,7 @@ #include #include #include +#include #include class ArgsManager; @@ -188,8 +189,10 @@ } bool addBlockToReconcile(const CBlockIndex *pindex); - void addProofToReconcile(const std::shared_ptr &proof, - bool isAccepted); + void + addProofToReconcile(const std::shared_ptr &proof, + const std::unordered_set &conflictingPeerIds, + bool isAccepted); bool isAccepted(const CBlockIndex *pindex) const; int getConfidence(const CBlockIndex *pindex) const; @@ -229,7 +232,11 @@ */ uint256 buildLocalSighash(CNode *pfrom) const; + /** Test API */ friend struct ::avalanche::AvalancheTest; + + void setPeerData(const std::shared_ptr proof, const Delegation &dg, + PeerId peerid); }; } // namespace avalanche diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -109,6 +109,9 @@ struct Processor::PeerData { std::shared_ptr proof; Delegation delegation; + PeerId peerid; + + PeerData() : peerid(NO_PEER) {} }; class Processor::NotificationsHandler @@ -121,12 +124,16 @@ void updatedBlockTip() override { LOCK(m_processor->cs_peerManager); + m_processor->peerManager->updatedBlockTip(); + + // Fetch or create a peer id for our local proof and cache it. + // This needs to happen after the peer manager updatedBlockTip() in + // order to get the correct peerid in the case our proof just left the + // orphan pool. if (m_processor->peerData && m_processor->peerData->proof) { - m_processor->peerManager->registerProof( + m_processor->peerData->peerid = m_processor->peerManager->getPeerId( m_processor->peerData->proof); } - - m_processor->peerManager->updatedBlockTip(); } }; @@ -137,9 +144,11 @@ public: ConflictingProofHandler(Processor *p) : m_processor(p) {} - void onConflictingProof(const std::shared_ptr &proof, - bool accepted) override { - m_processor->addProofToReconcile(proof, accepted); + void + onConflictingProof(const std::shared_ptr &proof, + const std::unordered_set &conflictingPeerIds, + bool accepted) override { + m_processor->addProofToReconcile(proof, conflictingPeerIds, accepted); } }; @@ -277,13 +286,16 @@ .second; } -void Processor::addProofToReconcile(const std::shared_ptr &proof, - bool isAccepted) { +void Processor::addProofToReconcile( + const std::shared_ptr &proof, + const std::unordered_set &conflictingPeerIds, bool isAccepted) { // TODO We don't want to accept an infinite number of conflicting proofs. // They should be some rules to make them expensive and/or limited by // design. - proofsVoteRecords.getWriteView()->insert( - std::make_pair(proof, VoteRecord(isAccepted))); + proofsVoteRecords.getWriteView()->insert(std::make_pair( + proof, + VoteRecord(isAccepted && !conflictingPeerIds.count( + peerData ? peerData->peerid : NO_PEER)))); } bool Processor::isAccepted(const CBlockIndex *pindex) const { @@ -676,4 +688,15 @@ } while (nodeid != NO_NODE); } +void Processor::setPeerData(const std::shared_ptr proof, + const Delegation &dg, PeerId peerid) { + if (!peerData) { + peerData = std::make_unique(); + } + + peerData->proof = proof; + peerData->delegation = dg; + peerData->peerid = peerid; +} + } // namespace avalanche diff --git a/src/avalanche/test/peermanager_tests.cpp b/src/avalanche/test/peermanager_tests.cpp --- a/src/avalanche/test/peermanager_tests.cpp +++ b/src/avalanche/test/peermanager_tests.cpp @@ -38,8 +38,10 @@ std::shared_ptr lastProof; bool lastVote; - void onConflictingProof(const std::shared_ptr &proof, - bool accepted) override { + void + onConflictingProof(const std::shared_ptr &proof, + const std::unordered_set &conflictingPeerIds, + bool accepted) override { lastProof = proof; lastVote = accepted; } 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 @@ -38,6 +38,38 @@ } static uint64_t getRound(const Processor &p) { return p.round; } + + static void setLocalPeerId(Processor &p, PeerId peerid) { + p.setPeerData(std::make_shared(), Delegation(), peerid); + } + + static std::optional + getVoteForProof(const Processor &p, + const std::shared_ptr &proof) { + auto conflictingProofsReadView = p.conflicting_proofs.getReadView(); + for (auto it = conflictingProofsReadView.begin(); + it != conflictingProofsReadView.end(); it++) { + if (it->first->getId() == proof->getId()) { + return std::make_optional(VoteRecord(it->second)); + } + } + return std::nullopt; + } + }; + + class TestConflictingProofHandler + : public PeerManager::ConflictingProofHandler { + public: + std::shared_ptr lastProof; + bool lastVote; + + void + onConflictingProof(const std::shared_ptr &proof, + const std::unordered_set &conflictingPeerIds, + bool accepted) override { + lastProof = proof; + lastVote = accepted; + } }; } // namespace } // namespace avalanche @@ -982,4 +1014,24 @@ schedulerThread.join(); } +BOOST_AUTO_TEST_CASE(localProofConflict) { + const PeerId localPeerId = 42; + AvalancheTest::setLocalPeerId(*m_processor, localPeerId); + + auto checkVote = [&](const std::unordered_set &conflictingPeerIds, + bool expectedVote) { + auto proof = GetProof(); + m_processor->addProofToReconcile(proof, conflictingPeerIds, true); + auto vote = AvalancheTest::getVoteForProof(*m_processor, proof); + BOOST_CHECK(vote); + BOOST_CHECK_EQUAL(vote->isAccepted(), expectedVote); + }; + + checkVote({41}, true); + checkVote({42}, false); + checkVote({43}, true); + checkVote({41, 43}, true); + checkVote({41, 42, 43}, false); +} + BOOST_AUTO_TEST_SUITE_END()