diff --git a/src/avalanche/processor.h b/src/avalanche/processor.h --- a/src/avalanche/processor.h +++ b/src/avalanche/processor.h @@ -251,12 +251,6 @@ void clearTimedoutRequests(); std::vector getInvsForNextPoll(bool forPoll = true); - /** - * Build and return the challenge whose signature is included in the - * AVAHELLO message that we send to a peer. - */ - uint256 buildLocalSighash(CNode *pfrom) const; - bool isWorthPolling(const CBlockIndex *pindex) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool isWorthPolling(const ProofRef &proof) const diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -612,27 +612,28 @@ } bool Processor::sendHello(CNode *pfrom) const { - if (!peerData) { - // We do not have a delegation to advertise. - return false; + Delegation delegation; + if (peerData) { + delegation = peerData->delegation; + pfrom->AddKnownProof(delegation.getProofId()); } + CHashWriter hasher(SER_GETHASH, 0); + hasher << delegation.getId(); + hasher << pfrom->GetLocalNonce(); + hasher << pfrom->nRemoteHostNonce; + hasher << pfrom->GetLocalExtraEntropy(); + hasher << pfrom->nRemoteExtraEntropy; + // Now let's sign! SchnorrSig sig; - - { - const uint256 hash = buildLocalSighash(pfrom); - - if (!sessionKey.SignSchnorr(hash, sig)) { - return false; - } + if (!sessionKey.SignSchnorr(hasher.GetHash(), sig)) { + return false; } - connman->PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()) - .Make(NetMsgType::AVAHELLO, - Hello(peerData->delegation, sig))); - - pfrom->AddKnownProof(peerData->delegation.getProofId()); + connman->PushMessage( + pfrom, CNetMsgMaker(pfrom->GetCommonVersion()) + .Make(NetMsgType::AVAHELLO, Hello(delegation, sig))); return true; } @@ -916,16 +917,6 @@ return invs; } -uint256 Processor::buildLocalSighash(CNode *pfrom) const { - CHashWriter hasher(SER_GETHASH, 0); - hasher << peerData->delegation.getId(); - hasher << pfrom->GetLocalNonce(); - hasher << pfrom->nRemoteHostNonce; - hasher << pfrom->GetLocalExtraEntropy(); - hasher << pfrom->nRemoteExtraEntropy; - return hasher.GetHash(); -} - bool Processor::isWorthPolling(const CBlockIndex *pindex) const { AssertLockHeld(cs_main); diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3731,15 +3731,16 @@ pfrom.GetId()); auto localProof = g_avalanche->getLocalProof(); - // If we sent a hello message, we should have a proof - assert(localProof); - // Add our proof id to the list or the recently announced proof - // INVs to this peer. This is used for filtering which INV can - // be requested for download. - LOCK(cs_main); - State(pfrom.GetId()) - ->m_recently_announced_proofs.insert(localProof->getId()); + if (localProof) { + // Add our proof id to the list or the recently announced + // proof INVs to this peer. This is used for filtering which + // INV can be requested for download. + LOCK(cs_main); + State(pfrom.GetId()) + ->m_recently_announced_proofs.insert( + localProof->getId()); + } } // Send getavaaddr and getavaproofs to our avalanche outbound or @@ -4930,6 +4931,12 @@ avalanche::Delegation delegation; vRecv >> delegation; + // A delegation with an all zero limited id indicates that the peer has + // no proof, so we're done. + if (delegation.getLimitedProofId() == uint256::ZERO) { + return; + } + avalanche::DelegationState state; CPubKey &pubkey = pfrom.m_avalanche_state->pubkey; if (!delegation.verify(state, pubkey)) { diff --git a/test/functional/abc_p2p_avalanche_peer_discovery.py b/test/functional/abc_p2p_avalanche_peer_discovery.py --- a/test/functional/abc_p2p_avalanche_peer_discovery.py +++ b/test/functional/abc_p2p_avalanche_peer_discovery.py @@ -12,6 +12,7 @@ from test_framework.address import ADDRESS_ECREG_UNSPENDABLE from test_framework.avatools import ( + AvaP2PInterface, avalanche_proof_from_hex, create_coinbase_stakes, gen_proof, @@ -22,6 +23,7 @@ from test_framework.key import ECKey, ECPubKey from test_framework.messages import ( MSG_AVA_PROOF, + MSG_TYPE_MASK, NODE_AVALANCHE, NODE_NETWORK, CInv, @@ -35,6 +37,16 @@ UNCONDITIONAL_RELAY_DELAY = 2 * 60 +class GetProofDataCountingInterface(AvaP2PInterface): + def __init__(self): + self.get_proof_data_count = 0 + super().__init__() + + def on_getdata(self, message): + if message.inv.type & MSG_TYPE_MASK == MSG_AVA_PROOF: + self.get_proof_data_count += 1 + + class AvalancheTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -69,6 +81,32 @@ proof = node.buildavalancheproof( proof_sequence, proof_expiration, wif_privkey, stakes) + self.log.info("Test the avahello signature with no proof") + + peer = get_ava_p2p_interface(node) + avahello = peer.wait_for_avahello().hello + assert_equal(avahello.delegation.limited_proofid, 0) + + self.log.info( + "A delegation with all zero limited id indicates that the peer has no proof") + + no_proof_peer = GetProofDataCountingInterface() + node.add_p2p_connection(no_proof_peer) + + master_key = ECKey() + master_key.generate() + null_delegation = node.delegateavalancheproof( + f"{0:0{64}x}", + bytes_to_wif(privkey.get_bytes()), + master_key.get_pubkey().get_bytes().hex(), + ) + no_proof_peer.send_avahello(null_delegation, privkey) + no_proof_peer.sync_send_with_ping() + + # No proof is requested + with p2p_lock: + assert_equal(no_proof_peer.get_proof_data_count, 0) + # Restart the node self.restart_node(0, self.extra_args[0] + [ "-avaproof={}".format(proof), @@ -99,8 +137,6 @@ "-avamasterkey=cND2ZvtabDbJ1gucx9GWH6XT9kgTAqfb6cotPt5Q5CyxVDhid2EN" ]) - master_key = ECKey() - master_key.generate() limited_id = avalanche_proof_from_hex(proof).limited_proofid delegation = node.delegateavalancheproof( f"{limited_id:0{64}x}",