Skip to content
Snippets Groups Projects
  1. Jan 20, 2025
    • Benjamin Gallois's avatar
      Fix `frame-benchmarking-cli` not buildable without rocksdb (#7263) · 2c4cecce
      Benjamin Gallois authored
      ## Description
      
      The `frame-benchmarking-cli` crate has not been buildable without the
      `rocksdb` feature since version 1.17.0.
      
      **Error:**  
      ```rust
      self.database()?.unwrap_or(Database::RocksDb),
                                   ^^^^^^^ variant or associated item not found in `Database`
      ```
      
      This issue is also related to the `rocksdb` feature bleeding (#3793),
      where the `rocksdb` feature was always activated even when compiling
      this crate with `--no-default-features`.
      
      **Fix:**  
      - Resolved the error by choosing `paritydb` as the default database when
      compiled without the `rocksdb` feature.
      - Fixed the issue where the `sc-cli` crate's `rocksdb` feature was
      always active, even compiling `frame-benchmarking-cli` with
      `--no-default-features`.
      
      ## Review Notes
      
      Fix the crate to be built without rocksdb, not intended to solve #3793.
      
      ---------
      
      Co-authored-by: command-bot <>
    • Sebastian Kunert's avatar
      Stabilize `ensure_execute_processes_have_correct_num_threads` test (#7253) · d5d9b127
      Sebastian Kunert authored
      Saw this test flake a few times, last time
      [here](https://github.com/paritytech/polkadot-sdk/actions/runs/12834432188/job/35791830215).
      
      We first fetch all processes in the test, then query `/proc/<pid>/stat`
      for every one of them. When the file was not found, we would error. Now
      we tolerate not finding this file. Ran 200 times locally without error,
      before would fail a few times, probably depending on process fluctuation
      (which I expect to be high on CI runners).
  2. Jan 15, 2025
  3. Jan 14, 2025
    • Alexandru Gheorghe's avatar
      approval-voting: Fix sending of assignments after restart (#6973) · d38bb953
      Alexandru Gheorghe authored
      There is a problem on restart where nodes will not trigger their needed
      assignment if they were offline while the time of the assignment passed.
      
      That happens because after restart we will hit this condition
      https://github.com/paritytech/polkadot-sdk/blob/4e805ca0/polkadot/node/core/approval-voting/src/lib.rs#L2495
      and considered will be `tick_now` which is already higher than the tick
      of our assignment.
      
      The fix is to schedule a wakeup for untriggered assignments at restart
      and let the logic of processing an wakeup decide if it needs to trigger
      the assignment or not.
      
      One thing that we need to be careful here is to make sure we don't
      schedule the wake up immediately after restart because, the node would
      still be behind with all the assignments that should have received and
      might make it wrongfully decide it needs to trigger its assignment, so I
      added a `RESTART_WAKEUP_DELAY: Tick = 12` which should be more t...
    • Alexandru Gheorghe's avatar
      Retry approval on availability failure if the check is still needed (#6807) · 6878ba1f
      Alexandru Gheorghe authored
      
      Recovering the POV can fail in situation where the node just restart and
      the DHT topology wasn't fully discovered yet, so the current node can't
      connect to most of its Peers. This is bad because for gossiping the
      assignment you need to be connected to just a few peers, so because we
      can't approve the candidate and other nodes will see this as a no show.
      
      This becomes bad in the scenario where you've got a lot of nodes
      restarting at the same time, so you end up having a lot of no-shows in
      the network that are never covered, in that case it makes sense for
      nodes to actually retry approving the candidate at a later data in time
      and retry several times if the block containing the candidate wasn't
      approved.
      
      ## TODO
      - [x] Add a subsystem test.
      
      ---------
      
      Signed-off-by: default avatarAlexandru Gheorghe <alexandru.gheorghe@parity.io>
    • Alin Dima's avatar
  4. Jan 13, 2025
  5. Jan 09, 2025
  6. Jan 05, 2025
    • thiolliere's avatar
      Implement cumulus StorageWeightReclaim as wrapping transaction extension +... · 63c73bf6
      thiolliere authored
      Implement cumulus StorageWeightReclaim as wrapping transaction extension + frame system ReclaimWeight (#6140)
      
      (rebasing of https://github.com/paritytech/polkadot-sdk/pull/5234)
      
      ## Issues:
      
      * Transaction extensions have weights and refund weight. So the
      reclaiming of unused weight must happen last in the transaction
      extension pipeline. Currently it is inside `CheckWeight`.
      * cumulus storage weight reclaim transaction extension misses the proof
      size of logic happening prior to itself.
      
      ## Done:
      
      * a new storage `ExtrinsicWeightReclaimed` in frame-system. Any logic
      which attempts to do some reclaim must use this storage to avoid double
      reclaim.
      * a new function `reclaim_weight` in frame-system pallet: info and post
      info in arguments, read the already reclaimed weight, calculate the new
      unused weight from info and post info. do the more accurate reclaim if
      higher.
      * `CheckWeight` is unchanged and still reclaim the weight in post
      dispatch
      * `ReclaimWeight` is a new transaction extension in frame system. For
      s...
  7. Dec 20, 2024
    • Xavier Lau's avatar
      Reorder dependencies' keys (#6967) · a843d15e
      Xavier Lau authored
      
      It doesn't make sense to only reorder the features array.
      
      For example:
      
      This makes it hard for me to compare the dependencies and features,
      especially some crates have a really really long dependencies list.
      ```toml​
      [dependencies]
      c = "*"
      a = "*"
      b = "*"
      
      [features]
      std = [
        "a",
        "b",
        "c",
      ]
      ```
      
      This makes my life easier.
      ```toml​
      [dependencies]
      a = "*"
      b = "*"
      c = "*"
      
      [features]
      std = [
        "a",
        "b",
        "c",
      ]
      ```
      
      ---------
      
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
      Co-authored-by: command-bot <>
  8. Dec 19, 2024
  9. Dec 13, 2024
    • Alexandru Gheorghe's avatar
      Fix approval-voting canonicalize off by one (#6864) · 2dd2bb5a
      Alexandru Gheorghe authored
      Approval voting canonicalize is off by one that means if we are
      finalizing blocks one by one, approval-voting cleans it up every other
      block for example:
      
      - With 1, 2, 3, 4, 5, 6 blocks created, the stored range would be
      StoredBlockRange(1,7)
      - When block 3 is finalized the canonicalize works and StoredBlockRange
      is (4,7)
      - When block 4 is finalized the canonicalize exists early because of the
      `if range.0 > canon_number` break clause, so blocks are not cleaned up.
      - When block 5 is finalized the canonicalize works and StoredBlockRange
      becomes (6,7) and both block 4 and 5 are cleaned up.
      
      The consequences of this is that sometimes we keep block entries around
      after they are finalized, so at restart we consider this blocks and send
      them to approval-distribution.
      
      In most cases this is not a problem, but in the case when finality is
      lagging on restart approval-distribution will receive 4 as being the
      oldest block it needs to work on, and since BlockFinalize...
    • Tsvetomir Dimitrov's avatar
      Collation fetching fairness (#4880) · 5153e2b5
      Tsvetomir Dimitrov authored
      
      Related to https://github.com/paritytech/polkadot-sdk/issues/1797
      
      # The problem
      When fetching collations in collator protocol/validator side we need to
      ensure that each parachain has got a fair core time share depending on
      its assignments in the claim queue. This means that the number of
      collations fetched per parachain should ideally be equal to (but
      definitely not bigger than) the number of claims for the particular
      parachain in the claim queue.
      
      # Why the current implementation is not good enough
      The current implementation doesn't guarantee such fairness. For each
      relay parent there is a `waiting_queue` (PerRelayParent -> Collations ->
      waiting_queue) which holds any unfetched collations advertised to the
      validator. The collations are fetched on first in first out principle
      which means that if two parachains share a core and one of the
      parachains is more aggressive it might starve the second parachain. How?
      At each relay parent up to `max_candidate_depth` candidates are accepted
      (enforced in `fn is_seconded_limit_reached`) so if one of the parachains
      is quick enough to fill in the queue with its advertisements the
      validator will never fetch anything from the rest of the parachains
      despite they are scheduled. This doesn't mean that the aggressive
      parachain will occupy all the core time (this is guaranteed by the
      runtime) but it will deny the rest of the parachains sharing the same
      core to have collations backed.
      
      # How to fix it
      The solution I am proposing is to limit fetches and advertisements based
      on the state of the claim queue. At each relay parent the claim queue
      for the core assigned to the validator is fetched. For each parachain a
      fetch limit is calculated (equal to the number of entries in the claim
      queue). Advertisements are not fetched for a parachain which has
      exceeded its claims in the claim queue. This solves the problem with
      aggressive parachains advertising too much collations.
      
      The second part is in collation fetching logic. The collator will keep
      track on which collations it has fetched so far. When a new collation
      needs to be fetched instead of popping the first entry from the
      `waiting_queue` the validator examines the claim queue and looks for the
      earliest claim which hasn't got a corresponding fetch. This way the
      collator will always try to prioritise the most urgent entries.
      
      ## How the 'fair share of coretime' for each parachain is determined?
      Thanks to async backing we can accept more than one candidate per relay
      parent (with some constraints). We also have got the claim queue which
      gives us a hint which parachain will be scheduled next on each core. So
      thanks to the claim queue we can determine the maximum number of claims
      per parachain.
      
      For example the claim queue is [A A A] at relay parent X so we know that
      at relay parent X we can accept three candidates for parachain A. There
      are two things to consider though:
      1. If we accept more than one candidate at relay parent X we are
      claiming the slot of a future relay parent. So accepting two candidates
      for relay parent X means that we are claiming the slot at rp X+1 or rp
      X+2.
      2. At the same time the slot at relay parent X could have been claimed
      by a previous relay parent(s). This means that we need to accept less
      candidates at X or even no candidates.
      
      There are a few cases worth considering:
      1. Slot claimed by previous relay parent.
          CQ @ rp X: [A A A]
          Advertisements at X-1 for para A: 2
          Advertisements at X-2 for para A: 2
      Outcome - at rp X we can accept only 1 advertisement since our slots
      were already claimed.
      2. Slot in our claim queue already claimed at future relay parent
          CQ @ rp X: [A A A]
          Advertisements at X+1 for para A: 1
          Advertisements at X+2 for para A: 1
      Outcome: at rp X we can accept only 1 advertisement since the slots in
      our relay parents were already claimed.
      
      The situation becomes more complicated with multiple leaves (forks).
      Imagine we have got a fork at rp X:
      ```
      CQ @ rp X: [A A A]
      (rp X) -> (rp X+1) -> rp(X+2)
               \-> (rp X+1')
      ```
      Now when we examine the claim queue at RP X we need to consider both
      forks. This means that accepting a candidate at X means that we should
      have a slot for it in *BOTH* leaves. If for example there are three
      candidates accepted at rp X+1' we can't accept any candidates at rp X
      because there will be no slot for it in one of the leaves.
      
      ## How the claims are counted
      There are two solutions for counting the claims at relay parent X:
      1. Keep a state for the claim queue (number of claims and which of them
      are claimed) and look it up when accepting a collation. With this
      approach we need to keep the state up to date with each new
      advertisement and each new leaf update.
      2. Calculate the state of the claim queue on the fly at each
      advertisement. This way we rebuild the state of the claim queue at each
      advertisements.
      
      Solution 1 is hard to implement with forks. There are too many variants
      to keep track of (different state for each leaf) and at the same time we
      might never need to use them. So I decided to go with option 2 -
      building claim queue state on the fly.
      
      To achieve this I've extended `View` from backing_implicit_view to keep
      track of the outer leaves. I've also added a method which accepts a
      relay parent and return all paths from an outer leaf to it. Let's call
      it `paths_to_relay_parent`.
      
      So how the counting works for relay parent X? First we examine the
      number of seconded and pending advertisements (more on pending in a
      second) from relay parent X to relay parent X-N (inclusive) where N is
      the length of the claim queue. Then we use `paths_to_relay_parent` to
      obtain all paths from outer leaves to relay parent X. We calculate the
      claims at relay parents X+1 to X+N (inclusive) for each leaf and get the
      maximum value. This way we guarantee that the candidate at rp X can be
      included in each leaf. This is the state of the claim queue which we use
      to decide if we can fetch one more advertisement at rp X or not.
      
      ## What is a pending advertisement
      I mentioned that we count seconded and pending advertisements at relay
      parent X. A pending advertisement is:
      1. An advertisement which is being fetched right now.
      2. An advertisement pending validation at backing subsystem.
      3. An advertisement blocked for seconding by backing because we don't
      know on of its parent heads.
      
      Any of these is considered a 'pending fetch' and a slot for it is kept.
      All of them are already tracked in `State`.
      
      ---------
      
      Co-authored-by: default avatarMaciej <maciej.zyszkiewicz@parity.io>
      Co-authored-by: command-bot <>
      Co-authored-by: default avatarAlin Dima <alin@parity.io>
  10. Dec 12, 2024
  11. Dec 11, 2024
    • Alexandru Gheorghe's avatar
      Make approval-distribution aggression a bit more robust and less spammy (#6696) · 85dd228d
      Alexandru Gheorghe authored
      
      After finality started lagging on kusama around `2025-11-25 15:55:40`
      nodes started being overloaded with messages and some restarted with
      ```
      Subsystem approval-distribution-subsystem appears unresponsive when sending a message of type polkadot_node_subsystem_types::messages::ApprovalDistributionMessage. origin=polkadot_service::relay_chain_selection::SelectRelayChainInner<sc_client_db::Backend<sp_runtime::generic::block::Block<sp_runtime::generic::header::Header<u32, sp_runtime::traits::BlakeTwo256>, sp_runtime::OpaqueExtrinsic>>, polkadot_overseer::Handle>
      ```
      
      I think this happened because our aggression in the current form is way
      too spammy and create problems in situation where we already constructed
      blocks with a load of candidates to check which what happened around
      `#25933682` before and after. However aggression, does help in the
      nightmare scenario where the network is segmented and sparsely
      connected, so I tend to think we shouldn't completely remove it.
      
      The current configuration is:
      ```
      l1_threshold: Some(16),
      l2_threshold: Some(28),
      resend_unfinalized_period: Some(8),
      ```
      The way aggression works right now :
      1. After L1 is triggered all nodes send all messages they created to all
      the other nodes and all messages they would have they already send
      according to the topology.
      2. Because of resend_unfinalized_period for each block all messages at
      step 1) are sent every 8 blocks, so for example let's say we have blocks
      1 to 24 unfinalized, then at block 25, all messages for block 1, 9 will
      be resent, and consequently at block 26, all messages for block 2, 10
      will be resent, this becomes worse as more blocks are created if backing
      backpressure did not kick in yet. In total this logic makes that each
      node receive 3 * total_number_of messages_per_block
      3. L2 aggression is way too spammy, when L2 aggression is enabled all
      nodes sends all messages of a block on GridXY, that means that all
      messages are received and sent by node at least 2*sqrt(num_validators),
      so on kusama would be 66 * NUM_MESSAGES_AT_FIRST_UNFINALIZED_BLOCK, so
      even with a reasonable number of messages like 10K, which you can have
      if you escalated because of no shows, you end-up sending and receiving
      ~660k messages at once, I think that's what makes the
      approval-distribution to appear unresponsive on some nodes.
      4. Duplicate messages are received by the nodes which turn, mark the
      node as banned, which may create more no-shows.
      
      ## Proposed improvements:
      1. Make L2 trigger way later 28 blocks, instead of 64, this should
      literally the last resort, until then we should try to let the
      approval-voting escalation mechanism to do its things and cover the
      no-shows.
      2. On L1 aggression don't send messages for blocks too far from the
      first_unfinalized there is no point in sending the messages for block
      20, if block 1 is still unfinalized.
      3. On L1 aggression, send messages then back-off for 3 *
      resend_unfinalized_period to give time for everyone to clear up their
      queues.
      4. If aggression is enabled accept duplicate messages from validators
      and don't punish them by reducting their reputation which, which may
      create no-shows.
      
      ---------
      
      Signed-off-by: default avatarAlexandru Gheorghe <alexandru.gheorghe@parity.io>
      Co-authored-by: default avatarAndrei Sandu <54316454+sandreim@users.noreply.github.com>
  12. Dec 10, 2024
    • Alexandru Gheorghe's avatar
      Fix order of resending messages after restart (#6729) · 65a4e5ee
      Alexandru Gheorghe authored
      
      The way we build the messages we need to send to approval-distribution
      can result in a situation where is we have multiple assignments covered
      by a coalesced approval, the messages are sent in this order:
      
      ASSIGNMENT1, APPROVAL, ASSIGNMENT2, because we iterate over each
      candidate and add to the queue of messages both the assignment and the
      approval for that candidate, and when the approval reaches the
      approval-distribution subsystem it won't be imported and gossiped
      because one of the assignment for it is not known.
      
      So in a network where a lot of nodes are restarting at the same time we
      could end up in a situation where a set of the nodes correctly received
      the assignments and approvals before the restart and approve their
      blocks and don't trigger their assignments. The other set of nodes
      should receive the assignments and approvals after the restart, but
      because the approvals never get broacasted anymore because of this bug,
      the only way they could approve is if other nodes start broadcasting
      their assignments.
      
      I think this bug contribute to the reason the network did not recovered
      on `25-11-25 15:55:40` after the restarts.
      
      Tested this scenario with a `zombienet` where `nodes` are finalising
      blocks because of aggression and all nodes are restarted at once and
      confirmed the network lags and doesn't recover before and it does after
      the fix
      
      ---------
      
      Signed-off-by: default avatarAlexandru Gheorghe <alexandru.gheorghe@parity.io>
    • Joseph Zhao's avatar
      Remove AccountKeyring everywhere (#5899) · 311ea438
      Joseph Zhao authored
      
      Close: #5858
      
      ---------
      
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
  13. Dec 09, 2024
    • Alexandru Gheorghe's avatar
      Fix `Possible bug: Vote import failed` after aggression is enabled (#6690) · da953454
      Alexandru Gheorghe authored
      
      After finality started lagging on kusama around 025-11-25 15:55:40
      validators started seeing ocassionally this log, when importing votes
      covering more than one assignment.
      ```
      Possible bug: Vote import failed
      ```
      
      That happens because the assumption that assignments from the same
      validator would have the same required routing doesn't hold after you
      enabled aggression, so you might end up receiving the first assignment
      then you modify the routing for it in `enable_aggression` then your
      receive the second assignment and the vote covering both assignments, so
      the rouing for the first and second assingment wouldn't match and we
      would fail to import the vote.
      
      From the logs I've seen, I don't think this is the reason the network
      didn't fully recover until the failsafe kicked it, because the votes had
      been already imported in approval-voting before this error.
      
      ---------
      
      Signed-off-by: default avatarAlexandru Gheorghe <alexandru.gheorghe@parity.io>
  14. Dec 03, 2024
  15. Nov 29, 2024
  16. Nov 25, 2024
  17. Nov 19, 2024
  18. Nov 18, 2024
    • Tsvetomir Dimitrov's avatar
      Remove `ProspectiveParachainsMode` usage in backing subsystem (#6215) · 7d5d7202
      Tsvetomir Dimitrov authored
      
      Since async backing parameters runtime api is released on all networks
      the code in backing subsystem can be simplified by removing the usages
      of `ProspectiveParachainsMode` and keeping only the branches of the code
      under `ProspectiveParachainsMode::Enabled`.
      
      The PR does that and reworks the tests in mod.rs to use async backing.
      It's a preparation for
      https://github.com/paritytech/polkadot-sdk/issues/5079
      
      ---------
      
      Co-authored-by: default avatarAlin Dima <alin@parity.io>
      Co-authored-by: command-bot <>
  19. Nov 13, 2024
    • Stephane Gurgenidze's avatar
      backing: improve session buffering for runtime information (#6284) · e617d1d0
      Stephane Gurgenidze authored
      ## Issue
      [[#3421] backing: improve session buffering for runtime
      information](https://github.com/paritytech/polkadot-sdk/issues/3421)
      
      ## Description
      In the current implementation of the backing module, certain pieces of
      information, which remain unchanged throughout a session, are fetched
      multiple times via runtime API calls. The goal of this task was to
      introduce a local cache to store such session-stable information and
      perform the runtime API call only once per session.
      
      This PR implements caching specifically for the validators list, node
      features, executor parameters, minimum backing votes threshold, and
      validator-to-group mapping, which were previously fetched from the
      runtime or computed each time `PerRelayParentState` was built. Now, this
      information is cached and reused within the session.
      
      ## TODO
      * [X] Create a separate struct for per-session caches;
      * [X] Cache validators list;
      * [X] Cache node features;
      * [X] Cache executor parameters;
      * [X] Cache minimum backing votes threshold;
      * [X] Cache validator-to-group mapping;
      * [X] Update tests to reflect these changes;
      * [X] Add prdoc.
      
      ## For the next PR
      Cache validator groups and any other session-stable data (if present).
    • Andrei Eres's avatar
      Remove debug message about pruning active leaves (#6440) · 5aeaa664
      Andrei Eres authored
      
      # Description
      
      The debug message was added to identify a potential memory leak.
      However, recent observations show that pruning works as expected.
      Therefore, it is best to remove this line, as it generates quite
      annoying logs.
      
      
      ## Integration
      
      Doesn't affect downstream projects.
      
      ---------
      
      Co-authored-by: default avatarGitHub Action <action@github.com>
  20. Nov 11, 2024
    • Nazar Mokrynskyi's avatar
      Remove network starter that is no longer needed (#6400) · b601d57a
      Nazar Mokrynskyi authored
      
      # Description
      
      This seems to be an old artifact of the long closed
      https://github.com/paritytech/substrate/issues/6827 that I noticed when
      working on related code earlier.
      
      ## Integration
      
      `NetworkStarter` was removed, simply remove its usage:
      ```diff
      -let (network, system_rpc_tx, tx_handler_controller, start_network, sync_service) =
      +let (network, system_rpc_tx, tx_handler_controller, sync_service) =
          build_network(BuildNetworkParams {
      ...
      -start_network.start_network();
      ```
      
      ## Review Notes
      
      Changes are trivial, the only reason for this to not be accepted is if
      it is desired to not start network automatically for whatever reason, in
      which case the description of network starter needs to change.
      
      # Checklist
      
      * [x] My PR includes a detailed description as outlined in the
      "Description" and its two subsections above.
      * [ ] My PR follows the [labeling requirements](
      
      https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
      ) of this project (at minimum one label for `T` required)
      * External contributors: ask maintainers to put the right label on your
      PR.
      
      ---------
      
      Co-authored-by: default avatarGitHub Action <action@github.com>
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
    • Alin Dima's avatar
      fix prospective-parachains best backable chain reversion bug (#6417) · 05ad5475
      Alin Dima authored
      
      Kudos to @EclesioMeloJunior for noticing it 
      
      Also added a regression test for it. The existing unit test was
      exercising only the case where the full chain is reverted
      
      ---------
      
      Co-authored-by: default avatarGitHub Action <action@github.com>
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
  21. Nov 07, 2024
    • Alin Dima's avatar
      make prospective-parachains debug logs less spammy (#6406) · 2680f20a
      Alin Dima authored
      Fixes https://github.com/paritytech/polkadot-sdk/issues/6172
    • Andrei Eres's avatar
      PVF: drop backing jobs if it is too late (#5616) · 6c8a347a
      Andrei Eres authored
      
      Fixes https://github.com/paritytech/polkadot-sdk/issues/5530
      
      This PR introduces the removal of backing jobs that have been back
      pressured for longer than `allowedAncestryLen`, as these candidates are
      no longer viable.
      
      It is reasonable to expect a result for a backing job execution within
      `allowedAncestryLen` blocks. Therefore, we set the job TTL as a relay
      block number and synchronize the validation host by sending activated
      leaves.
      
      ---------
      
      Co-authored-by: default avatarAndrei Sandu <54316454+sandreim@users.noreply.github.com>
      Co-authored-by: default avatarBranislav Kontur <bkontur@gmail.com>
    • Nazar Mokrynskyi's avatar
      Syncing strategy refactoring (part 3) (#5737) · 12d90524
      Nazar Mokrynskyi authored
      # Description
      
      This is a continuation of
      https://github.com/paritytech/polkadot-sdk/pull/5666 that finally fixes
      https://github.com/paritytech/polkadot-sdk/issues/5333.
      
      This should allow developers to create custom syncing strategies or even
      the whole syncing engine if they so desire. It also moved syncing engine
      creation and addition of corresponding protocol outside
      `build_network_advanced` method, which is something Bastian expressed as
      desired in
      https://github.com/paritytech/polkadot-sdk/issues/5#issuecomment-1700816458
      
      Here I replaced strategy-specific types and methods in `SyncingStrategy`
      trait with generic ones. Specifically `SyncingAction` is now used by all
      strategies instead of strategy-specific types with conversions.
      `StrategyKey` was an enum with a fixed set of options and now replaced
      with an opaque type that strategies create privately and send to upper
      layers as an opaque type. Requests and responses are now handled in a
      generic way regardless of the strategy, which reduced and simplified
      strategy API.
      
      `PolkadotSyncingStrategy` now lives in its dedicated module (had to edit
      .gitignore for this) like other strategies.
      
      `build_network_advanced` takes generic `SyncingService` as an argument
      alongside with a few other low-level types (that can probably be
      extracted in the future as well) without any notion of specifics of the
      way syncing is actually done. All the protocol and tasks are created
      outside and not a part of the network anymore. It still adds a bunch of
      protocols like for light client and some others that should eventually
      be restructured making `build_network_advanced` just building generic
      network and not application-specific protocols handling.
      
      ## Integration
      
      Just like https://github.com/paritytech/polkadot-sdk/pull/5666
      introduced `build_polkadot_syncing_strategy`, this PR introduces
      `build_default_block_downloader`, but for convenience and to avoid
      typical boilerplate a simpler high-level function
      `build_default_syncing_engine` is added that will take care of creating
      typical block downloader, syncing strategy and syncing engine, which is
      what most users will be using going forward. `build_network` towards the
      end of the PR was renamed to `build_network_advanced` and
      `build_network`'s API was reverted to
      pre-https://github.com/paritytech/polkadot-sdk/pull/5666, so most users
      will not see much of a difference during upgrade unless they opt-in to
      use new API.
      
      ## Review Notes
      
      For `StrategyKey` I was thinking about using something like private type
      and then storing `TypeId` inside instead of a static string in it, let
      me know if that would preferred.
      
      The biggest change happened to requests that different strategies make
      and how their responses are handled. The most annoying thing here is
      that block response decoding, in contrast to all other responses, is
      dependent on request. This meant request had to be sent throughout the
      system. While originally `Response` was `Vec<u8>`, I didn't want to
      re-encode/decode request and response just to fit into that API, so I
      ended up with `Box<dyn Any + Send>`. This allows responses to be truly
      generic and each strategy will know how to downcast it back to the
      concrete type when handling the response.
      
      Import queue refactoring was needed to move `SyncingEngine` construction
      out of `build_network` that awkwardly implemented for `SyncingService`,
      but due to `&mut self` wasn't usable on `Arc<SyncingService>` for no
      good reason. `Arc<SyncingService>` itself is of course useless, but
      refactoring to replace it with just `SyncingService` was unfortunately
      rejected in https://github.com/paritytech/polkadot-sdk/pull/5454
      
      As usual I recommend to review this PR as a series of commits instead of
      as the final diff, it'll make more sense that way.
      
      # Checklist
      
      * [x] My PR includes a detailed description as outlined in the
      "Description" and its two subsections above.
      * [x] My PR follows the [labeling requirements](
      
      https://github.com/paritytech/polkadot-sdk/blob/master/docs/contributor/CONTRIBUTING.md#Process
      ) of this project (at minimum one label for `T` required)
      * External contributors: ask maintainers to put the right label on your
      PR.
      * [x] I have made corresponding changes to the documentation (if
      applicable)
  22. Nov 06, 2024
  23. Nov 05, 2024
  24. Nov 04, 2024
  25. Oct 31, 2024
  26. Oct 30, 2024
    • Sebastian Kunert's avatar
      Add overhead benchmark to frame-omni-bencher (#5891) · 40547f9f
      Sebastian Kunert authored
      # Benchmark Overhead Command for Parachains
      
      This implements the `benchmark overhead` command for parachains. Full
      context is available at:
      https://github.com/paritytech/polkadot-sdk/issues/5303. Previous attempt
      was this https://github.com/paritytech/polkadot-sdk/pull/5283, but here
      we have integration into frame-omni-bencher and improved tooling.
      
      ## Changes Overview
      
      Users are now able to use `frame-omni-bencher` to generate
      `extrinsic_weight.rs` and `block_weight.rs` files for their runtime. The
      core logic for generating these remains untouched; this PR provides
      mostly machinery to make it work for parachains at all.
      
      Similar to the pallet benchmarks, we gain the option to benchmark based
      on just a runtime:
      
      ```
      frame-omni-bencher v1 benchmark overhead --runtime {{runtime}}
      ```
      
      or with a spec:
      
      ```
      frame-omni-bencher v1 benchmark overhead --chain {{spec}} --genesis-builder spec
      ```
      
      In this case, the genesis state is generated from the runtime pres...