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

Add Runtime::OmniNode variant to polkadot-parachain #4805

Merged
merged 17 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions .github/workflows/release-50_publish-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ env:
VERSION: ${{ inputs.version }}

jobs:
fetch-artifacts: # this job will be triggered for the polkadot-parachain rc and release or polkadot rc image build
if: ${{ inputs.binary == 'polkadot-parachain' || inputs.binary == 'chain-spec-builder' || inputs.image_type == 'rc' }}
fetch-artifacts: # this job will be triggered for the polkadot-parachain-omni-node rc and release or polkadot rc image build
if: ${{ inputs.binary == 'polkadot-parachain-omni-node' || inputs.binary == 'chain-spec-builder' || inputs.image_type == 'rc' }}
runs-on: ubuntu-latest

steps:
- name: Checkout sources
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1

#TODO: this step will be needed when automated triggering will work
#TODO: this step will be needed when automated triggering will work
#this step runs only if the workflow is triggered automatically when new release is published
# if: ${{ env.EVENT_NAME == 'release' && env.EVENT_ACTION != '' && env.EVENT_ACTION == 'published' }}
# run: |
Expand Down Expand Up @@ -121,8 +121,8 @@ jobs:
name: release-artifacts
path: release-artifacts/${{ env.BINARY }}/**/*

build-container: # this job will be triggered for the polkadot-parachain rc and release or polkadot rc image build
if: ${{ inputs.binary == 'polkadot-parachain' || inputs.binary == 'chain-spec-builder' || inputs.image_type == 'rc' }}
build-container: # this job will be triggered for the polkadot-parachain-omni-node rc and release or polkadot rc image build
if: ${{ inputs.binary == 'polkadot-parachain-omni-node' || inputs.binary == 'chain-spec-builder' || inputs.image_type == 'rc' }}
runs-on: ubuntu-latest
needs: fetch-artifacts
environment: release
Expand Down Expand Up @@ -268,10 +268,10 @@ jobs:
- name: Cache Docker layers
uses: actions/cache@e12d46a63a90f2fae62d114769bbf2a179198b5c # v3.3.3
with:
path: /tmp/.buildx-cache
key: ${{ runner.os }}-buildx-${{ github.sha }}
restore-keys: |
${{ runner.os }}-buildx-
path: /tmp/.buildx-cache
key: ${{ runner.os }}-buildx-${{ github.sha }}
restore-keys: |
${{ runner.os }}-buildx-

- name: Login to Docker Hub
uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0
Expand All @@ -286,7 +286,7 @@ jobs:
echo "date=$date" >> $GITHUB_OUTPUT

- name: Build and push
id: docker_build
id: docker_build
uses: docker/build-push-action@4a13e500e55cf31b7a5d59a38ab2040ab0f42f56 # v5.1.0
with:
push: true
Expand Down
14 changes: 7 additions & 7 deletions .gitlab/pipeline/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,10 @@ build-linux-stable-cumulus:
RUSTFLAGS: "-Cdebug-assertions=y -Dwarnings"
script:
- echo "___Building a binary, please refrain from using it in production since it goes with the debug assertions.___"
- time cargo build --release --locked -p polkadot-parachain-bin --bin polkadot-parachain
- time cargo build --release --locked -p polkadot-parachain-omni-node --bin polkadot-parachain-omni-node
- echo "___Packing the artifacts___"
- mkdir -p ./artifacts
- mv ./target/release/polkadot-parachain ./artifacts/.
- mv ./target/release/polkadot-parachain-omni-node ./artifacts/.
- echo "___The VERSION is either a tag name or the curent branch if triggered not by a tag___"
- echo ${CI_COMMIT_REF_NAME} | tee ./artifacts/VERSION

