Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@tomaka
Copy link
Contributor

@tomaka tomaka commented Aug 7, 2018

  • Removed all the unused fields from NetworkConfiguration. I removed the fields that were neither set by the CLI nor implemented by libp2p. Re-adding some fields (such as max_handshakes) can easily be done at the time we implement them.
  • The listen_address and public_address are now a Multiaddr instead of a SocketAddr. This doesn't change much in practice for now.
  • Removed the IpFilter and NetworkIoMessage types and the dependency on the ipnetwork crate.

@tomaka tomaka added A0-please_review Pull request needs code review. M4-core labels Aug 7, 2018
}
match swarm_controller.listen_on(shared.config.listen_address.clone()) {
Ok(new_addr) => {
debug!(target: "sub-libp2p", "Libp2p listening on {}", new_addr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that we print new_addr instead of listen_address.
The only practical difference is that if the user specifies port 0, then new_addr will contain the actual port we're listening on, and not 0.

Copy link
Contributor

@pepyakin pepyakin Aug 7, 2018

Choose a reason for hiding this comment

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

what would happen if there is something already listening on the specified multiaddr?

@gavofyork gavofyork added A8-looksgood and removed A0-please_review Pull request needs code review. labels Aug 7, 2018
}

/// Create new default configuration for localhost-only connection with random port (usefull for testing)
/// Create new default configuration for localhost-only connection with random port (useful for testing)
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, help me understand, I thought that AddrComponent::TCP(30333) should assign the specific port. Why this saying "with random port" then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I screwed up.

@arkpar
Copy link
Member

arkpar commented Aug 8, 2018

Filter functionality is quite useful in parity-ethereum. Is there anything to replace it in libp2p?

@tomaka
Copy link
Contributor Author

tomaka commented Aug 8, 2018

Filter functionality is quite useful in parity-ethereum. Is there anything to replace it in libp2p?

I removed it in this PR as it was neither implemented nor used, but it shouldn't be hard to actually implement.

@gavofyork gavofyork merged commit 59225c6 into paritytech:master Aug 8, 2018
@tomaka tomaka deleted the net-cleanups branch August 8, 2018 18:09
dvdplm added a commit that referenced this pull request Aug 9, 2018
* master:
  README: fixed typo in docker run command (#518)
  Merge *_at methods. (#515)
  New flags to listen to all interfaces (#495)
  If contract reaches max depth, return Err (#503)
  Some networking cleanups (#504)
  Derivable Encode & Decode (#509)
  substrate: return Option in all storage related RPC methods (#510)
  Build with locked Cargo.lock on CI (#514)
  Place call data into a newly allocated pages (#502)
gavofyork pushed a commit that referenced this pull request Aug 10, 2018
* Some networking cleanups

* Fix tests

* Fix wrong port in new_local
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
* Update cargo files

* First round of build fixes

* update lock file
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
liuchengxu pushed a commit to chainx-org/substrate that referenced this pull request Aug 23, 2021
liuchengxu pushed a commit to autonomys/substrate that referenced this pull request Jun 3, 2022
…age-access

Control non-root storage access
helin6 pushed a commit to boolnetwork/substrate that referenced this pull request Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants