Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Set up basic core package #8

Merged
merged 9 commits into from
Apr 10, 2019
Merged

Set up basic core package #8

merged 9 commits into from
Apr 10, 2019

Conversation

albrow
Copy link
Contributor

@albrow albrow commented Apr 5, 2019

Most methods are stubbed for now. This just shows the interface and allows us to write some basic tests for core.Node which make use of the webrtc-direct transport.

Go tests actually pass, but the race checker is being triggered by some low-level libp2p code. Will need to investigate further.

Wasm tests fail due to a timeout when trying to connect. I think this may have to do with incorrect/unusable port numbers being selected by Node.js.

@albrow albrow requested a review from fabioberger April 5, 2019 00:14
fabioberger
fabioberger reviewed Apr 5, 2019
View reviewed changes
core/core.go Outdated Show resolved Hide resolved
@albrow
Copy link
Contributor Author

albrow commented Apr 5, 2019

Fixed race condition via libp2p/go-mplex#41.

Node.js/WebAssembly tests are still failing so I'll look into that next.

@albrow
Copy link
Contributor Author

albrow commented Apr 5, 2019

Now CI is running into issues with other dependencies (looks like commit hashes that no longer exist). Pion is in the middle of renaming some things and gearing up for the release of v2. It'll probably get sorted out soon.

In the meantime I can keep working on the failing Node.js/WebAssembly tests.

@albrow
Copy link
Contributor Author

albrow commented Apr 8, 2019

IMO the code is complete enough for the first implementation of the core package. We're still missing a few things like the database storage layer and peer discovery, but a lot of this depends on other work that has yet to be done.

Still WIP because of upstream dependency issues. The build currently fails with:

verifying github.com/ipfs/[email protected]/go.mod: checksum mismatch
	downloaded: h1:d4KVXhMt913cLBEI/PXAy6ko+W7e9AhyAKBGh803qeE=
	go.sum:     h1:bYmHO9fuKO1Ca7dpdDBWQl0mndy5b0HFqSJjGlNYtzs=

As well as some other errors that look like:

go: github.com/pions/[email protected]: unknown revision 9276b506f1f3

@fabioberger I don't think the code will change much if you want to go ahead and review.

fabioberger
fabioberger suggested changes Apr 9, 2019
View reviewed changes
core/core.go Show resolved Hide resolved
core/core.go Outdated Show resolved Hide resolved
@albrow albrow requested a review from fabioberger April 9, 2019 23:02
@albrow albrow changed the title WIP: Set up basic core package Set up basic core package Apr 9, 2019
fabioberger
fabioberger approved these changes Apr 9, 2019
View reviewed changes
core/core.go
// TODO(albrow): We should be closing the host here. Unfortunately there is a
// bug where the transport panics with "panic: close of closed channel". Needs
// to be fixed upstream.
// return n.host.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a link to the issue/PR?

Copy link
Contributor Author

@albrow albrow Apr 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Doesn't exist yet. I think it's faster to talk to the libp2p/go-libp2p-webrtc-direct maintainer directly.

Copy link
Contributor Author

@albrow albrow Apr 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Actually changed my mind. I might open an issue but for the moment development on the transport is blocked by pion/sctp#32 so it will be difficult to reproduce the error. See libp2p/go-libp2p-webrtc-direct#29 for some more context. I'll update this thread or mention it in Slack once an issue or PR is out.

@albrow albrow merged commit 37808b5 into master Apr 10, 2019
@albrow albrow deleted the feature/core branch April 10, 2019 00:24
@albrow
Copy link
Contributor Author

albrow commented Apr 10, 2019

Shoot. CI just failed on master (timed out) after this PR was merged. I suspect it may have to do with the same performance problems described in pion/sctp#32. Gonna revert for now.

@albrow albrow mentioned this pull request Apr 10, 2019
Merged
@albrow albrow restored the feature/core branch April 10, 2019 00:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fabioberger fabioberger fabioberger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@albrow @fabioberger