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

XCM: Properly set the pricing for the DMP router #6843

Merged
merged 46 commits into from
Apr 20, 2023

Conversation

KiChjang
Copy link
Contributor

@KiChjang KiChjang commented Mar 8, 2023

Partial paritytech/substrate#6121.

This PR properly adds a delivery fee to the DMP router used by the relay chain. It uses a base fee of CENTS * 3, and is then multiplied by 1.01 to the power of the number of pending messages in the message queue of the destination. Such a formula is intended to make the fees cheap initially when the queue is small, but would get prohibitively expensive relatively quickly.

@KiChjang KiChjang added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. B1-note_worthy Changes should be noted in the release notes labels Mar 8, 2023
@paritytech-ci paritytech-ci requested review from a team March 8, 2023 13:07
@KiChjang KiChjang added T1-runtime This PR/Issue is related to the topic “runtime”. T6-XCM This PR/Issue is related to XCM. and removed T1-runtime This PR/Issue is related to the topic “runtime”. T6-XCM This PR/Issue is related to XCM. labels Mar 8, 2023
@@ -14,13 +14,34 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! To prevent Out of Memory errors on the `DownwardMessageQueue`,
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting and line wrapping in this doc comment is somewhat not on par with the rest of the code; always been suggesting using https://marketplace.visualstudio.com/items?itemName=stkb.rewrap or similar.

Copy link
Contributor

@kianenigma kianenigma Apr 11, 2023

Choose a reason for hiding this comment

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

Some spacing would also help; see this line from this PR as an inspiration.

@paritytech-ci paritytech-ci requested a review from a team April 11, 2023 09:04

/// The number to multiply the base delivery fee by.
#[pallet::storage]
#[pallet::getter(fn delivery_fee_factor)]
Copy link
Contributor

Choose a reason for hiding this comment

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

getters are being deprecated: please don't add more :D

https://github.com/paritytech/substrate/issues/13259

@paritytech-ci paritytech-ci requested a review from a team April 11, 2023 09:06
@vstam1 vstam1 requested a review from kianenigma April 11, 2023 16:48
@@ -26,6 +26,7 @@ sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", de
sp-mmr-primitives = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-staking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-arithmetic = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

Not being used?

@@ -27,6 +27,7 @@ sp-io = { git = "https://github.com/paritytech/substrate", branch = "master", de
sp-mmr-primitives = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-staking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
sp-arithmetic = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

Same?

@paritytech-ci paritytech-ci requested a review from a team April 13, 2023 21:37
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Thanks for the well-documented PR! 💯

@@ -1410,7 +1410,7 @@ construct_runtime! {
ParaScheduler: parachains_scheduler::{Pallet, Storage} = 55,
Paras: parachains_paras::{Pallet, Call, Storage, Event, Config, ValidateUnsigned} = 56,
Initializer: parachains_initializer::{Pallet, Call, Storage} = 57,
Dmp: parachains_dmp::{Pallet, Call, Storage} = 58,
Dmp: parachains_dmp::{Pallet, Storage} = 58,
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be all refactored to use default parts in the future, no?

runtime/parachains/src/dmp.rs Outdated Show resolved Hide resolved
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@vstam1
Copy link
Contributor

vstam1 commented Apr 20, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit a5e253d into master Apr 20, 2023
@paritytech-processbot paritytech-processbot bot deleted the kckyeung/xcm-sender-price branch April 20, 2023 11:04
ordian added a commit that referenced this pull request Apr 26, 2023
* master: (30 commits)
  update rocksdb to 0.20.1 (#7113)
  Reduce base proof size weight component to zero (#7081)
  PVF: Move PVF workers into separate crate (#7101)
  Companion for #13923 (#7111)
  update safe call filter (#7080)
  PVF: Don't dispute on missing artifact (#7011)
  XCM: Properly set the pricing for the DMP router (#6843)
  pvf: Update docs for PVF artifacts (#6551)
  Bump syn from 2.0.14 to 2.0.15 (#7093)
  Companion for substrate#13771 (#6983)
  Added Dwellir Nigeria bootnodes. (#7097)
  Companion for Substrate #13889 (#7063)
  Switch to DNS name based bootnodes for Rococo (#7040)
  companion for substrate#13883 (#7059)
  [xcm] Added `UnpaidExecution` instruction to `UnpaidRemoteExporter` (#7091)
  Bump serde_json from 1.0.85 to 1.0.96 (#7072)
  Bump hex-literal from 0.3.4 to 0.4.1 (#7071)
  Small simplification (#7089)
  [XCM - UnpaidRemoteExporter] Remove unreachable code (#7088)
  sync versions with current release (#7083)
  ...
ordian added a commit that referenced this pull request Apr 26, 2023
* master: (39 commits)
  malus: dont panic on missing validation data (#6952)
  Offences Migration v1: Removes `ReportsByKindIndex` (#7114)
  Fix stalling dispute coordinator. (#7125)
  Fix rolling session window (#7126)
  [ci] Update buildah command and version (#7128)
  Bump assigned_slots params (#6991)
  XCM: Remote account converter (#6662)
  Rework `dispute-coordinator` to use `RuntimeInfo` for obtaining session information instead of `RollingSessionWindow` (#6968)
  Revert default proof size back to 64 KB (#7115)
  update rocksdb to 0.20.1 (#7113)
  Reduce base proof size weight component to zero (#7081)
  PVF: Move PVF workers into separate crate (#7101)
  Companion for #13923 (#7111)
  update safe call filter (#7080)
  PVF: Don't dispute on missing artifact (#7011)
  XCM: Properly set the pricing for the DMP router (#6843)
  pvf: Update docs for PVF artifacts (#6551)
  Bump syn from 2.0.14 to 2.0.15 (#7093)
  Companion for substrate#13771 (#6983)
  Added Dwellir Nigeria bootnodes. (#7097)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants