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

Changed TrustChain so it's a single chain instead of multiple fragmented communities #3572

Merged
merged 9 commits into from
Jun 25, 2018

Conversation

devos50
Copy link
Contributor

@devos50 devos50 commented Apr 7, 2018

In this PR, I refactored the communities that were subclassing the TrustChain overlay. Instead, we have a single ledger now and we create separate overlays and communicate with the TrustChain ledger instead. See the IPv8 pull request for more information.

I also updated the master peers of the market + tunnel community. I promise this will be the last time :). Finally, I fixed a bug during the tests where IPv8 would talk with the real world.

Requires Tribler/py-ipv8#96 to be merged first.

I've written a few experiments to verify that everything works as expected with this PR:

@devos50 devos50 changed the title WIP: Changed TrustChain so it's a single chain instead of multiple fragmented communities READY: Changed TrustChain so it's a single chain instead of multiple fragmented communities Apr 7, 2018
@devos50 devos50 requested review from qstokkink and xoriole and removed request for qstokkink April 7, 2018 08:59
@devos50 devos50 force-pushed the one_chain branch 2 times, most recently from 12b76bc to a2da438 Compare April 7, 2018 11:50
@devos50 devos50 force-pushed the one_chain branch 3 times, most recently from 6eb7e76 to 1aa3178 Compare April 7, 2018 13:36
.gitmodules Outdated
@@ -9,4 +9,4 @@
url = https://github.com/spesmilo/electrum
[submodule "py-ipv8"]
path = Tribler/pyipv8
url = https://github.com/Tribler/py-ipv8.git
url = https://github.com/devos50/py-ipv8.git
Copy link
Contributor

Choose a reason for hiding this comment

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

❗️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know and this will be removed after the review and when the IPv8 PR is merged.

xoriole
xoriole previously approved these changes Apr 9, 2018
@devos50 devos50 changed the title READY: Changed TrustChain so it's a single chain instead of multiple fragmented communities ON HOLD: Changed TrustChain so it's a single chain instead of multiple fragmented communities May 22, 2018
@devos50 devos50 changed the title ON HOLD: Changed TrustChain so it's a single chain instead of multiple fragmented communities WIP: Changed TrustChain so it's a single chain instead of multiple fragmented communities Jun 12, 2018
@devos50 devos50 force-pushed the one_chain branch 6 times, most recently from 55f5cf9 to d0dcc09 Compare June 13, 2018 11:20
@devos50
Copy link
Contributor Author

devos50 commented Jun 13, 2018

retest this please

@devos50 devos50 changed the title WIP: Changed TrustChain so it's a single chain instead of multiple fragmented communities READY: Changed TrustChain so it's a single chain instead of multiple fragmented communities Jun 13, 2018
@@ -40,13 +34,19 @@
STATE_START_TORRENT_CHECKER, STATE_START_REMOTE_TORRENT_HANDLER,
STATE_START_API_ENDPOINTS, STATE_START_WATCH_FOLDER, STATE_START_CREDIT_MINING)
from Tribler.dispersy.util import blockingCallFromThread, blocking_call_on_reactor_thread
from Tribler.pyipv8.ipv8.attestation.trustchain.community import TrustChainCommunity
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be imported when loading the community.

if block:
return succeed(block)

monitor_lc = LoopingCall(check_has_block)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a TaskManager managed task and can leak.

@@ -271,7 +273,7 @@ def update_torrent(self, peers, handle, download):
def get_peer_from_address(self, address):
circuit_peer = None
for peer in self.get_peers():
if peer.address == address:
if peer.address == address and peer.public_key.key_to_bin().startswith("LibNaCLPK:"):
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this now, fixed in IPv8.

xoriole
xoriole previously approved these changes Jun 25, 2018
ichorid
ichorid previously approved these changes Jun 25, 2018
Copy link
Contributor

@ichorid ichorid left a comment

Choose a reason for hiding this comment

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

Everything is fine!

global_time = self.claim_global_time()
dist = GlobalTimeDistributionPayload(global_time).to_pack_list()
payload = HalfBlockPairPayload.from_half_blocks(block, linked_block).to_pack_list()
packet = self._ez_pack(self._prefix, 22, [dist, payload], False)
Copy link
Contributor

Choose a reason for hiding this comment

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

22 = Magic number ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is a convention that I followed from the Dispersy era. I will use some defined constants in a subsequent PR 👍

ichorid
ichorid previously approved these changes Jun 25, 2018
@devos50 devos50 changed the title READY: Changed TrustChain so it's a single chain instead of multiple fragmented communities Changed TrustChain so it's a single chain instead of multiple fragmented communities Jun 25, 2018
@devos50 devos50 merged commit 07892bf into Tribler:next Jun 25, 2018
@devos50 devos50 deleted the one_chain branch June 25, 2018 14:45
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.

5 participants