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

Move client consensus parts out of primitives and into client/consensus/api #9319

Merged
14 commits merged into from
Jul 30, 2021
Merged

Move client consensus parts out of primitives and into client/consensus/api #9319

14 commits merged into from
Jul 30, 2021

Conversation

gilescope
Copy link
Contributor

@gilescope gilescope commented Jul 9, 2021 •
edited
Loading

polkadot companion: paritytech/polkadot#3456

@gilescope gilescope added A3-in_progress Pull request is in progress. No review needed at this stage. B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. D2-breaksapi labels Jul 9, 2021
@gilescope
Copy link
Contributor Author

gilescope commented Jul 9, 2021 •
edited
Loading

Happy to bikeshead any name for BlockImportStatus except BlockImportResult - it's not a Result and calling it that just trips the reader up.

@gilescope gilescope added D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit 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 Jul 10, 2021
seunlanlege
seunlanlege approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@seunlanlege seunlanlege left a comment

Choose a reason for hiding this comment

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

lgtm

@andresilva
Copy link
Contributor

(please don't merge this without my review, which will come shortly)

@gilescope gilescope force-pushed the giles-split-consensus branch 2 times, most recently from 6458723 to 7b505bf Compare July 16, 2021 10:02
andresilva
andresilva suggested changes Jul 16, 2021
View reviewed changes
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

I don't think creating a new crate is necessary and I think you should be able to just use sc-consensus. Some of the formatting changes are bit unnecessary IMO, in particular in code that wasn't changed in any way. I think there's still more stuff that can be moved like SelectChain and BlockAnnounceValidator. Overall changes LGTM, just commented with some nits.

client/finality-grandpa/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/api/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
client/network/test/src/lib.rs Outdated Show resolved Hide resolved
client/network/test/src/lib.rs Outdated Show resolved Hide resolved
client/network/test/src/lib.rs Outdated Show resolved Hide resolved
primitives/consensus/common/src/import_queue.rs Outdated Show resolved Hide resolved
client/service/src/client/client.rs Outdated Show resolved Hide resolved
client/consensus/api/src/import_queue.rs Outdated Show resolved Hide resolved
@andresilva
Copy link
Contributor

Also needs a companion PR to polkadot, some imports there are broken now.

@gilescope
Copy link
Contributor Author

There is a companion PR: paritytech/polkadot#3456 (but it's my first time so do please let me know if anything is out of kilter).

@gilescope gilescope mentioned this pull request Jul 20, 2021
Merged
@gilescope
Copy link
Contributor Author

Sorry! Will stop rebasing and merge from master from now on.

@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 Jul 23, 2021
@gilescope gilescope requested a review from expenses July 27, 2021 07:50
expenses
expenses reviewed Jul 27, 2021
View reviewed changes
Copy link
Contributor

@expenses expenses left a comment

Choose a reason for hiding this comment

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

I think we might be able to do some import trimming here of sp-consensus and sc-consensus.

@expenses expenses self-requested a review July 28, 2021 11:29
expenses
expenses approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@expenses expenses left a comment

Choose a reason for hiding this comment

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

I think this probably good to go. Needs a few more reviews first though.

tomusdrw
tomusdrw approved these changes Jul 29, 2021
View reviewed changes
primitives/consensus/common/src/import_queue.rs Outdated Show resolved Hide resolved
client/consensus/common/src/lib.rs Outdated Show resolved Hide resolved
andresilva
andresilva approved these changes Jul 30, 2021
View reviewed changes
Copy link
Contributor

@andresilva andresilva left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for delay in review 🙏

docs/STYLE_GUIDE.md Outdated Show resolved Hide resolved
@gilescope @andresilva
Update docs/STYLE_GUIDE.md
1d93d35 
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
@gilescope
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Jul 30, 2021

Trying merge.

@ghost ghost merged commit 4d28ebe into paritytech:master Jul 30, 2021
ericjmiller pushed a commit to ericjmiller/substrate that referenced this pull request Aug 2, 2021
@gilescope @andresilva
Move client consensus parts out of primitives and into client/consens…
e7a8f7a 
…us/api (paritytech#9319)

* moved client code out of primitives

* bump ci

* Fixup from merge.

* Removed unused deps thanks to review feedback

* Removing unneeded deps

* updating lock file

* note about rustfmt

* fixed typo to bump ci

* Move lonely CacheKeyId to parent

* cargo fmt

* updating import style

* Update docs/STYLE_GUIDE.md

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
bkchr added a commit that referenced this pull request Aug 2, 2021
@ericjmiller @bkchr
@gavofyork @emostov @shawntabrizi @gilescope @andresilva @arkpar @TriplEight
#9386: copy rustfmt.toml to node-template (#9461)
d3b4830 
* copying rustfmt from root to node-template build.
Also, running rustfmt on this.

* Update .maintain/node-template-release/src/main.rs

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

* Add some important events to batch & staking to ensure ease of analysis (#9462)

* Add some important events to batch & staking to ensure ease of analysis

* Update frame/staking/src/pallet/mod.rs

Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>

* Update lib.rs

* fix test

Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

* Move client consensus parts out of primitives and into client/consensus/api (#9319)

* moved client code out of primitives

* bump ci

* Fixup from merge.

* Removed unused deps thanks to review feedback

* Removing unneeded deps

* updating lock file

* note about rustfmt

* fixed typo to bump ci

* Move lonely CacheKeyId to parent

* cargo fmt

* updating import style

* Update docs/STYLE_GUIDE.md

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

* Refactor Benchmarks for Less Wasm Memory Usage (#9373)

* extract repeat out of benchmark

* remove r

* unused

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* use linked map to keep order

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Delete pallet_balances.rs

* Delete out

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* steps and repeat to tuple (current_*, total_*)

* idea for list command

* fmt

* use benchmark list in cli

* handle steps in cli

* move log update to cli

* fmt

* remove old todo

* line width

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* benchmark metadata function

* don't need this warm up

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* fix warnings

* fix node-template

* fix

* fmt

* line width

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* improve docs

* improve cli

* fix format

* fix bug?

* Revert "fix bug?"

This reverts commit 8051bf1.

* skip frame-metadata

* extract repeat out of benchmark

* remove r

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* use linked map to keep order

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Delete pallet_balances.rs

* Delete out

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* steps and repeat to tuple (current_*, total_*)

* idea for list command

* fmt

* use benchmark list in cli

* handle steps in cli

* move log update to cli

* remove old todo

* line width

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* benchmark metadata function

* don't need this warm up

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_balances --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/balances/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* fix warnings

* fix node-template

* fix

* fmt

* line width

* cargo run --quiet --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* improve docs

* improve cli

* fix format

* fix bug?

* Revert "fix bug?"

This reverts commit 8051bf1.

* skip frame-metadata

* Update .gitlab-ci.yml

* fix import

* Update .gitlab-ci.yml

Co-authored-by: Parity Benchmarking Bot <admin@parity.io>

* Warp sync part I (#9227)

* Started warp sync

* BABE & GRANDPA recovery

* Warp sync protocol

* Sync warp proofs first

* Added basic documentation

* Apply suggestions from code review

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

* Style changes

* Apply suggestions from code review

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

* fmt

* Apply suggestions from code review

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>

* Fixed chage trie pruning wrt missing blocks

* Restore parent finalization

* fmt

* fmt

* Revert pwasm-utils bump

* Change error type & check API version

* Apply suggestions from code review

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

* Build fix

* Fixed target block check

* Formatting

Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Add rustfmt skip to default frame benchmarking template (#9473)

This was missed in the introduction pr of rustfmt. There we only had
updated the Substrate local template.

* CI: stop publishing to crates.io until unleash is fixed (#9474)

* CI: stop publishing to crates.io until unleash is fixed; allow restarting k8s runners

* CI: fix CI if ci-release tag is pushed

Co-authored-by: Eric Miller <emiller@lirio.co>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Gavin Wood <gavin@parity.io>
Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Squirrel <gilescope@gmail.com>
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
Co-authored-by: Arkadiy Paronyan <arkady.paronyan@gmail.com>
Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>
@chevdor chevdor mentioned this pull request Aug 9, 2021
Merged
@github-actions github-actions bot mentioned this pull request Aug 24, 2021
Closed
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomusdrw tomusdrw tomusdrw approved these changes

@andresilva andresilva andresilva approved these changes

@expenses expenses expenses approved these changes

@seunlanlege seunlanlege seunlanlege approved these changes

@sorpaas sorpaas Awaiting requested review from sorpaas sorpaas is a code owner

@shawntabrizi shawntabrizi Awaiting requested review from shawntabrizi

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D2-breaksapi D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gilescope @andresilva @tomusdrw @expenses @seunlanlege