Skip to content

Commit

Permalink
Dandelion fixes
Browse files Browse the repository at this point in the history
- expiration wasn't handled correctly
- objects with no child stems never expired. While this is better for
  anonymity, it can cause objects getting stuck
- upon expiration the nodes weren't marked as not having the object, causing it
  to not be advertised
- I haven't tested it but at least I don't think can make things worse
  • Loading branch information
PeterSurda committed Jul 11, 2019
1 parent e07cd14 commit 465a276
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/network/dandelion.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,13 @@ def getNodeStem(self, node=None):
def expire(self):
with self.lock:
deadline = time()
# only expire those that have a child node, i.e. those without a child not will stick around
toDelete = [[v.stream, k, v.child] for k, v in self.hashMap.iteritems() if v.timeout < deadline and v.child]
toDelete = [[v.stream, k, v.child]
for k, v in self.hashMap.iteritems()
if v.timeout < deadline]
for row in toDelete:
self.removeHash(row[1], 'expiration')
invQueue.put((row[0], row[1], row[2]))
return toDelete

def reRandomiseStems(self):
with self.lock:
Expand Down
28 changes: 25 additions & 3 deletions src/network/invthread.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,27 @@
import protocol
import state


def handleExpiredDandelion(expired):
"""For expired dandelion objects, mark all remotes as not having
the object"""
if not expired:
return
for i in \
BMConnectionPool().inboundConnections.values() + \
BMConnectionPool().outboundConnections.values():
if not i.fullyEstablished:
continue
for x in expired:
streamNumber, hashid, _ = x
try:
del i.objectsNewToMe[hashid]
except KeyError:
if streamNumber in i.streams:
with i.objectsNewToThemLock:
i.objectsNewToThem[hashid] = time()


class InvThread(threading.Thread, StoppableThread):
def __init__(self):
threading.Thread.__init__(self, name="InvBroadcaster")
Expand All @@ -20,8 +41,9 @@ def __init__(self):

def handleLocallyGenerated(self, stream, hashId):
Dandelion().addHash(hashId, stream=stream)
for connection in BMConnectionPool().inboundConnections.values() + \
BMConnectionPool().outboundConnections.values():
for connection in \
BMConnectionPool().inboundConnections.values() + \
BMConnectionPool().outboundConnections.values():
if state.dandelion and connection != Dandelion().objectChildStem(hashId):
continue
connection.objectsNewToThem[hashId] = time()
Expand All @@ -31,7 +53,7 @@ def run(self):
chunk = []
while True:
# Dandelion fluff trigger by expiration
Dandelion().expire()
handleExpiredDandelion(Dandelion().expire())
try:
data = invQueue.get(False)
chunk.append((data[0], data[1]))
Expand Down

2 comments on commit 465a276

@rbrunner7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, seems to work!

I tested with submitting and fetching messages through the API, with Monero's MMS. Yesterday and today I had two test runs with plain release 0.6.3.2 i.e. without this code here, and both ran into problems after about half an hour, with message transfer times suddenly going up to several minutes. (Earlier tests showed that messages can probably get stuck indefinitely eventually.)

One more test with this code active had no such problem anymore, even after 1 hour.

@PeterSurda
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbrunner7 Thank you for your report.

Please sign in to comment.