Skip to content

mass gx update#5637

Merged
Stebalien merged 6 commits intomasterfrom
feat/pubsub-rename
Oct 25, 2018
Merged

mass gx update#5637
Stebalien merged 6 commits intomasterfrom
feat/pubsub-rename

Conversation

@Stebalien
Copy link
Copy Markdown
Member

@Stebalien Stebalien commented Oct 24, 2018 •
edited
Loading

  • pubsub rename
  • yamux race fixes
  • libp2p no longer disconnects on version mismatch
  • multiaddr changes (reduced allocation)
  • every other change that has happened recently.

@Stebalien Stebalien requested a review from Kubuxu as a code owner October 24, 2018 16:37
@ghost ghost assigned Stebalien Oct 24, 2018
@ghost ghost added the status/in-progress In progress label Oct 24, 2018
@Stebalien
Copy link
Copy Markdown
Member Author

Stebalien commented Oct 24, 2018

Yeah, I'm just going to piggy back a massive gx update on this.

@Stebalien
update for pubsub rename
efa282d 
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
gx update
c97c345 
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
switch to new raceless routing event interface
e41ac96 
fixes #5616

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
Copy link
Copy Markdown
Member Author

Stebalien commented Oct 24, 2018

Ok, so, I'm getting a really annoying test failure introduced by ipfs/go-bitswap#15. Specifically, ToNetV1 is returning stream reset.

  • A single large write succeeds.
  • A small write followed by a large write succeeds. This is what we used to do (write a few bytes for the length, write everything else).
  • Any write at least (1024 * 4 - 40) followed by a second write fails (well, I think it's the second that's failing). That 40 looks suspiciously like 32 + 8.

Stebalien added a commit to libp2p/go-libp2p that referenced this pull request Oct 24, 2018
@Stebalien
fix a data-race in the mock net
841a627 
This was causing ipfs/kubo#5637 (comment)
@Stebalien Stebalien mentioned this pull request Oct 24, 2018
Merged
@Stebalien
Copy link
Copy Markdown
Member Author

Stebalien commented Oct 24, 2018

When in doubt, it's a data-race: libp2p/go-libp2p#464

@Stebalien
Copy link
Copy Markdown
Member Author

Stebalien commented Oct 24, 2018

(ok, not the only issue...)

@Stebalien
Copy link
Copy Markdown
Member Author

Stebalien commented Oct 24, 2018

No, that was the issue. I just failed at fixing it.

@Stebalien
update for changed relay defaults
4d956d2 
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
gx update go-libp2p
636bbc7 
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
gx: update yamux
e35d4ea 
(fixes a panic due to a race)

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien changed the title update for pubsub rename mass gx update Oct 24, 2018
kevina
kevina reviewed Oct 24, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@kevina kevina left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I am not familiar with a lot of the code so I have one question, but otherwise this LGTM.

core/core.go
// Enabled by default.
libp2pOpts = append(libp2pOpts, libp2p.DisableRelay())
} else {
relayOpts := []circuit.RelayOpt{circuit.OptDiscovery}
Copy link
Copy Markdown
Contributor

@kevina kevina Oct 24, 2018

Choose a reason for hiding this comment

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

What's the justification for always including circuit.OptDiscovery in relayOpts?

Copy link
Copy Markdown
Member Author

@Stebalien Stebalien Oct 25, 2018

Choose a reason for hiding this comment

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

Without this, p2p circuit addresses that don't explicitly specify the relay won't work. When turning on the relay transport by default in the libp2p constructor, we added this option (the feature was already there) and turned it off by default. However, to maintain functionality (and keep the tests passing) I had to turn it back on in go-ipfs.

@Stebalien Stebalien merged commit 0bd181c into master Oct 25, 2018
@ghost ghost removed the status/in-progress In progress label Oct 25, 2018
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@Stebalien
Merge pull request ipfs/kubo#5637 from ipfs/feat/pubsub-rename
75dbb93 
mass gx update

This commit was moved from ipfs/kubo@0bd181c
@hacdias hacdias deleted the feat/pubsub-rename branch May 9, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

1 more reviewer

@kevina kevina kevina left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Stebalien Stebalien

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Stebalien @kevina