Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added delay when removing tunnel info, fixed mid encoding #67

Merged
merged 3 commits into from
Mar 4, 2018

Conversation

devos50
Copy link
Contributor

@devos50 devos50 commented Mar 4, 2018

In this PR, I've added an optional delay when removing/destroying circuits, relays and exit sockets. This allows some post-mortem data to flow over the circuit, like payout messages in Tribler.

I've also addressed mid encoding in the Tunnel object. The way we encode it is a common source of errors. We now always store the mid in binary. We should eventually fix it to store a Peer object instead but requires takes a bit more work.

@devos50 devos50 force-pushed the remove_hex_encode branch from 987b21c to 9a9b306 Compare March 4, 2018 12:27
@devos50 devos50 changed the title WIP: Removed mid hex encoding from tunnels READY: Added delay when removing tunnel info, fixed mid encoding Mar 4, 2018
@devos50 devos50 requested a review from qstokkink March 4, 2018 12:36
@qstokkink
Copy link
Collaborator

Question: what is the expected behavior on unload()?

@devos50
Copy link
Contributor Author

devos50 commented Mar 4, 2018

@qstokkink good question, I didn't consider the situation when unloading an overlay. In that case, we would like to remove the data immediately I guess? This could be realized by adding a remove_now keyword argument to remove_relay, remove_circuit etc and if it's true, we remove the data immediately. What do you think about this?

@qstokkink
Copy link
Collaborator

@devos50 considering the payout (actual money) happens after circuit destruction, I think the entire community should wait until circuits have been destructed and have been paid out. Although it would take up to the remove_tunnel_delay for the community to unload (which is a relatively long time).

However, considering this functionality supports your Market, I'm fine with whatever you think is best.

@devos50 devos50 force-pushed the remove_hex_encode branch 2 times, most recently from 3e618c1 to 8a7a899 Compare March 4, 2018 14:04
@devos50
Copy link
Contributor Author

devos50 commented Mar 4, 2018

@qstokkink I think that when a community unloads, it shouldn't wait for additional incoming messages. I know this might cause a payout not to complete but this is not the end of the world since at most 250MB is lost (the maximum amount of data that can go over a circuit).

I've updated my PR.

@qstokkink qstokkink changed the title READY: Added delay when removing tunnel info, fixed mid encoding Added delay when removing tunnel info, fixed mid encoding Mar 4, 2018
@qstokkink qstokkink merged commit 9e417ab into Tribler:master Mar 4, 2018
@devos50 devos50 deleted the remove_hex_encode branch March 4, 2018 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants