Skip to content

Commit

Permalink
Merge branch 'master' into elois-relay-mock-async-backing
Browse files Browse the repository at this point in the history
  • Loading branch information
librelois committed May 13, 2024
2 parents cfe9481 + 477a120 commit a98e92c
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 28 deletions.
30 changes: 30 additions & 0 deletions .forklift/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[compression]
type = "zstd"

[compression.zstd]
compressionLevel = 3

[general]
jobNameVariable = "CI_JOB_NAME"
jobsBlackList = []
logLevel = "warn"
threadsCount = 6

[metrics]
enabled = true
pushEndpoint = "placeholder"

[metrics.extraLabels]
environment = "production"
job_name = "$CI_JOB_NAME"
project_name = "$CI_PROJECT_PATH"

[storage]
type = "s3"

[storage.s3]
accessKeyId = "placeholder"
bucketName = "placeholder"
concurrency = 10
endpointUrl = "placeholder"
secretAccessKey = "placeholder"
14 changes: 6 additions & 8 deletions .github/workflows/quick-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
# GitHub Actions allows using 'env' in a container context.
# However, env variables don't work for forks: https://github.com/orgs/community/discussions/44322
# This workaround sets the container image for each job using 'set-image' job output.
runs-on: arc-runners-polkadot-sdk-default
runs-on: arc-runners-polkadot-sdk
timeout-minutes: 10
outputs:
IMAGE: ${{ steps.set_image.outputs.IMAGE }}
Expand All @@ -24,7 +24,7 @@ jobs:
- id: set_image
run: cat .github/env >> $GITHUB_OUTPUT
fmt:
runs-on: arc-runners-polkadot-sdk-default
runs-on: arc-runners-polkadot-sdk
timeout-minutes: 10
needs: [set-image]
container:
Expand All @@ -34,7 +34,7 @@ jobs:
- name: Cargo fmt
run: cargo +nightly fmt --all -- --check
check-dependency-rules:
runs-on: arc-runners-polkadot-sdk-default
runs-on: arc-runners-polkadot-sdk
timeout-minutes: 10
needs: [set-image]
container:
Expand All @@ -46,8 +46,7 @@ jobs:
cd substrate/
../.gitlab/ensure-deps.sh
check-rust-feature-propagation:
runs-on: arc-runners-polkadot-sdk-default
# runs-on: ubuntu-latest
runs-on: arc-runners-polkadot-sdk
timeout-minutes: 10
needs: [set-image]
container:
Expand All @@ -57,8 +56,7 @@ jobs:
- name: run zepter
run: zepter run check
test-rust-features:
runs-on: arc-runners-polkadot-sdk-default
# runs-on: ubuntu-latest
runs-on: arc-runners-polkadot-sdk
timeout-minutes: 10
needs: [set-image]
container:
Expand All @@ -68,7 +66,7 @@ jobs:
- name: run rust features
run: bash .gitlab/rust-features.sh .
check-toml-format:
runs-on: arc-runners-polkadot-sdk-default
runs-on: arc-runners-polkadot-sdk
timeout-minutes: 10
needs: [set-image]
container:
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/test-github-actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

env:
FORKLIFT_storage_s3_bucketName: ${{ secrets.FORKLIFT_storage_s3_bucketName }}
FORKLIFT_storage_s3_accessKeyId: ${{ secrets.FORKLIFT_storage_s3_accessKeyId }}
FORKLIFT_storage_s3_secretAccessKey: ${{ secrets.FORKLIFT_storage_s3_secretAccessKey }}
FORKLIFT_storage_s3_endpointUrl: ${{ secrets.FORKLIFT_storage_s3_endpointUrl }}
FORKLIFT_metrics_pushEndpoint: ${{ secrets.FORKLIFT_metrics_pushEndpoint }}

