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

Simplify trait bounds in network to prepare for collator-rpc #12082

Merged
merged 37 commits into from
Aug 31, 2022

Conversation

skunert
Copy link
Contributor

@skunert skunert commented Aug 22, 2022

This is a preparation PR for paritytech/cumulus#989.

Why is this needed?

We will introduce a minimal relay chain node in cumulus, which only runs a subset
of all Polkadot subsystems and also only required network functionality.

This minimal node will connect to the relay chain via RPC and fetch the required information.
This PR refactors some issues in substrate to make implementing this RPC client easier, behaviour should not change.

Change Summary

I try to remove as many bounds as possible from the client instance that goes into the network.

  • ProofProvider was already unused since the sync refactoring
  • HeaderMetadata was already unused since the sync refactoring
  • BlockBackend is used in the Bitswap protocol. The protocol was already optional but required the BlockBackend bound. I introduce new abstractions, and Bitswap can now be passed into the network via parameters.
  • Create an async wrapper for AuthorityDiscovery, to allow custom implementations that do async network calls
  • Introduce NetworkHeaderBackend that accepts only Hash instead of BlockId, which resembles our RPC interface more

@skunert skunert added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Aug 22, 2022
@skunert skunert requested a review from a team August 22, 2022 13:04
@skunert skunert added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Aug 22, 2022
client/network/src/bitswap.rs Outdated Show resolved Hide resolved
client/network/common/src/header_backend.rs Outdated Show resolved Hide resolved
@skunert skunert merged commit 7c1a39f into paritytech:master Aug 31, 2022
@sinkingsugar
Copy link

sinkingsugar commented Dec 6, 2022

Fyi ipfs clients don't seem to receive responses from substate bitswap anymore after this PR...
(We are digging on it right now on our side..)

Edit: Or rather this PR #12242 finishing the job

ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…ech#12082)

* Hack towards PoC

* Abstract away runtime requirement

* blockchainevents

* Remove bitswap

* Remove unused sync more

* Remove unused features in network

* Re-enable bitswap change

* Remove `Chain` trait bound

* Reimplement blockchain-rpc-events

* Move network to cumulus

* Make AuthorityDiscovery async

* Remove `ProofProvider` requirement from network behaviour

* Extract bitswap

* Adjustments after merge

* Remove HeaderMetadata trait from network

* Introduce NetworkHeaderBackend

* Add comments

* Improve comments

* Move NetworkHeaderBackend to new module

* Improve naming, remove redundand send + sync

* Clean up generics

* Fix CI

* Improve comment and readability

* Remove NetworkHeaderBackend

* Fix Cargo.lock

Co-authored-by: Sebastian Kunert <skunert@Sebastians-MacBook-Pro.fritz.box>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
Status: done
Development

Successfully merging this pull request may close these issues.

4 participants