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

Removal of light client from substrate #9684

Merged
merged 17 commits into from
Oct 30, 2021
Merged

Removal of light client from substrate #9684

merged 17 commits into from
Oct 30, 2021

Conversation

gilescope
Copy link
Contributor

@gilescope gilescope commented Sep 3, 2021 •
edited
Loading

As discussed in the retreat we are simplifying before refactoring by not shipping a light client out of the box with substrate. (Current users should look towards https://github.com/paritytech/substrate-connect as a purpose built solution to this usecase)

The client network tests that support light client requests use a full light client at the moment which is why more is not initially removed.

polkadot companion: paritytech/polkadot#4105

skip check-dependent-cumulus

@gilescope gilescope added A3-in_progress Pull request is in progress. No review needed at this stage. B5-clientnoteworthy C3-medium PR touches the given topic and has a medium impact on builders. D2-breaksapi A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Sep 3, 2021
@gilescope gilescope changed the title WIP: Removal of light client from substrate Removal of light client from substrate Sep 7, 2021
@bkchr
Copy link
Member

bkchr commented Oct 6, 2021

Status?

@gilescope
Copy link
Contributor Author

sorry missed that comment. will bring it up to date and see what's left.

@arkpar
Copy link
Member

arkpar commented Oct 12, 2021 •
edited
Loading

There's a lot of stuff that may be removed from client/db as well. Also I guess we can delete everything related to CHTs as well.

@gilescope gilescope added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Oct 12, 2021
@arkpar
Copy link
Member

arkpar commented Oct 19, 2021

Looks good. I'll make a PR to cleanup client/db and related crates later.

@gilescope
Copy link
Contributor Author

@arkpar the problem I have removing the light client db is that the networking tests test light client rpc functionality and the 'test' light client uses the light client db at the moment.

@gilescope gilescope mentioned this pull request Oct 19, 2021
Merged
@gilescope
Copy link
Contributor Author

skip check-dependent-cumulus

1 similar comment
@TriplEight
Copy link
Contributor

skip check-dependent-cumulus

bkchr
bkchr suggested changes Oct 21, 2021
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

The sc-light client wasn't removed?

@bkchr
Copy link
Member

bkchr commented Oct 21, 2021

@arkpar the problem I have removing the light client db is that the networking tests test light client rpc functionality and the 'test' light client uses the light client db at the moment.

Just have read this. The task of this pr is to remove this stuff. I mean these light client tests make sense and should stay, but then we need a refactoring or whatever. However, most/all of the crate should be removed.

@gilescope
Copy link
Contributor Author

ok will stop there and let @arkpar merge his stuff in as a separate pr.

gilescope and others added 4 commits October 22, 2021 13:39
@gilescope @bkchr
Update client/service/test/src/lib.rs
48f05af 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@gilescope @bkchr
Update client/service/test/src/lib.rs
25440c2 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@gilescope
cargo fmt
03d689f
@gilescope
Merge branch 'master' into giles-remove-light-client
2937c97
@gilescope
Copy link
Contributor Author

/cla run-cla-check

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Oct 22, 2021

Queueing command execution: run-cla-check

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Oct 22, 2021

@gilescope Command execution has finished.

@gilescope
Copy link
Contributor Author

bot merge

@paritytech-processbot
Copy link

Error:
Error: When trying to meet the "Project Owners" approval requirements: this pull request does not belong to a project defined in Process.json.

Approval by "Project Owners" is only attempted if other means defined in the criteria for merge are not satisfied first.

The following errors might have affected the outcome of this attempt:

  • 'Batch: Availability and Validity' does not match any projects in polkadot's Process.json
  • 'Batch: Codebase Restructure' does not match any projects in polkadot's Process.json

Merge failed. Check out the criteria for merge. If you're not meeting the approval count, check if the approvers are members of substrateteamleads or core-devs.

@gilescope
Copy link
Contributor Author

ah I see theres changes requested on the polkadot companion.

@bkchr
Copy link
Member

bkchr commented Oct 22, 2021

Don't merge. The companion isn't ready yet.

@nuke-web3 nuke-web3 mentioned this pull request Oct 22, 2021
Closed
@gilescope gilescope added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Oct 26, 2021
@gilescope
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 1d818f3 into master Oct 30, 2021
@paritytech-processbot paritytech-processbot bot deleted the giles-remove-light-client branch October 30, 2021 12:38
@gilescope gilescope mentioned this pull request Nov 8, 2021
Merged
@github-actions github-actions bot mentioned this pull request Dec 2, 2021
Closed
@ghzlatarev ghzlatarev mentioned this pull request Dec 6, 2021
Merged
23 tasks
@sander2 sander2 mentioned this pull request Jan 6, 2022
Merged
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
@gilescope @bkchr @grishasobol
Removal of light client from substrate (paritytech#9684)
9374cac 
* Removal of light client from substrate

* add missing import

* These tests relate to there being light and non light clients.

* removing lightnodes from test

* cargo fmt

* not needed

* LightDataChecker not needed any longer

* cargo fmt

* Update client/service/test/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/service/test/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* cargo fmt

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
aurexav added a commit to darwinia-network/darwinia-common that referenced this pull request Aug 4, 2022
@aurexav
Companion of paritytech/substrate#9684, !
97c37dd
@gilescope gilescope mentioned this pull request Sep 29, 2022
Merged
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@gilescope @bkchr @ark0f
Removal of light client from substrate (paritytech#9684)
cd05554 
* Removal of light client from substrate

* add missing import

* These tests relate to there being light and non light clients.

* removing lightnodes from test

* cargo fmt

* not needed

* LightDataChecker not needed any longer

* cargo fmt

* Update client/service/test/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/service/test/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* cargo fmt

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@arkpar arkpar arkpar approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C3-medium PR touches the given topic and has a medium impact on builders. D2-breaksapi
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gilescope @bkchr @arkpar @TriplEight