jobs:
set-image:
# GitHub Actions allows using 'env' in a container context.
Expand Down Expand Up @@ -38,7 +45,7 @@ jobs:
- name: Checkout
uses: actions/checkout@v4
- name: script
run: WASM_BUILD_NO_COLOR=1 time cargo test -p staging-node-cli --release --locked -- --ignored
run: WASM_BUILD_NO_COLOR=1 time forklift cargo test -p staging-node-cli --release --locked -- --ignored
quick-benchmarks:
runs-on: arc-runners-polkadot-sdk-beefy
timeout-minutes: 30
Expand All @@ -54,4 +61,4 @@ jobs:
- name: Checkout
uses: actions/checkout@v4
- name: script
run: time cargo run --locked --release -p staging-node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --quiet
run: time forklift cargo run --locked --release -p staging-node-cli --bin substrate-node --features runtime-benchmarks -- benchmark pallet --chain dev --pallet "*" --extrinsic "*" --steps 2 --repeat 1 --quiet
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ default:
.forklift-cache:
before_script:
- mkdir ~/.forklift
- cp $FL_FORKLIFT_CONFIG ~/.forklift/config.toml
- cp .forklift/config.toml ~/.forklift/config.toml
- >
if [ "$FORKLIFT_BYPASS" != "true" ]; then
echo "FORKLIFT_BYPASS not set";
Expand Down
16 changes: 16 additions & 0 deletions prdoc/pr_4326.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: CheckWeight checks for combined extrinsic length and proof size

doc:
- audience: Runtime Dev
description: |
The `CheckWeight` `SignedExtension` will now perform an additional check. The extension was verifying the extrinsic length and
weight limits individually. However, the proof size dimension of the weight and extrinsic length together are bound by the PoV size limit.
The `CheckWeight` extension will now check that the combined size of the proof and the extrinsic lengths will not
exceed the PoV size limit.

crates:
- name: frame-system
bump: minor
125 changes: 108 additions & 17 deletions substrate/frame/system/src/extensions/check_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,6 @@ where
}
}

/// Checks if the current extrinsic can fit into the block with respect to block weight limits.
///
/// Upon successes, it returns the new block weight as a `Result`.
fn check_block_weight(
info: &DispatchInfoOf<T::RuntimeCall>,
) -> Result<crate::ConsumedWeight, TransactionValidityError> {
let maximum_weight = T::BlockWeights::get();
let all_weight = Pallet::<T>::block_weight();
calculate_consumed_weight::<T::RuntimeCall>(maximum_weight, all_weight, info)
}

/// Checks if the current extrinsic can fit into the block with respect to block length limits.
///
/// Upon successes, it returns the new block length as a `Result`.
Expand Down Expand Up @@ -113,7 +102,12 @@ where
len: usize,
) -> Result<(), TransactionValidityError> {
let next_len = Self::check_block_length(info, len)?;
let next_weight = Self::check_block_weight(info)?;

let all_weight = Pallet::<T>::block_weight();
let maximum_weight = T::BlockWeights::get();
let next_weight =
calculate_consumed_weight::<T::RuntimeCall>(&maximum_weight, all_weight, info)?;
check_combined_proof_size(&maximum_weight, next_len, &next_weight)?;
Self::check_extrinsic_weight(info)?;

crate::AllExtrinsicsLen::<T>::put(next_len);
Expand All @@ -136,8 +130,32 @@ where
}
}

/// Check that the combined extrinsic length and proof size together do not exceed the PoV limit.
pub fn check_combined_proof_size(
maximum_weight: &BlockWeights,
next_len: u32,
next_weight: &crate::ConsumedWeight,
) -> Result<(), TransactionValidityError> {
// This extra check ensures that the extrinsic length does not push the
// PoV over the limit.
let total_pov_size = next_weight.total().proof_size().saturating_add(next_len as u64);
if total_pov_size > maximum_weight.max_block.proof_size() {
log::debug!(
target: LOG_TARGET,
"Extrinsic exceeds total pov size: {}kb, limit: {}kb",
total_pov_size as f64/1024.0,
maximum_weight.max_block.proof_size() as f64/1024.0
);
return Err(InvalidTransaction::ExhaustsResources.into())
}
Ok(())
}

