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

Update libp2p, async-std, and other deps #922

Merged
merged 7 commits into from
Jan 14, 2021
Merged

Update libp2p, async-std, and other deps #922

merged 7 commits into from
Jan 14, 2021

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Jan 9, 2021

Summary of changes
Changes introduced in this pull request:

  • Migrates to new error prone async-std channels (0.1.8)
  • Libp2p update to 0.28
    • Updated bitswap dep and reinitialized request-response (I kept their oneshot channels)

Reference issue to close (if applicable)

Closes

Other information and links

@austinabell
Copy link
Contributor Author

Having issues with libp2p connections timing out with this new version, going to let this sit for a bit to debug. Could also just remove that version bump from this PR and revisit later, undecided.

@ec2
Copy link
Member

ec2 commented Jan 11, 2021

Interesting... When does it actually time out? When its trying to connect? Or when you try to send some RPC(BlockSync) req?

@austinabell
Copy link
Contributor Author

Interesting... When does it actually time out? When its trying to connect? Or when you try to send some RPC(BlockSync) req?

When dialing trying to send rpc requests

@austinabell
Copy link
Contributor Author

austinabell commented Jan 11, 2021

I think the issue is actually because libp2p/rust-libp2p#1867 dropped support for one-way requests (we are not responding to Hello currently) and possibly this is blocking us from being able to send chain exchange requests to that same peer.

I'm going to investigate a bit more but I'm just leaning toward updating our hello response (was going to be done later anyway) and following this pattern, so we are available for future updates of the upstream request-response.

I'm not sure how this translates with go-interop, since I don't believe they follow the same pattern. But potentially the connection closed would then translate to the ResponseOmission variant

@austinabell
Copy link
Contributor Author

Going to just leave it as 0.28 update for now and open a draft PR for 0.34 changes (it's currently broken running against the go implementation)

let mplex_config = {
let mut mplex_config = mplex::MplexConfig::new();
mplex_config.max_buffer_len(usize::MAX);
// mplex_config.set_max_buffer_size(usize::MAX);
Copy link
Member

Choose a reason for hiding this comment

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

Did you forget to remove this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ya, it was what the type changed in future libp2p version, I commented for easier switching, but #928 exists

/// Keeps track of Chain exchange requests to responses
#[behaviour(ignore)]
cx_request_table: HashMap<RequestId, OneShotSender<ChainExchangeResponse>>,
/// Boxed futures of responses for the
Copy link
Member

Choose a reason for hiding this comment

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

for the.....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

haha

@austinabell austinabell merged commit 2f87782 into main Jan 14, 2021
@austinabell austinabell deleted the austin/depupd branch January 14, 2021 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants