Overhaul core Tracker: review announce handler (peer mutability) #1276
Description
In the tracer core the announce handler requires the peer argument to be mutable. That's because the peer IP might change because the tracker ignores the IP in the request and assign the IP from the connection.
Some time ago I wrote a comment to review that. I'm considering making the peer immutable and returning the assigned peer IP. The function I want to change:
pub async fn announce(
&self,
info_hash: &InfoHash,
peer: &mut peer::Peer,
remote_client_ip: &IpAddr,
peers_wanted: &PeersWanted,
) -> Result<AnnounceData, AnnounceError> {
// code-review: maybe instead of mutating the peer we could just return
// a tuple with the new peer and the announce data: (Peer, AnnounceData).
// It could even be a different struct: `StoredPeer` or `PublicPeer`.
self.whitelist_authorization.authorize(info_hash).await?;
tracing::debug!("Before: {peer:?}");
peer.change_ip(&assign_ip_address_to_peer(remote_client_ip, self.config.net.external_ip));
tracing::debug!("After: {peer:?}");
let stats = self.upsert_peer_and_get_stats(info_hash, peer);
let peers = self
.in_memory_torrent_repository
.get_peers_for(info_hash, peer, peers_wanted.limit());
Ok(AnnounceData {
peers,
stats,
policy: self.config.announce_policy,
})
}The new signature:
pub async fn announce(
&self,
info_hash: &InfoHash,
peer: &peer::Peer,
remote_client_ip: &IpAddr,
peers_wanted: &PeersWanted,
) -> Result<(AnnounceData, IpAddr), AnnounceError> {}I think (I have to confirm it) I made it mutable to return the final IP because it's used for testing purposes.
What do you think @da2ce7?