/// Checks if the current extrinsic can fit into the block with respect to block weight limits.
///
/// Upon successes, it returns the new block weight as a `Result`.
pub fn calculate_consumed_weight<Call>(
maximum_weight: BlockWeights,
maximum_weight: &BlockWeights,
mut all_weight: crate::ConsumedWeight,
info: &DispatchInfoOf<Call>,
) -> Result<crate::ConsumedWeight, TransactionValidityError>
Expand Down Expand Up @@ -742,17 +760,90 @@ mod tests {

// when
assert_ok!(calculate_consumed_weight::<<Test as Config>::RuntimeCall>(
maximum_weight.clone(),
&maximum_weight,
all_weight.clone(),
&mandatory1
&mandatory1,
));
assert_err!(
calculate_consumed_weight::<<Test as Config>::RuntimeCall>(
maximum_weight,
&maximum_weight,
all_weight,
&mandatory2
&mandatory2,
),
InvalidTransaction::ExhaustsResources
);
}

#[test]
fn maximum_proof_size_includes_length() {
let maximum_weight = BlockWeights::builder()
.base_block(Weight::zero())
.for_class(DispatchClass::non_mandatory(), |w| {
w.base_extrinsic = Weight::zero();
w.max_total = Some(Weight::from_parts(20, 10));
})
.for_class(DispatchClass::Mandatory, |w| {
w.base_extrinsic = Weight::zero();
w.reserved = Some(Weight::from_parts(5, 10));
w.max_total = None;
})
.build_or_panic();

assert_eq!(maximum_weight.max_block, Weight::from_parts(20, 10));

// We have 10 reftime and 5 proof size left over.
let next_weight = crate::ConsumedWeight::new(|class| match class {
DispatchClass::Normal => Weight::from_parts(10, 5),
DispatchClass::Operational => Weight::from_parts(0, 0),
DispatchClass::Mandatory => Weight::zero(),
});

// Simple checks for the length
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
assert_ok!(check_combined_proof_size(&maximum_weight, 5, &next_weight));
assert_err!(
check_combined_proof_size(&maximum_weight, 6, &next_weight),
InvalidTransaction::ExhaustsResources
);

// We have 10 reftime and 0 proof size left over.
let next_weight = crate::ConsumedWeight::new(|class| match class {
DispatchClass::Normal => Weight::from_parts(10, 10),
DispatchClass::Operational => Weight::from_parts(0, 0),
DispatchClass::Mandatory => Weight::zero(),
});
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
assert_err!(
check_combined_proof_size(&maximum_weight, 1, &next_weight),
InvalidTransaction::ExhaustsResources
);

// We have 10 reftime and 2 proof size left over.
// Used weight is spread across dispatch classes this time.
let next_weight = crate::ConsumedWeight::new(|class| match class {
DispatchClass::Normal => Weight::from_parts(10, 5),
DispatchClass::Operational => Weight::from_parts(0, 3),
DispatchClass::Mandatory => Weight::zero(),
});
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
assert_ok!(check_combined_proof_size(&maximum_weight, 2, &next_weight));
assert_err!(
check_combined_proof_size(&maximum_weight, 3, &next_weight),
InvalidTransaction::ExhaustsResources
);

// Ref time is over the limit. Should not happen, but we should make sure that it is
// ignored.
let next_weight = crate::ConsumedWeight::new(|class| match class {
DispatchClass::Normal => Weight::from_parts(30, 5),
DispatchClass::Operational => Weight::from_parts(0, 0),
DispatchClass::Mandatory => Weight::zero(),
});
assert_ok!(check_combined_proof_size(&maximum_weight, 0, &next_weight));
assert_ok!(check_combined_proof_size(&maximum_weight, 5, &next_weight));
assert_err!(
check_combined_proof_size(&maximum_weight, 6, &next_weight),
InvalidTransaction::ExhaustsResources
);
}
}

0 comments on commit a98e92c

Please sign in to comment.