Expand Down Expand Up @@ -288,10 +288,10 @@ build-short-benchmark-cumulus:
- .run-immediately
- .collect-artifacts
script:
- cargo build --profile release --locked --features=runtime-benchmarks,on-chain-release-build -p polkadot-parachain-bin --bin polkadot-parachain --workspace
- cargo build --profile release --locked --features=runtime-benchmarks,on-chain-release-build -p polkadot-parachain-omni-node --bin polkadot-parachain-omni-node --workspace
- mkdir -p artifacts
- target/release/polkadot-parachain --version
- cp ./target/release/polkadot-parachain ./artifacts/
- target/release/polkadot-parachain-omni-node --version
- cp ./target/release/polkadot-parachain-omni-node ./artifacts/

# substrate

Expand All @@ -313,7 +313,7 @@ build-linux-substrate:
# tldr: we need to checkout the branch HEAD explicitly because of our dynamic versioning approach while building the substrate binary
# see https://github.com/paritytech/ci_cd/issues/682#issuecomment-1340953589
- git checkout -B "$CI_COMMIT_REF_NAME" "$CI_COMMIT_SHA"
- !reference [.forklift-cache, before_script]
- !reference [ .forklift-cache, before_script ]
script:
- time WASM_BUILD_NO_COLOR=1 cargo build --locked --release -p staging-node-cli
- mv $CARGO_TARGET_DIR/release/substrate-node ./artifacts/substrate/substrate
Expand Down Expand Up @@ -353,7 +353,7 @@ build-runtimes-polkavm:
CARGO_TARGET_DIR: "$CI_PROJECT_DIR/target"
before_script:
- mkdir -p ./artifacts/subkey
- !reference [.forklift-cache, before_script]
- !reference [ .forklift-cache, before_script ]
script:
- cd ./substrate/bin/utils/subkey
- time SKIP_WASM_BUILD=1 cargo build --locked --release
Expand Down
2 changes: 1 addition & 1 deletion .gitlab/pipeline/short-benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ short-benchmark-westend: &short-bench
tags:
- benchmark
script:
- ./artifacts/polkadot-parachain benchmark pallet --chain $RUNTIME_CHAIN --pallet "*" --extrinsic "*" --steps 2 --repeat 1
- ./artifacts/polkadot-parachain-omni-node benchmark pallet --chain $RUNTIME_CHAIN --pallet "*" --extrinsic "*" --steps 2 --repeat 1
Copy link
Contributor

Choose a reason for hiding this comment

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

@ggwpez the days for this command is also numbered, is there a ticket to move this to the omni-bencher?

Side question: can the omni-node do benchmarking as well? what are the runtime-dependent parts of the benchmarking code that live in the node side? I suppose there are dependencies that led to the omni-bencher being created, but idk what they are.


short-benchmark-asset-hub-rococo:
<<: *short-bench-cumulus
Expand Down
2 changes: 1 addition & 1 deletion .gitlab/pipeline/zombienet/polkadot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ zombienet-polkadot-smoke-0002-parachains-parachains-upgrade-smoke:
- echo "gh-dir ${GH_DIR}"
- echo "local-dir ${LOCAL_DIR}"
- echo "polkadot image ${ZOMBIENET_INTEGRATION_TEST_IMAGE}"
- echo "polkadot-parachain image ${CUMULUS_IMAGE}"
- echo "polkadot-parachain-omni-node image ${CUMULUS_IMAGE}"
- echo "malus image ${MALUS_IMAGE}"
script:
- /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 24 additions & 13 deletions bridges/snowbridge/pallets/ethereum-client/benchmark.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
# Motivation

