Parachains should charge for proof size weight #2326

ggwpez · 2023-03-15T16:47:57Z

Closes #2322, depends on paritytech/substrate#13612

Currently the parachains do not charge weight for the proof size component of call weights.
With this MR it will calculate one fee for ref-time and one for proof-size and choose the larger one. This affects all parachain runtimes. There are some sanity checks in place for Statemin*.
Eventually we went a more compact and automated implementation, but that would require breaking changes in Substrate, so skipping that for now.

Use the trait directly instead of something that will have a blanket implementation for it. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

joepetrowski

Some documentation typos/bad find/replace I think. But logic looks good.

joepetrowski · 2023-03-15T17:17:40Z

parachains/runtimes/assets/statemine/src/constants.rs

+			// Map 10kb proof to 1 CENT.
+			let p = super::currency::CENTS;
+			let q = 10_000;


I suppose we don't have a reference on the Relay Chain.

Yes, since the relay chain has u64::MAX as max block proof-size whereas parachains only have 5 MiB.
We will probably need some further adjustments to find the sweet-spot, but for now this should provide something that we can work with.

There are going to be changes in Substrate soon that increase the proof size of everything (until we have https://github.com/paritytech/substrate/issues/13501), so to prevent accidental fuckups i added the tests that 1K transfers fit in a block.

parachains/runtimes/assets/statemine/src/lib.rs

parachains/runtimes/assets/statemint/src/constants.rs

parachains/runtimes/assets/statemint/src/lib.rs

parachains/runtimes/assets/westmint/src/constants.rs

parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/constants.rs

parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/constants.rs

parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/constants.rs

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma · 2023-03-16T16:37:44Z

parachains/runtimes/assets/statemint/src/constants.rs

@@ -55,13 +56,45 @@ pub mod fee {
 	///   - Setting it to `0` will essentially disable the weight fee.
 	///   - Setting it to `1` will cause the literal `#[weight = x]` values to be charged.
 	pub struct WeightToFee;
-	impl WeightToFeePolynomial for WeightToFee {
+	impl frame_support::weights::WeightToFee for WeightToFee {


Could be simplified by implementing WeightToFee for Tuple? or too many tuples impls? 🙈

Yes we could do a simple maximum norm. In the long run i would rather make the traits 2D weight compatible in Substrate, so we wont need it.

For now it is fine or? Since this targets 0.9.38.1

Yeah nah, you would be confused if it is addition, or max or what.

I suppose we can provide some nice helpers at he substrate layer though:

struct RefTimeWeightCostPerMicroSec<Balance, Cost: Get<Balance>>(..); impl WeightToFee for RefTimeConstPerMicroSec { .. } struct ProofSizeWeightCostPerKByte<Balance, Cost: Get<Balance>>(..); impl WeightToFee for RefTimeConstPerMicroSec { .. }

Or even a more generic:

struct CostPerUnit<Balance, Cost: Get<Balance>, Unit: ops::Div<Balance>>;

I think you can see where I am going with this. Just an idea

For now it is fine or? Since this targets 0.9.38.1

Yeah fine for now. Perhaps a follow-up issue that you can mentor?

kianenigma

End of the day, all runtimes want to express:

I want x unit of balance per ref_time unit.
I want y unit of balance per proof_size unit.
Take the max of them.

Perhaps we can provide structs that easily let you express x and y the former two as Get or const GENERIC.

ggwpez · 2023-03-16T16:49:33Z

cc @xlc in case you want to review.

parachains/runtimes/assets/statemine/src/constants.rs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez · 2023-03-17T15:22:39Z

bot merge

paritytech-processbot · 2023-03-17T15:22:46Z

Waiting for commit status.

paritytech-processbot · 2023-03-17T15:23:00Z

Merge cancelled due to error. Error: Checks failed for b6ca4b5

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez · 2023-03-17T15:52:19Z

The label check here only allows me to set T0, T1 or T2. Why do we then have the other T* labels?
They sound very useful but cant be used @the-right-joyce?

ggwpez · 2023-03-17T16:25:04Z

bot merge

* master: Companion for #13624 (#2354) Introduce Fellowship into Collectives (#2186) NFTs 2.0 on Statemine (#2314) Bump assert_cmd from 2.0.8 to 2.0.10 (#2341) Bump clap from 4.1.8 to 4.1.11 (#2352) Companion for substrate #13312: Rename `Deterministic` to `Enforce` (#2350) [Companion #13634] keystore overhaul (iter) (#2345) Revert #2304 (#2349) Deprecate Currency: Companion for #12951 (#2334) Bump ci-linux image for rust 1.68 Always pass port to jsonrpsee WebSocket client (#2339) bump zombienet to v1.3.40 (#2348) Improve build times by disabling wasm-builder in `no_std` (#2308) Bump toml from 0.7.2 to 0.7.3 (#2340) Bump serde from 1.0.152 to 1.0.156 (#2329) Parachains should charge for proof size weight (#2326) dmp-queue: Store messages if already processed more than the maximum (#2343) [Companion #13615] Keystore overhaul (#2336) Bump quote from 1.0.23 to 1.0.26 (#2331)

ggwpez added 2 commits March 15, 2023 17:28

Generalize trait requirement

c5ae80d

Use the trait directly instead of something that will have a blanket implementation for it. Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Charge for proof size weight

7855684

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez added the A3-in_progress Pull request is in progress. No review needed at this stage. label Mar 15, 2023

paritytech-ci requested review from a team March 15, 2023 16:50

joepetrowski approved these changes Mar 15, 2023

View reviewed changes

ggwpez mentioned this pull request Mar 15, 2023

Add dedicated FeePolynomial struct paritytech/substrate#13612

Merged

ggwpez and others added 4 commits March 16, 2023 15:18

Fix doc comments

afedfa8

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

Fix docs

3a0d538

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Update lock file

2c70c68

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

Fix imports

7550fdb

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

kianenigma reviewed Mar 16, 2023

View reviewed changes

paritytech-ci requested a review from a team March 16, 2023 16:38

kianenigma approved these changes Mar 16, 2023

View reviewed changes

sam0x17 approved these changes Mar 16, 2023

View reviewed changes

xlc reviewed Mar 17, 2023

View reviewed changes

parachains/runtimes/assets/statemine/src/constants.rs Show resolved Hide resolved

ggwpez added 2 commits March 17, 2023 16:15

Merge remote-tracking branch 'origin/master' into oty-charge-proof-size

dbdf7e4

Docs

b6ca4b5

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez removed T7-system_parachains This PR/Issue is related to System Parachains. T1-runtime This PR/Issue is related to the topic “runtime”. labels Mar 17, 2023

Fix imports

cbac742

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

ggwpez added T1-runtime This PR/Issue is related to the topic “runtime”. and removed T5-parachains This PR/Issue is related to Parachains. labels Mar 17, 2023

paritytech-processbot bot merged commit 91b3512 into master Mar 17, 2023

paritytech-processbot bot deleted the oty-charge-proof-size branch March 17, 2023 16:25

ggwpez added T7-system_parachains This PR/Issue is related to System Parachains. T5-parachains This PR/Issue is related to Parachains. labels Mar 17, 2023

ggwpez mentioned this pull request Mar 19, 2023

Co #13637: Remove Weight::without_{ref_time, proof_size} #2346

Merged

This was referenced May 10, 2023

[0.9.40] Weights v2 - Include WeightToFee on XCM Trader paritytech/trappist#168

Closed

[0.9.40] Change WeightToFee struct to charge for proof size weight. paritytech/trappist#169

Closed

AurevoirXavier mentioned this pull request May 22, 2023

To polkadot-v0.9.40 darwinia-network/darwinia#1153

Merged

2 tasks

github-actions bot mentioned this pull request Jun 15, 2023

Update substrate/polkadot/cumulus from v0.9.40 to v0.9.43 moonbeam-foundation/moonbeam#2354

Closed

AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request Jul 11, 2023

Companion of paritytech/cumulus#2326

7687790

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parachains should charge for proof size weight #2326

Parachains should charge for proof size weight #2326

ggwpez commented Mar 15, 2023

joepetrowski left a comment

joepetrowski Mar 15, 2023

ggwpez Mar 15, 2023 •

edited

Loading

kianenigma Mar 16, 2023

ggwpez Mar 16, 2023

ggwpez Mar 16, 2023

kianenigma Mar 16, 2023

kianenigma Mar 16, 2023

kianenigma left a comment •

edited

Loading

ggwpez commented Mar 16, 2023

ggwpez commented Mar 17, 2023

paritytech-processbot bot commented Mar 17, 2023

paritytech-processbot bot commented Mar 17, 2023

ggwpez commented Mar 17, 2023 •

edited

Loading

ggwpez commented Mar 17, 2023

Parachains should charge for proof size weight #2326

Parachains should charge for proof size weight #2326

Conversation

ggwpez commented Mar 15, 2023

joepetrowski left a comment

Choose a reason for hiding this comment

joepetrowski Mar 15, 2023

Choose a reason for hiding this comment

ggwpez Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

kianenigma Mar 16, 2023

Choose a reason for hiding this comment

ggwpez Mar 16, 2023

Choose a reason for hiding this comment

ggwpez Mar 16, 2023

Choose a reason for hiding this comment

kianenigma Mar 16, 2023

Choose a reason for hiding this comment

kianenigma Mar 16, 2023

Choose a reason for hiding this comment

kianenigma left a comment • edited Loading

Choose a reason for hiding this comment

ggwpez commented Mar 16, 2023

ggwpez commented Mar 17, 2023

paritytech-processbot bot commented Mar 17, 2023

paritytech-processbot bot commented Mar 17, 2023

ggwpez commented Mar 17, 2023 • edited Loading

ggwpez commented Mar 17, 2023

ggwpez Mar 15, 2023 •

edited

Loading

kianenigma left a comment •

edited

Loading

ggwpez commented Mar 17, 2023 •

edited

Loading