Skip to content
Snippets Groups Projects
  1. Jan 04, 2025
  2. Dec 30, 2024
  3. 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 <>
  4. Dec 16, 2024
    • Nazar Mokrynskyi's avatar
      Upgrade libp2p from 0.52.4 to 0.54.1 (#6248) · c8812883
      Nazar Mokrynskyi authored
      
      # Description
      
      Fixes https://github.com/paritytech/polkadot-sdk/issues/5996
      
      https://github.com/libp2p/rust-libp2p/releases/tag/libp2p-v0.53.0
      https://github.com/libp2p/rust-libp2p/blob/master/CHANGELOG.md
      
      ## Integration
      
      Nothing special is needed, just note that `yamux_window_size` is no
      longer applicable to libp2p (litep2p seems to still have it though).
      
      ## Review Notes
      
      There are a few simplifications and improvements done in libp2p 0.53
      regarding swarm interface, I'll list a few key/applicable here.
      
      https://github.com/libp2p/rust-libp2p/pull/4788 removed
      `write_length_prefixed` function, so I inlined its code instead.
      
      https://github.com/libp2p/rust-libp2p/pull/4120 introduced new
      `libp2p::SwarmBuilder` instead of now deprecated
      `libp2p::swarm::SwarmBuilder`, the transition is straightforward and
      quite ergonomic (can be seen in tests).
      
      https://github.com/libp2p/rust-libp2p/pull/4581 is the most annoying
      change I have seen that basically makes many enums `#[non_exhaustive]`.
      I mapped some, but those that couldn't be mapped I dealt with by
      printing log messages once they are hit (the best solution I could come
      up with, at least with stable Rust).
      
      https://github.com/libp2p/rust-libp2p/issues/4306 makes connection close
      as soon as there are no handler using it, so I had to replace
      `KeepAlive::Until` with an explicit future that flips internal boolean
      after timeout, achieving the old behavior, though it should ideally be
      removed completely at some point.
      
      `yamux_window_size` is no longer used by libp2p thanks to
      https://github.com/libp2p/rust-libp2p/pull/4970 and generally Yamux
      should have a higher performance now.
      
      I have resolved and cleaned up all deprecations related to libp2p except
      `BandwidthSinks`. Libp2p deprecated it (though it is still present in
      0.54.1, which is why I didn't handle it just yet). Ideally Substrate
      would finally [switch to the official Prometheus
      client](https://github.com/paritytech/substrate/issues/12699), in which
      case we'd get metrics for free. Otherwise a bit of code will need to be
      copy-pasted to maintain current behavior with `BandwidthSinks` gone,
      which I left a TODO about.
      
      The biggest change in 0.54.0 is
      https://github.com/libp2p/rust-libp2p/pull/4568 that changed transport
      APIs and enabled unconditional potential port reuse, which can lead to
      very confusing errors if running two Substrate nodes on the same machine
      without changing listening port explicitly.
      
      Overall nothing scary here, but testing is always appreciated.
      
      # 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.
      
      ---
      
      Polkadot Address: 1vSxzbyz2cJREAuVWjhXUT1ds8vBzoxn2w4asNpusQKwjJd
      
      ---------
      
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
  5. Dec 13, 2024
  6. Dec 10, 2024
  7. Dec 05, 2024
    • Andrei Eres's avatar
      Optimize initialization of networking protocol benchmarks (#6636) · f4a196ab
      Andrei Eres authored
      
      # Description
      These changes should enhance the quality of benchmark results by
      excluding worker initialization time from the measurements and reducing
      the overall duration of the benchmarks.
      
      ### Integration
      It should not affect any downstream projects.
      
      ### Review Notes
      - Workers initialize once per benchmark to avoid side effects.  
      - The listen address is assigned when a worker starts.  
      - Benchmarks are divided into two groups by size to create better charts
      for comparison.
      
      ---------
      
      Co-authored-by: default avatarGitHub Action <action@github.com>
  8. Dec 04, 2024
    • Alexandru Vasile's avatar
      chore: Update litep2p to v0.8.3 (#6742) · 5ca72675
      Alexandru Vasile authored
      
      ## [0.8.3] - 2024-12-03
      
      This release includes two fixes for small memory leaks on edge-cases in
      the notification and request-response protocols.
      
      ### Fixed
      
      - req-resp: Fix memory leak of pending substreams
      ([#297](https://github.com/paritytech/litep2p/pull/297))
      - notification: Fix memory leak of pending substreams
      ([#296](https://github.com/paritytech/litep2p/pull/296))
      
      cc @paritytech/networking
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
  9. Nov 29, 2024
  10. Nov 28, 2024
    • Alexandru Vasile's avatar
      chore: Update litep2p to v0.8.2 (#6677) · 51c3e95a
      Alexandru Vasile authored
      
      This includes a critical fix for debug release versions of litep2p
      (which are running in Kusama as validators).
      
      While at it, have stopped the oncall pain of alerts around
      `incoming_connections_total`. We can rethink the metric expose of
      litep2p in Q1.
      
      
      
      
      
      ## [0.8.2] - 2024-11-27
      
      This release ensures that the provided peer identity is verified at the
      crypto/noise protocol level, enhancing security and preventing potential
      misuses.
      The release also includes a fix that caused `TransportService` component
      to panic on debug builds.
      
      ### Fixed
      
      - req-resp: Fix panic on connection closed for substream open failure
      ([#291](https://github.com/paritytech/litep2p/pull/291))
      - crypto/noise: Verify crypto/noise signature payload
      ([#278](https://github.com/paritytech/litep2p/pull/278))
      
      ### Changed
      
      - transport_service/logs: Provide less details for trace logs
      ([#292](https://github.com/paritytech/litep2p/pull/292))
      
      
      ## Testing Done
      
      This has been extensively tested in Kusama on all validators, that are
      now running litep2p.
      
      Deployed PR: https://github.com/paritytech/polkadot-sdk/pull/6638
      
      ### Litep2p Dashboards
      ![Screenshot 2024-11-26 at 19 19
      41](https://github.com/user-attachments/assets/e00b2b2b-7e64-4d96-ab26-165e2b8d0dc9)
      
      
      ### Libp2p vs Litep2p CPU usage
      
      After deploying litep2p we have reduced CPU usage from around 300-400%
      to 200%, this is a significant boost in performance, freeing resources
      for other subcomponents to function more optimally.
      
      
      ![image(1)](https://github.com/user-attachments/assets/fa793df5-4d58-4601-963d-246e56dd2a26)
      
      
      
      cc @paritytech/sdk-node
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
      Co-authored-by: default avatarGitHub Action <action@github.com>
  11. Nov 27, 2024
    • Alexandru Vasile's avatar
      litep2p/req-resp: Always provide main protocol name in responses (#6603) · 2a0b2680
      Alexandru Vasile authored
      Request responses are initialized with a main protocol name, and
      optional protocol names as a fallback.
      
      Running litep2p in kusama as a validator has surfaced a `debug_asserts`
      coming from the sync component:
      
      https://github.com/paritytech/polkadot-sdk/blob/3906c578
      
      /substrate/client/network/sync/src/strategy/chain_sync.rs#L640-L646
      
      The issue is that we initiate a request-response over the main protocol
      name `/genesis/sync/2` but receive a response over the legacy procotol
      `ksm/sync/2`. This behavior is correct because litep2p propagates to the
      higher levels the protocol that responded.
      
      In contrast, libp2p provides the main protocol name regardless of
      negotiating a legacy protocol.
      
      Because of this, higher level components assume that only the main
      protocol name will respond.
      To not break this assumption, this PR alings litep2p shim layer with the
      libp2p behavior.
      
      
      Closes: https://github.com/paritytech/polkadot-sdk/issues/6581
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
  12. Nov 21, 2024
    • Alexandru Vasile's avatar
      network-gossip: Ensure sync event is processed on unknown peer roles (#6553) · 7c9e34b5
      Alexandru Vasile authored
      
      The `GossipEngine::poll_next` implementation polls both the
      `notification_service` and the `sync_event_stream`.
      
      If both polls produce valid data to be processed
      (`Poll::Ready(Some(..))`), then the sync event is ignored when we
      receive `NotificationEvent::NotificationStreamOpened` and the role
      cannot be deduced.
      
      This PR ensures both events are processed gracefully. While at it, I
      have added a warning to the sync engine related to
      `notification_service` producing `Poll::Ready(None)`.
      
      This effectively ensures that `SyncEvents` propagate to the network
      potentially fixing any state mismatch.
      
      
      For more context: https://github.com/paritytech/polkadot-sdk/issues/6507
      
      cc @paritytech/sdk-node
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
  13. Nov 19, 2024
  14. Nov 18, 2024
    • Liu-Cheng Xu's avatar
      Pure state sync refactoring (part-1) (#6249) · 06a68bec
      Liu-Cheng Xu authored
      This pure refactoring of state sync is preparing for
      https://github.com/paritytech/polkadot-sdk/issues/4. As the rough plan
      in
      https://github.com/paritytech/polkadot-sdk/issues/4#issuecomment-2439588876,
      there will be two PRs for the state sync refactoring.
      
      This first PR focuses on isolating the function
      `process_state_key_values()` as the central point for storing received
      state data in memory. This function will later be adapted to forward the
      state data directly to the DB layer for persistent sync. A follow-up PR
      will handle the encapsulation of `StateSyncMetadata` to support this
      persistent storage.
      
      Although there are many commits in this PR, each commit is small and
      intentionally incremental to facilitate a smoother review, please review
      them commit by commit. Each commit should represent an equivalent
      rewrite of the existing logic, with one exception
      https://github.com/paritytech/polkadot-sdk/commit/bb447b2d,
      which has a slight deviation from the original but is correct IMHO.
      Please give this commit special attention during the review.
  15. Nov 15, 2024
    • Alexandru Vasile's avatar
      network/litep2p: Update litep2p network backend to version 0.8.1 (#6484) · 8bea091e
      Alexandru Vasile authored
      This PR updates the litep2p backend to version 0.8.1 from 0.8.0.
      - Check the [litep2p updates forum
      post](https://forum.polkadot.network/t/litep2p-network-backend-updates/9973/3)
      for performance dashboards.
      - Check [litep2p release
      notes](https://github.com/paritytech/litep2p/pull/288)
      
      The v0.8.1 release includes key fixes that enhance the stability and
      performance of the litep2p library. The focus is on long-running
      stability and improvements to polling mechanisms.
      
      ### Long Running Stability Improvements
      
      This issue caused long-running nodes to reject all incoming connections,
      impacting overall stability.
      
      Addressed a bug in the connection limits functionality that incorrectly
      tracked connections due for rejection.
      
      This issue caused an artificial increase in inbound peers, which were
      not being properly removed from the connection limit count.
      
      This fix ensures more accurate tracking and management of peer
      connections [#286](https://github.com/paritytech/litep2p/pull/286).
      
      ### Polling implementation fixes
      
      This release provides multiple fixes to the polling mechanism, improving
      how connections and events are processed:
      - Resolved an overflow issue in TransportContext’s polling index for
      streams, preventing potential crashes
      ([#283](https://github.com/paritytech/litep2p/pull/283)).
      - Fixed a delay in the manager’s poll_next function that prevented
      immediate polling of newly added futures
      ([#287](https://github.com/paritytech/litep2p/pull/287)).
      - Corrected an issue where the listener did not return Poll::Ready(None)
      when it was closed, ensuring proper signal handling
      ([#285](https://github.com/paritytech/litep2p/pull/285)).
      
      
      ### Fixed
      
      - manager: Fix connection limits tracking of rejected connections
      ([#286](https://github.com/paritytech/litep2p/pull/286))
      - transport: Fix waking up on filtered events from `poll_next`
      ([#287](https://github.com/paritytech/litep2p/pull/287))
      - transports: Fix missing Poll::Ready(None) event from listener
      ([#285](https://github.com/paritytech/litep2p/pull/285))
      - manager: Avoid overflow on stream implementation for
      `TransportContext`
      ([#283](https://github.com/paritytech/litep2p/pull/283))
      - manager: Log when polling returns Ready(None)
      ([#284](https://github.com/paritytech/litep2p/pull/284))
      
      
      ### Testing Done
      
      Started kusama nodes running side by side with a higher number of
      inbound and outbound connections (500).
      We previously tested with peers bounded at 50. This testing filtered out
      the fixes included in the latest release.
      
      With this high connection testing setup, litep2p outperforms libp2p in
      almost every domain, from performance to the warnings / errors
      encountered while operating the nodes.
      
      TLDR: this is the version we need to test on kusama validators next
      
      - Litep2p
      
      Repo            | Count      | Level      | Triage report
      -|-|-|-
      polkadot-sdk | 409 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Peer disconnected with inflight after backoffs. Banned,
      disconnecting. )
      litep2p | 128 | warn | Refusing to add known address that corresponds to
      a different peer ID
      litep2p | 54 | warn | inbound identify substream opened for peer who
      doesn't exist
      polkadot-sdk | 7 | error | :broken_heart: Called `on_validated_block_announce` with a
      bad peer ID .*
      polkadot-sdk    | 1          | warn       | :x: Error while dialing .*: .*
      polkadot-sdk | 1 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Invalid justification. Banned, disconnecting. )
      
      - Libp2p
      
      Repo            | Count      | Level      | Triage report
      -|-|-|-
      polkadot-sdk | 1023 | warn | :broken_heart: Ignored block \(#.* -- .*\) announcement
      from .* because all validation slots are occupied.
      polkadot-sdk | 472 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Unsupported protocol. Banned, disconnecting. )
      polkadot-sdk | 379 | error | :broken_heart: Called `on_validated_block_announce` with
      a bad peer ID .*
      polkadot-sdk | 163 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Invalid justification. Banned, disconnecting. )
      polkadot-sdk | 116 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Peer disconnected with inflight after backoffs. Banned,
      disconnecting. )
      polkadot-sdk | 83 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Same block request multiple times. Banned,
      disconnecting. )
      polkadot-sdk | 4 | warn | Re-finalized block #.* \(.*\) in the canonical
      chain, current best finalized is #.*
      polkadot-sdk | 2 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Genesis mismatch. Banned, disconnecting. )
      polkadot-sdk | 2 | warn | Report .*: .* to .*. Reason: .*. Banned,
      disconnecting. ( Not requested block data. Banned, disconnecting. )
      polkadot-sdk | 2 | warn | Can't listen on .* because: .*
      polkadot-sdk    | 1          | warn       | :x:
      
       Error while dialing .*: .*
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
  16. Nov 13, 2024
  17. Nov 12, 2024
  18. Nov 07, 2024
    • Liu-Cheng Xu's avatar
      Expose more syncing types to enable custom syncing strategy (#6163) · 0e09ad44
      Liu-Cheng Xu authored
      This PR exposes additional syncing types to facilitate the development
      of a custom syncing strategy based on the existing Polkadot syncing
      strategy. Specifically, my goal is to isolate the state sync and chain
      sync components, allowing the state to be downloaded from the P2P
      network without running a full regular Substrate node. I also need to
      intercept the state responses during the state sync process.
      
      The newly exposed types are necessary to implement this custom syncing
      strategy.
    • Andrei Eres's avatar
      Add networking benchmarks for libp2p (#6077) · 8795ae66
      Andrei Eres authored
      
      # Description
      
      Implemented benchmarks for Notifications and RequestResponse protocols
      with libp2p implementation. These benchmarks allow us to monitor
      regressions and implement fixes before they are observed in real chain.
      In the future, they can be used for targeted optimizations of litep2p
      compared to libp2p.
      
      Part of https://github.com/paritytech/polkadot-sdk/issues/5220
      
      Next steps:
      - Add benchmarks for litep2p implementation
      - Optimize load to get better results
      - Add benchmarks to CI to catch regressions
      
      
      
      ## Integration
      
      Benchmarks don't affect downstream projects.
      
      ---------
      
      Co-authored-by: default avataralvicsam <alvicsam@gmail.com>
      Co-authored-by: default avatarGitHub Action <action@github.com>
    • Alexandru Vasile's avatar
      net/discovery: Do not propagate external addr with different peerIDs (#6380) · bb8c7a3b
      Alexandru Vasile authored
      
      This PR ensures that external addresses with different PeerIDs are not
      propagated to the higher layer of the network code.
      
      While at it, this ensures that libp2p only adds the `/p2p/peerid` part
      to the discovered address if it does not contain it already.
      
      This is a followup from:
      - https://github.com/paritytech/polkadot-sdk/pull/6298
      
      cc @paritytech/networking
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
    • 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 regardl...
  19. Nov 06, 2024
    • Alexandru Vasile's avatar
      litep2p/peerset: Do not disconnect all peers on `SetReservedPeers` command (#6016) · ccb2a889
      Alexandru Vasile authored
      
      Previously, when receiving the `SetReservedPeers { reserved }` all peers
      not in the `reserved` set were removed.
      
      This is incorrect, the intention of `SetReservedPeers` is to change the
      active set of reserved peers and disconnect previously reserved peers
      not in the new set.
      
      While at it, have added a few other improvements to make the peerset
      more robust:
      - `SetReservedPeers`: does not disconnect all peers
      - `SetReservedPeers`: if a reserved peer is no longer reserved, the
      peerset tries to move the peers to the regular set if the slots allow
      this move. This ensures the (now regular) peer counts towards slot
      allocation.
      - every 1 seconds: If we don't have enough connect peers, add the
      reserved peers to the list that the peerstore ignores. Reserved peers
      are already connected and the peerstore might return otherwise a
      reserved peer
      
      
      ### Next Steps
      
      - [x] More testing
      
      cc @paritytech/networking
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
      Co-authored-by: default avatarMichal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
  20. Nov 05, 2024
    • Alexandru Vasile's avatar
      litep2p: Update litep2p to v0.8.0 (#6353) · 94389a93
      Alexandru Vasile authored
      
      This PR updates litep2p to the latest release.
      
      - `KademliaEvent::PutRecordSucess` is renamed to fix word typo
      - `KademliaEvent::GetProvidersSuccess` and
      `KademliaEvent::IncomingProvider` are needed for bootnodes on DHT work
      and will be utilized later
      
      
      ### Added
      
      - kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
      ([#258](https://github.com/paritytech/litep2p/pull/258))
      - kad: Providers part 7: better types and public API, public addresses &
      known providers ([#246](https://github.com/paritytech/litep2p/pull/246))
      - kad: Providers part 6: stop providing
      ([#245](https://github.com/paritytech/litep2p/pull/245))
      - kad: Providers part 5: `GET_PROVIDERS` query
      ([#236](https://github.com/paritytech/litep2p/pull/236))
      - kad: Providers part 4: refresh local providers
      ([#235](https://github.com/paritytech/litep2p/pull/235))
      - kad: Providers part 3: publish provider records (start providing)
      ([#234](https://github.com/paritytech/litep2p/pull/234))
      
      ### Changed
      
      - transport_service: Improve connection stability by downgrading
      connections on substream inactivity
      ([#260](https://github.com/paritytech/litep2p/pull/260))
      - transport: Abort canceled dial attempts for TCP, WebSocket and Quic
      ([#255](https://github.com/paritytech/litep2p/pull/255))
      - kad/executor: Add timeout for writting frames
      ([#277](https://github.com/paritytech/litep2p/pull/277))
      - kad: Avoid cloning the `KademliaMessage` and use reference for
      `RoutingTable::closest`
      ([#233](https://github.com/paritytech/litep2p/pull/233))
      - peer_state: Robust state machine transitions
      ([#251](https://github.com/paritytech/litep2p/pull/251))
      - address_store: Improve address tracking and add eviction algorithm
      ([#250](https://github.com/paritytech/litep2p/pull/250))
      - kad: Remove unused serde cfg
      ([#262](https://github.com/paritytech/litep2p/pull/262))
      - req-resp: Refactor to move functionality to dedicated methods
      ([#244](https://github.com/paritytech/litep2p/pull/244))
      - transport_service: Improve logs and move code from tokio::select macro
      ([#254](https://github.com/paritytech/litep2p/pull/254))
      
      ### Fixed
      
      - tcp/websocket/quic: Fix cancel memory leak
      ([#272](https://github.com/paritytech/litep2p/pull/272))
      - transport: Fix pending dials memory leak
      ([#271](https://github.com/paritytech/litep2p/pull/271))
      - ping: Fix memory leak of unremoved `pending_opens`
      ([#274](https://github.com/paritytech/litep2p/pull/274))
      - identify: Fix memory leak of unused `pending_opens`
      ([#273](https://github.com/paritytech/litep2p/pull/273))
      - kad: Fix not retrieving local records
      ([#221](https://github.com/paritytech/litep2p/pull/221))
      
      See release changelog for more details:
      https://github.com/paritytech/litep2p/releases/tag/v0.8.0
      
      cc @paritytech/networking
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
  21. Oct 22, 2024
    • tmpolaczyk's avatar
      Use bool::then instead of then_some with function calls (#6156) · 6418131a
      tmpolaczyk authored
      
      I noticed that hardware benchmarks are being run even though we pass the
      --no-hardware-benchmarks cli flag. After some debugging, the cause is an
      incorrect usage of the `then_some` method.
      
      From [std
      docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):
      
      > Arguments passed to then_some are eagerly evaluated; if you are
      passing the result of a function call, it is recommended to use
      [then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
      which is lazily evaluated.
      
      ```rust
      let mut a = 0;
      let mut function_with_side_effects = || { a += 1; };
      
      true.then_some(function_with_side_effects());
      false.then_some(function_with_side_effects());
      
      // `a` is incremented twice because the value passed to `then_some` is
      // evaluated eagerly.
      assert_eq!(a, 2);
      ```
      
      This PR fixes all the similar usages of the `then_some` method across
      the codebase.
      
      polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD
      
      ---------
      
      Signed-off-by: default avatarOliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
      Co-authored-by: default avatarShawn Tabrizi <shawntabrizi@gmail.com>
      Co-authored-by: default avatarOliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  22. Oct 18, 2024
    • Bastian Köcher's avatar
      sync: Remove checking of the extrinsics root (#5686) · 5e0843e5
      Bastian Köcher authored
      With the introduction of `system_version` in
      https://github.com/paritytech/polkadot-sdk/pull/4257 the extrinsic root
      may also use the `V1` layout. At this point in the sync code it would
      require some special handling to find out the `system_version`. So, this
      pull request is removing it. The extrinsics root is checked when
      executing the block later, so that at least no invalid block gets
      imported.
    • georgepisaltu's avatar
      FRAME: Reintroduce `TransactionExtension` as a replacement for `SignedExtension` (#3685) · b76e91ac
      georgepisaltu authored
      
      Original PR https://github.com/paritytech/polkadot-sdk/pull/2280
      reverted in https://github.com/paritytech/polkadot-sdk/pull/3665
      
      This PR reintroduces the reverted functionality with additional changes,
      related effort
      [here](https://github.com/paritytech/polkadot-sdk/pull/3623).
      Description is copied over from the original PR
      
      First part of [Extrinsic
      Horizon](https://github.com/paritytech/polkadot-sdk/issues/2415)
      
      Introduces a new trait `TransactionExtension` to replace
      `SignedExtension`. Introduce the idea of transactions which obey the
      runtime's extensions and have according Extension data (né Extra data)
      yet do not have hard-coded signatures.
      
      Deprecate the terminology of "Unsigned" when used for
      transactions/extrinsics owing to there now being "proper" unsigned
      transactions which obey the extension framework and "old-style" unsigned
      which do not. Instead we have __*General*__ for the former and
      __*Bare*__ for the latter. (Ultimately, the latter will be phased out as
      a type of transaction, and Bare will only be used for Inherents.)
      
      Types of extrinsic are now therefore:
      - Bare (no hardcoded signature, no Extra data; used to be known as
      "Unsigned")
      - Bare transactions (deprecated): Gossiped, validated with
      `ValidateUnsigned` (deprecated) and the `_bare_compat` bits of
      `TransactionExtension` (deprecated).
        - Inherents: Not gossiped, validated with `ProvideInherent`.
      - Extended (Extra data): Gossiped, validated via `TransactionExtension`.
        - Signed transactions (with a hardcoded signature) in extrinsic v4.
      - General transactions (without a hardcoded signature) in extrinsic v5.
      
      `TransactionExtension` differs from `SignedExtension` because:
      - A signature on the underlying transaction may validly not be present.
      - It may alter the origin during validation.
      - `pre_dispatch` is renamed to `prepare` and need not contain the checks
      present in `validate`.
      - `validate` and `prepare` is passed an `Origin` rather than a
      `AccountId`.
      - `validate` may pass arbitrary information into `prepare` via a new
      user-specifiable type `Val`.
      - `AdditionalSigned`/`additional_signed` is renamed to
      `Implicit`/`implicit`. It is encoded *for the entire transaction* and
      passed in to each extension as a new argument to `validate`. This
      facilitates the ability of extensions to acts as underlying crypto.
      
      There is a new `DispatchTransaction` trait which contains only default
      function impls and is impl'ed for any `TransactionExtension` impler. It
      provides several utility functions which reduce some of the tedium from
      using `TransactionExtension` (indeed, none of its regular functions
      should now need to be called directly).
      
      Three transaction version discriminator ("versions") are now permissible
      (RFC [here](https://github.com/polkadot-fellows/RFCs/pull/84)) in
      extrinsic version 5:
      - 0b00000100 or 0b00000101: Bare (used to be called "Unsigned"):
      contains Signature or Extra (extension data). After bare transactions
      are no longer supported, this will strictly identify an Inherents only.
      Available in both extrinsic versions 4 and 5.
      - 0b10000100: Old-school "Signed" Transaction: contains Signature, Extra
      (extension data) and an extension version byte, introduced as part of
      [RFC99](https://github.com/polkadot-fellows/RFCs/blob/main/text/0099-transaction-extension-version.md).
      Still available as part of extrinsic v4.
      - 0b01000101: New-school "General" Transaction: contains Extra
      (extension data) and an extension version byte, as per RFC99, but no
      Signature. Only available in extrinsic v5.
      
      For the New-school General Transaction, it becomes trivial for authors
      to publish extensions to the mechanism for authorizing an Origin, e.g.
      through new kinds of key-signing schemes, ZK proofs, pallet state,
      mutations over pre-authenticated origins or any combination of the
      above.
      
      `UncheckedExtrinsic` still maintains encode/decode backwards
      compatibility with extrinsic version 4, where the first byte was encoded
      as:
      - 0b00000100 - Unsigned transactions
      - 0b10000100 - Old-school Signed transactions, without the extension
      version byte
      
      Now, `UncheckedExtrinsic` contains a `Preamble` and the actual call. The
      `Preamble` describes the type of extrinsic as follows:
      ```rust
      /// A "header" for extrinsics leading up to the call itself. Determines the type of extrinsic and
      /// holds any necessary specialized data.
      #[derive(Eq, PartialEq, Clone)]
      pub enum Preamble<Address, Signature, Extension> {
      	/// An extrinsic without a signature or any extension. This means it's either an inherent or
      	/// an old-school "Unsigned" (we don't use that terminology any more since it's confusable with
      	/// the general transaction which is without a signature but does have an extension).
      	///
      	/// NOTE: In the future, once we remove `ValidateUnsigned`, this will only serve Inherent
      	/// extrinsics and thus can be renamed to `Inherent`.
      	Bare(ExtrinsicVersion),
      	/// An old-school transaction extrinsic which includes a signature of some hard-coded crypto.
      	/// Available only on extrinsic version 4.
      	Signed(Address, Signature, ExtensionVersion, Extension),
      	/// A new-school transaction extrinsic which does not include a signature by default. The
      	/// origin authorization, through signatures or other means, is performed by the transaction
      	/// extension in this extrinsic. Available starting with extrinsic version 5.
      	General(ExtensionVersion, Extension),
      }
      ```
      
      ## Code Migration
      
      ### NOW: Getting it to build
      
      Wrap your `SignedExtension`s in `AsTransactionExtension`. This should be
      accompanied by renaming your aggregate type in line with the new
      terminology. E.g. Before:
      
      ```rust
      /// The SignedExtension to the basic transaction logic.
      pub type SignedExtra = (
      	/* snip */
      	MySpecialSignedExtension,
      );
      /// Unchecked extrinsic type as expected by this runtime.
      pub type UncheckedExtrinsic =
      	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
      ```
      
      After:
      
      ```rust
      /// The extension to the basic transaction logic.
      pub type TxExtension = (
      	/* snip */
      	AsTransactionExtension<MySpecialSignedExtension>,
      );
      /// Unchecked extrinsic type as expected by this runtime.
      pub type UncheckedExtrinsic =
      	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, TxExtension>;
      ```
      
      You'll also need to alter any transaction building logic to add a
      `.into()` to make the conversion happen. E.g. Before:
      
      ```rust
      fn construct_extrinsic(
      		/* snip */
      ) -> UncheckedExtrinsic {
      	let extra: SignedExtra = (
      		/* snip */
      		MySpecialSignedExtension::new(/* snip */),
      	);
      	let payload = SignedPayload::new(call.clone(), extra.clone()).unwrap();
      	let signature = payload.using_encoded(|e| sender.sign(e));
      	UncheckedExtrinsic::new_signed(
      		/* snip */
      		Signature::Sr25519(signature),
      		extra,
      	)
      }
      ```
      
      After:
      
      ```rust
      fn construct_extrinsic(
      		/* snip */
      ) -> UncheckedExtrinsic {
      	let tx_ext: TxExtension = (
      		/* snip */
      		MySpecialSignedExtension::new(/* snip */).into(),
      	);
      	let payload = SignedPayload::new(call.clone(), tx_ext.clone()).unwrap();
      	let signature = payload.using_encoded(|e| sender.sign(e));
      	UncheckedExtrinsic::new_signed(
      		/* snip */
      		Signature::Sr25519(signature),
      		tx_ext,
      	)
      }
      ```
      
      ### SOON: Migrating to `TransactionExtension`
      
      Most `SignedExtension`s can be trivially converted to become a
      `TransactionExtension`. There are a few things to know.
      
      - Instead of a single trait like `SignedExtension`, you should now
      implement two traits individually: `TransactionExtensionBase` and
      `TransactionExtension`.
      - Weights are now a thing and must be provided via the new function `fn
      weight`.
      
      #### `TransactionExtensionBase`
      
      This trait takes care of anything which is not dependent on types
      specific to your runtime, most notably `Call`.
      
      - `AdditionalSigned`/`additional_signed` is renamed to
      `Implicit`/`implicit`.
      - Weight must be returned by implementing the `weight` function. If your
      extension is associated with a pallet, you'll probably want to do this
      via the pallet's existing benchmarking infrastructure.
      
      #### `TransactionExtension`
      
      Generally:
      - `pre_dispatch` is now `prepare` and you *should not reexecute the
      `validate` functionality in there*!
      - You don't get an account ID any more; you get an origin instead. If
      you need to presume an account ID, then you can use the trait function
      `AsSystemOriginSigner::as_system_origin_signer`.
      - You get an additional ticket, similar to `Pre`, called `Val`. This
      defines data which is passed from `validate` into `prepare`. This is
      important since you should not be duplicating logic from `validate` to
      `prepare`, you need a way of passing your working from the former into
      the latter. This is it.
      - This trait takes a `Call` type parameter. `Call` is the runtime call
      type which used to be an associated type; you can just move it to become
      a type parameter for your trait impl.
      - There's no `AccountId` associated type any more. Just remove it.
      
      Regarding `validate`:
      - You get three new parameters in `validate`; all can be ignored when
      migrating from `SignedExtension`.
      - `validate` returns a tuple on success; the second item in the tuple is
      the new ticket type `Self::Val` which gets passed in to `prepare`. If
      you use any information extracted during `validate` (off-chain and
      on-chain, non-mutating) in `prepare` (on-chain, mutating) then you can
      pass it through with this. For the tuple's last item, just return the
      `origin` argument.
      
      Regarding `prepare`:
      - This is renamed from `pre_dispatch`, but there is one change:
      - FUNCTIONALITY TO VALIDATE THE TRANSACTION NEED NOT BE DUPLICATED FROM
      `validate`!!
      - (This is different to `SignedExtension` which was required to run the
      same checks in `pre_dispatch` as in `validate`.)
      
      Regarding `post_dispatch`:
      - Since there are no unsigned transactions handled by
      `TransactionExtension`, `Pre` is always defined, so the first parameter
      is `Self::Pre` rather than `Option<Self::Pre>`.
      
      If you make use of `SignedExtension::validate_unsigned` or
      `SignedExtension::pre_dispatch_unsigned`, then:
      - Just use the regular versions of these functions instead.
      - Have your logic execute in the case that the `origin` is `None`.
      - Ensure your transaction creation logic creates a General Transaction
      rather than a Bare Transaction; this means having to include all
      `TransactionExtension`s' data.
      - `ValidateUnsigned` can still be used (for now) if you need to be able
      to construct transactions which contain none of the extension data,
      however these will be phased out in stage 2 of the Transactions Horizon,
      so you should consider moving to an extension-centric design.
      
      ---------
      
      Signed-off-by: default avatargeorgepisaltu <george.pisaltu@parity.io>
      Co-authored-by: default avatarGuillaume Thiolliere <gui.thiolliere@gmail.com>
      Co-authored-by: default avatarBranislav Kontur <bkontur@gmail.com>
  23. Oct 16, 2024
    • Alexandru Vasile's avatar
      litep2p/discovery: Fix memory leak in `litep2p.public_addresses()` (#5998) · 38aa4b75
      Alexandru Vasile authored
      This PR ensures that the `litep2p.public_addresses()` never grows
      indefinitely.
      - effectively fixes subtle memory leaks
      - fixes authority DHT records being dropped due to size limits being
      exceeded
      - provides a healthier subset of public addresses to the `/identify`
      protocol
      
      This PR adds a new `ExternalAddressExpired` event to the
      litep2p/discovery process.
      
      Substrate uses an LRU `address_confirmations` bounded to 32 address
      entries.
      The oldest entry is propagated via the `ExternalAddressExpired` event
      when a new address is added to the list (if capacity is exceeded).
      
      The expired address is then removed from the
      `litep2p.public_addresses()`, effectively limiting its size to 32
      entries (the size of `address_confirmations` LRU).
      
      cc @paritytech/networking @alexggh
      
      
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
  24. Oct 15, 2024
    • Michal Kucharczyk's avatar
      fork-aware transaction pool added (#4639) · 26c11fc5
      Michal Kucharczyk authored
      ### Fork-Aware Transaction Pool Implementation
      
      This PR introduces a fork-aware transaction pool (fatxpool) enhancing
      transaction management by maintaining the valid state of txpool for
      different forks.
      
      ### High-level overview
      The high level overview was added to
      [`sc_transaction_pool::fork_aware_txpool`](https://github.com/paritytech/polkadot-sdk/blob/3ad0a1b7/substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs#L21)
      module. Use:
      ```
      cargo  doc --document-private-items -p sc-transaction-pool --open
      ```
      to build the doc. It should give a good overview and nice entry point
      into the new pool's mechanics.
      
      <details>
        <summary>Quick overview (documentation excerpt)</summary>
      
      #### View
      For every fork, a view is created. The view is a persisted state of the
      transaction pool computed and updated at the tip of the fork. The view
      is built around the existing `ValidatedPool` structure.
      
      A view is created on every new best block notification. To create a
      view, one of the existing views is chosen and cloned.
      
      When the chain progresses, the view is kept in the cache
      (`retracted_views`) to allow building blocks upon intermediary blocks in
      the fork.
      
      The views are deleted on finalization: views lower than the finalized
      block are removed.
      
      The views are updated with the transactions from the mempool—all
      transactions are sent to the newly created views.
      A maintain process is also executed for the newly created
      views—basically resubmitting and pruning transactions from the
      appropriate tree route.
      
      ##### View store
      View store is the helper structure that acts as a container for all the
      views. It provides some convenient methods.
      
      ##### Submitting transactions
      Every transaction is submitted to every view at the tips of the forks.
      Retracted views are not updated.
      Every transaction also goes into the mempool.
      
      ##### Internal mempool
      Shortly, the main purpose of an internal mempool is to prevent a
      transaction from being lost. That could happen when a transaction is
      invalid on one fork and could be valid on another. It also allows the
      txpool to accept transactions when no blocks have been reported yet.
      
      The mempool removes its transactions when they get finalized.
      Transactions are also periodically verified on every finalized event and
      removed from the mempool if no longer valid.
      
      #### Events
      Transaction events from multiple views are merged and filtered to avoid
      duplicated events.
      `Ready` / `Future` / `Inblock` events are originated in the Views and
      are de-duplicated and forwarded to external listeners.
      `Finalized` events are originated in fork-aware-txpool logic.
      `Invalid` events requires special care and can be originated in both
      view and fork-aware-txpool logic.
      
      #### Light maintain
      Sometime transaction pool does not have enough time to prepare fully
      maintained view with all retracted transactions being revalidated. To
      avoid providing empty ready transaction set to block builder (what would
      result in empty block) the light maintain was implemented. It simply
      removes the imported transactions from ready iterator.
      
      #### Revalidation
      Revalidation is performed for every view. The revalidation process is
      started after a trigger is executed. The revalidation work is terminated
      just after a new best block / finalized event is notified to the
      transaction pool.
      The revalidation result is applied to the newly created view which is
      built upon the revalidated view.
      
      Additionally, parts of the mempool are also revalidated to make sure
      that no transactions are stuck in the mempool.
      
      
      #### Logs
      The most important log allowing to understand the state of the txpool
      is:
      ```
                    maintain: txs:(0, 92) views:[2;[(327, 76, 0), (326, 68, 0)]] event:Finalized { hash: 0x8...f, tree_route: [] }  took:3.463522ms
                                   ^   ^         ^     ^   ^  ^      ^   ^  ^        ^                                                   ^
      unwatched txs in mempool ────┘   │         │     │   │  │      │   │  │        │                                                   │
         watched txs in mempool ───────┘         │     │   │  │      │   │  │        │                                                   │
                           views  ───────────────┘     │   │  │      │   │  │        │                                                   │
                            1st view block # ──────────┘   │  │      │   │  │        │                                                   │
                                 number of ready tx ───────┘  │      │   │  │        │                                                   │
                                      numer of future tx ─────┘      │   │  │        │                                                   │
                                              2nd view block # ──────┘   │  │        │                                                   │
                                            number of ready tx ──────────┘  │        │                                                   │
                                                 number of future tx ───────┘        │                                                   │
                                                                       event ────────┘                                                   │
                                                                             duration  ──────────────────────────────────────────────────┘
      ```
      It is logged after the maintenance is done.
      
      The `debug` level enables per-transaction logging, allowing to keep
      track of all transaction-related actions that happened in txpool.
      </details>
      
      
      ### Integration notes
      
      For teams having a custom node, the new txpool needs to be instantiated,
      typically in `service.rs` file, here is an example:
      
      https://github.com/paritytech/polkadot-sdk/blob/9c547ff3
      
      /cumulus/polkadot-omni-node/lib/src/common/spec.rs#L152-L161
      
      To enable new transaction pool the following cli arg shall be specified:
      `--pool-type=fork-aware`. If it works, there shall be information
      printed in the log:
      ```
      2024-09-20 21:28:17.528  INFO main txpool: [Parachain]  creating ForkAware txpool.
      ````
      
      For debugging the following debugs shall be enabled:
      ```
            "-lbasic-authorship=debug",
            "-ltxpool=debug",
      ```
      *note:* trace for txpool enables per-transaction logging.
      
      ### Future work
      The current implementation seems to be stable, however further
      improvements are required.
      Here is the umbrella issue for future work:
      - https://github.com/paritytech/polkadot-sdk/issues/5472
      
      
      Partially fixes: #1202
      
      ---------
      
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
      Co-authored-by: default avatarSebastian Kunert <skunert49@gmail.com>
      Co-authored-by: default avatarIulian Barbu <14218860+iulianbarbu@users.noreply.github.com>
  25. Sep 26, 2024
  26. Sep 20, 2024
  27. Sep 19, 2024
    • Andrei Eres's avatar
      Use maximum allowed response size for request/response protocols (#5753) · 0c9d8fed
      Andrei Eres authored
      # Description
      
      Adjust the PoV response size to the default values used in the
      substrate.
      Fixes https://github.com/paritytech/polkadot-sdk/issues/5503
      
      ## Integration
      
      The changes shouldn't impact downstream projects since we are only
      increasing the limit.
      
      ## Review Notes
      
      You can't see it from the changes, but it affects all protocols that use
      the `POV_RESPONSE_SIZE` constant.
      - Protocol::ChunkFetchingV1
      - Protocol::ChunkFetchingV2
      - Protocol::CollationFetchingV1
      - Protocol::CollationFetchingV2
      - Protocol::PoVFetchingV1
      - Protocol::AvailableDataFetchingV1
      
      ## Increasing timeouts
      
      
      https://github.com/paritytech/polkadot-sdk/blob/fae15379
      
      /polkadot/node/network/protocol/src/request_response/mod.rs#L126-L129
      
      I assume the current PoV request timeout is set to 1.2s to handle 5
      consecutive requests during a 6s block. This setting does not relate to
      the PoV response size. I see no reason to change the current timeouts
      after adjusting the response size.
      
      However, we should consider networking speed limitations if we want to
      increase the maximum PoV size to 10 MB. With the number of parallel
      requests set to 10, validators will need the following networking
      speeds:
      - 5 MB PoV: at least 42 MB/s, ideally 50 MB/s.  
      - 10 MB PoV: at least 84 MB/s, ideally 100 MB/s.
      
      The current required speed of 50 MB/s aligns with the 62.5 MB/s
      specified [in the reference hardware
      requirements](https://wiki.polkadot.network/docs/maintain-guides-how-to-validate-polkadot#reference-hardware).
      Increasing the PoV size to 10 MB may require a higher networking speed.
      
      ---------
      
      Co-authored-by: default avatarAndrei Sandu <54316454+sandreim@users.noreply.github.com>
  28. Sep 18, 2024
  29. Sep 17, 2024
    • Nazar Mokrynskyi's avatar
      Syncing strategy refactoring (part 2) (#5666) · 43cd6fd4
      Nazar Mokrynskyi authored
      # Description
      
      Follow-up to https://github.com/paritytech/polkadot-sdk/pull/5469 and
      mostly covering https://github.com/paritytech/polkadot-sdk/issues/5333.
      
      The primary change here is that syncing strategy is no longer created
      inside of syncing engine, instead syncing strategy is an argument of
      syncing engine, more specifically it is an argument to `build_network`
      that most downstream users will use. This also extracts addition of
      request-response protocols outside of network construction, making sure
      they are physically not present when they don't need to be (imagine
      syncing strategy that uses none of Substrate's protocols in its
      implementation for example).
      
      This technically allows to completely replace syncing strategy with
      whatever strategy chain might need.
      
      There will be at least one follow-up PR that will simplify
      `SyncingStrategy` trait and other public interfaces to remove mentions
      of block/state/warp sync requests, replacing them with generic APIs,
      such that strategies where warp sync is not applicable don't have to
      provide dummy method implementations, etc.
      
      ## Integration
      
      Downstream projects will have to write a bit of boilerplate calling
      `build_polkadot_syncing_strategy` function to create previously default
      syncing strategy.
      
      ## Review Notes
      
      Please review PR through individual commits rather than the final diff,
      it will be easier that way. The changes are mostly just moving code
      around one step at a time.
      
      # 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)
  30. Sep 10, 2024
    • Alexandru Vasile's avatar
      litep2p: Update network backend to v0.7.0 (#5609) · 12eeb5df
      Alexandru Vasile authored
      
      This release introduces several new features, improvements, and fixes to
      the litep2p library. Key updates include enhanced error handling,
      configurable connection limits, and a new API for managing public
      addresses.
      
      For a detailed set of changes, see [litep2p
      changelog](https://github.com/paritytech/litep2p/blob/master/CHANGELOG.md#070---2024-09-05).
      
      This PR makes use of:
      - connection limits to optimize network throughput
      - better errors that are propagated to substrate metrics 
      - public addresses API to report healthy addresses to the Identify
      protocol
      
      ### Warp sync time improvement
      
      Measuring warp sync time is a bit inaccurate since the network is not
      deterministic and we might end up using faster peers (peers with more
      resources to handle our requests). However, I did not see warp sync
      times of 16 minutes, instead, they are roughly stabilized between 8 and
      10 minutes.
      
      For measuring warp-sync time, I've used
      [sub-trige-logs](https://github.com/lexnv/sub-triage-logs/?tab=readme-ov-file#warp-time)
      
      ### Litep2p
      
      Phase | Time
       -|-
      Warp  | 426.999999919s
      State | 99.000000555s
      Total | 526.000000474s
      
      ### Libp2p
      
      Phase | Time
       -|-
      Warp  | 731.999999837s
      State | 71.000000882s
      Total | 803.000000719s
      
      Closes: https://github.com/paritytech/polkadot-sdk/issues/4986
      
      
      ### Low peer count
      
      After exposing the `litep2p::public_addresses` interface, we can report
      to litep2p confirmed external addresses. This should mitigate or at
      least improve: https://github.com/paritytech/polkadot-sdk/issues/4925.
      Will keep the issue around to confirm this.
      
      
      ### Improved metrics
      
      We are one step closer to exposing similar metrics as libp2p:
      https://github.com/paritytech/polkadot-sdk/issues/4681.
      
      cc @paritytech/networking 
      
      ### Next Steps
      - [x] Use public address interface to confirm addresses to identify
      protocol
      
      ---------
      
      Signed-off-by: default avatarAlexandru Vasile <alexandru.vasile@parity.io>
    • Nazar Mokrynskyi's avatar
      Syncing strategy refactoring (#5469) · 1f1f20a8
      Nazar Mokrynskyi authored
      This is a step towards
      https://github.com/paritytech/polkadot-sdk/issues/5333
      
      The commits with code moving (but no other changes) and actual changes
      are separated for easier review.
      
      Essentially this results in `SyncingStrategy` trait replacing struct
      (which is renamed to `PolkadotSyncingStrategy`, open for better name
      suggestions). Technically it is already possible to replace
      `PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>`
      in syncing engine, but I decided to postpone such change until we
      actually have an ability to customize it. It should also be possible to
      swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports
      regular full sync from genesis (it also implements `SyncingStrategy`
      trait).
      
      While extracted trait still has a lot of non-generic stuff in it like
      exposed knowledge of warp sync and `StrategyKey` with hardcoded set of
      options, I believe this is a step in the right direction and will
      address those in upcoming PRs.
      
      With https://github.com/paritytech/polkadot-sdk/pull/5431 that landed
      earlier warp sync configuration is more straightforward, but there are
      still numerous things interleaved that will take some time to abstract
      away nicely and expose in network config for developers. For now this is
      an internal change even though data structures are technically public
      and require major version bump.
    • Liu-Cheng Xu's avatar
      Fix edge case where state sync is not triggered (#5635) · 8236718e
      Liu-Cheng Xu authored
      This PR addresses an issue where state sync may fail to start if the
      conditions required for its initiation are not met when a finalized
      block notification is received. `pending_state_sync_attempt` is
      introduced to trigger the state sync later when the conditions are
      satisfied.
      
      This issue was spotted when I worked on #5406, specifically,
      `queue_blocks` was not empty when the finalized block notification was
      received, and then the state sync was stalled. cc @dmitry-markin
      
      
      
      ---------
      
      Co-authored-by: default avatarDmitry Markin <dmitry@markin.tech>
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
  31. Sep 06, 2024
    • Liu-Cheng Xu's avatar
      Introduce `BlockGap` (#5592) · fdb4554e
      Liu-Cheng Xu authored
      Previously, block gaps could only be created by warp sync, but block
      gaps will also be generated by fast sync once #5406 is fixed. This PR is
      part 1 of the detailed implementation plan in
      https://github.com/paritytech/polkadot-sdk/issues/5406#issuecomment-2325064863:
      refactor `BlockGap`.
      
      This refactor converts the existing `(NumberFor<Block>,
      NumberFor<Block>)` into a dedicated `BlockGap<NumberFor<Block>>` struct.
      This change is purely structural and does not alter existing logic, but
      lays the groundwork for the follow-up PR.
      
      The compatibility concern caused by the new structure is addressed in
      the second commit.
      
      cc @dmitry-markin
      
      
      
      ---------
      
      Co-authored-by: default avatarBastian Köcher <git@kchr.de>
  32. Sep 02, 2024
    • Nazar Mokrynskyi's avatar
      Improve `sc-service` API (#5364) · da654103
      Nazar Mokrynskyi authored
      
      This improves `sc-service` API by not requiring the whole
      `&Configuration`, using specific configuration options instead.
      `RpcConfiguration` was also extracted from `Configuration` to group all
      RPC options together.
      
      We don't use Substrate's CLI and would rather not use `Configuration`
      either, but some key public functions require it even though they
      ignored most of the fields anyway.
      
      `RpcConfiguration` is very helpful not just for consolidation of the
      fields, but also to finally make RPC optional for our use case, while
      Substrate still runs RPC server on localhost even if listening address
      is explicitly set to `None`, which is annoying (and I suspect there is a
      reason for it, so didn't want to change the default just yet).
      
      While this is a breaking change, most developers will not notice it if
      they use higher-level APIs.
      
      Fixes https://github.com/paritytech/polkadot-sdk/issues/2897
      
      ---------
      
      Co-authored-by: default avatarNiklas Adolfsson <niklasadolfsson1@gmail.com>