Demonstrate that
[FastAggregateVerify](https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-3.3.4) is the most
expensive call in ethereum beacon light client, though in [#13031](https://github.com/paritytech/substrate/pull/13031)
Parity team has wrapped some low level host functions for `bls-12381` but adding a high level host function specific
for it is super helpful.

# Benchmark

We add several benchmarks
[here](https://github.com/Snowfork/snowbridge/blob/8891ca3cdcf2e04d8118c206588c956541ae4710/parachain/pallets/ethereum-client/src/benchmarking/mod.rs#L98-L124)
as following to demonstrate
Expand All @@ -15,25 +17,31 @@ is the main bottleneck. Test data
is real from goerli network which contains 512 public keys from sync committee.

## sync_committee_period_update
Base line benchmark for extrinsic [sync_committee_period_update](https://github.com/Snowfork/snowbridge/blob/8891ca3cdcf2e04d8118c206588c956541ae4710/parachain/pallets/ethereum-client/src/lib.rs#L233)

Base line benchmark for
extrinsic [sync_committee_period_update](https://github.com/Snowfork/snowbridge/blob/8891ca3cdcf2e04d8118c206588c956541ae4710/parachain/pallets/ethereum-client/src/lib.rs#L233)

## bls_fast_aggregate_verify

Subfunction of extrinsic `sync_committee_period_update` which does what
[FastAggregateVerify](https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-3.3.4) requires.

## bls_aggregate_pubkey

Subfunction of `bls_fast_aggregate_verify` which decompress and instantiate G1 pubkeys only.

## bls_verify_message
Subfunction of `bls_fast_aggregate_verify` which verify the prepared signature only.

Subfunction of `bls_fast_aggregate_verify` which verify the prepared signature only.

# Result

## hardware spec

Run benchmark in a EC2 instance

```
cargo run --release --bin polkadot-parachain --features runtime-benchmarks -- benchmark machine --base-path /mnt/scratch/benchmark
cargo run --release --bin polkadot-parachain-omni-node --features runtime-benchmarks -- benchmark machine --base-path /mnt/scratch/benchmark
Copy link
Contributor

Choose a reason for hiding this comment

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

From my understanding, all node benchmark commands should eventually be moved to omni-bencher CLI. I am not sure if the tool is ready yet, but good to know what is the end goal.


+----------+----------------+-------------+-------------+-------------------+
| Category | Function | Score | Minimum | Result |
Expand All @@ -53,7 +61,7 @@ cargo run --release --bin polkadot-parachain --features runtime-benchmarks -- be
## benchmark

```
cargo run --release --bin polkadot-parachain \
cargo run --release --bin polkadot-parachain-omni-node \
--features runtime-benchmarks \
-- \
benchmark pallet \
Expand All @@ -68,18 +76,21 @@ benchmark pallet \

### [Weights](https://github.com/Snowfork/cumulus/blob/ron/benchmark-beacon-bridge/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/snowbridge_pallet_ethereum_client.rs)

|extrinsic | minimum execution time benchmarked(us) |
| --------------------------------------- |----------------------------------------|
|sync_committee_period_update | 123_126 |
|bls_fast_aggregate_verify| 121_083 |
|bls_aggregate_pubkey | 90_306 |
|bls_verify_message | 28_000 |
| extrinsic | minimum execution time benchmarked(us) |
|------------------------------|----------------------------------------|
| sync_committee_period_update | 123_126 |
| bls_fast_aggregate_verify | 121_083 |
| bls_aggregate_pubkey | 90_306 |
| bls_verify_message | 28_000 |

- [bls_fast_aggregate_verify](#bls_fast_aggregate_verify) consumes 98% execution time of [sync_committee_period_update](#sync_committee_period_update)
- [bls_fast_aggregate_verify](#bls_fast_aggregate_verify) consumes 98% execution time
of [sync_committee_period_update](#sync_committee_period_update)

- [bls_aggregate_pubkey](#bls_aggregate_pubkey) consumes 75% execution time of [bls_fast_aggregate_verify](#bls_fast_aggregate_verify)
- [bls_aggregate_pubkey](#bls_aggregate_pubkey) consumes 75% execution time
of [bls_fast_aggregate_verify](#bls_fast_aggregate_verify)

- [bls_verify_message](#bls_verify_message) consumes 23% execution time of [bls_fast_aggregate_verify](#bls_fast_aggregate_verify)
- [bls_verify_message](#bls_verify_message) consumes 23% execution time
of [bls_fast_aggregate_verify](#bls_fast_aggregate_verify)

# Conclusion

Expand Down
2 changes: 1 addition & 1 deletion bridges/snowbridge/scripts/benchmark.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Example command for updating pallet benchmarking
pushd ../cumulus
cargo run --release --bin polkadot-parachain \
cargo run --release --bin polkadot-parachain-omni-node \
--features runtime-benchmarks \
-- \
benchmark pallet \
Expand Down
14 changes: 7 additions & 7 deletions bridges/testing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@ To start those tests, you need to:

- download latest [zombienet release](https://github.com/paritytech/zombienet/releases);

- build Polkadot binary by running `cargo build -p polkadot --release --features fast-runtime` command in the
[`polkadot-sdk`](https://github.com/paritytech/polkadot-sdk) repository clone;
- build Polkadot binary by running `cargo build -p polkadot --release --features fast-runtime` command in the
[`polkadot-sdk`](https://github.com/paritytech/polkadot-sdk) repository clone;

- build Polkadot Parachain binary by running `cargo build -p polkadot-parachain-bin --release` command in the
[`polkadot-sdk`](https://github.com/paritytech/polkadot-sdk) repository clone;
- build Polkadot Parachain binary by running `cargo build -p polkadot-parachain-omni-node --release` command in the
[`polkadot-sdk`](https://github.com/paritytech/polkadot-sdk) repository clone;

- ensure that you have [`node`](https://nodejs.org/en) installed. Additionally, we'll need globally installed
`polkadot/api-cli` package (use `npm install -g @polkadot/api-cli@beta` to install it);
`polkadot/api-cli` package (use `npm install -g @polkadot/api-cli@beta` to install it);

- build Substrate relay by running `cargo build -p substrate-relay --release` command in the
[`parity-bridges-common`](https://github.com/paritytech/parity-bridges-common) repository clone.
[`parity-bridges-common`](https://github.com/paritytech/parity-bridges-common) repository clone.

- copy fresh `substrate-relay` binary, built in previous point, to the `~/local_bridge_testing/bin/substrate-relay`;

- change the `POLKADOT_SDK_PATH` and `ZOMBIENET_BINARY_PATH` (and ensure that the nearby variables
have correct values) in the `./run-tests.sh`.
have correct values) in the `./run-tests.sh`.

After that, you could run tests with the `./run-tests.sh` command. Hopefully, it'll show the
"All tests have completed successfully" message in the end. Otherwise, it'll print paths to zombienet
Expand Down
4 changes: 2 additions & 2 deletions bridges/testing/run-new-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ if [ "$ZOMBIENET_DOCKER_PATHS" -eq 1 ]; then
unset ZOMBIENET_IMAGE

export POLKADOT_BINARY=/usr/local/bin/polkadot
export POLKADOT_PARACHAIN_BINARY=/usr/local/bin/polkadot-parachain
export POLKADOT_PARACHAIN_BINARY=/usr/local/bin/polkadot-parachain-omni-node

export ZOMBIENET_BINARY=/usr/local/bin/zombie
export SUBSTRATE_RELAY_BINARY=/usr/local/bin/substrate-relay
else
export POLKADOT_BINARY=$POLKADOT_SDK_PATH/target/release/polkadot
export POLKADOT_PARACHAIN_BINARY=$POLKADOT_SDK_PATH/target/release/polkadot-parachain
export POLKADOT_PARACHAIN_BINARY=$POLKADOT_SDK_PATH/target/release/polkadot-parachain-omni-node

export ZOMBIENET_BINARY=~/local_bridge_testing/bin/zombienet-linux-x64
export SUBSTRATE_RELAY_BINARY=~/local_bridge_testing/bin/substrate-relay
Expand Down
Loading
Loading