From 41b6915ecb4b5691cdeeb585e26d46c4897ae151 Mon Sep 17 00:00:00 2001 From: jpserrat <35823283+jpserrat@users.noreply.github.com> Date: Mon, 25 Nov 2024 12:54:04 -0300 Subject: [PATCH 1/8] remove ReportCollator message (#6628) Closes #6415 # Description Remove unused message `ReportCollator` and test related to this message on the collator protocol validator side. cc: @tdimitrov --------- Co-authored-by: Tsvetomir Dimitrov Co-authored-by: command-bot <> --- .../src/collator_side/mod.rs | 2 +- .../src/validator_side/mod.rs | 3 - .../src/validator_side/tests/mod.rs | 60 ------------------- polkadot/node/subsystem-types/src/messages.rs | 15 ++--- .../src/node/collators/collator-protocol.md | 6 -- .../src/node/subsystems-and-jobs.md | 7 +-- .../src/types/overseer-protocol.md | 3 - prdoc/pr_6628.prdoc | 12 ++++ 8 files changed, 20 insertions(+), 88 deletions(-) create mode 100644 prdoc/pr_6628.prdoc diff --git a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs index 504b0d716043..d77480272cb4 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -899,7 +899,7 @@ async fn process_msg( ); } }, - msg @ (ReportCollator(..) | Invalid(..) | Seconded(..)) => { + msg @ (Invalid(..) | Seconded(..)) => { gum::warn!( target: LOG_TARGET, "{:?} message is not expected on the collator side of the protocol", diff --git a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs index 86358f503d04..36ec959c3406 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -1462,9 +1462,6 @@ async fn process_msg( "DistributeCollation message is not expected on the validator side of the protocol", ); }, - ReportCollator(id) => { - report_collator(&mut state.reputation, ctx.sender(), &state.peer_data, id).await; - }, NetworkBridgeUpdate(event) => { if let Err(e) = handle_network_msg(ctx, state, keystore, event).await { gum::warn!( diff --git a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs index 7bc61dd4ebec..f2f23c188a66 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs @@ -638,66 +638,6 @@ fn act_on_advertisement_v2() { }); } -// Test that other subsystems may modify collators' reputations. -#[test] -fn collator_reporting_works() { - let test_state = TestState::default(); - - test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { - let TestHarness { mut virtual_overseer, .. } = test_harness; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![test_state.relay_parent], - )), - ) - .await; - - respond_to_runtime_api_queries(&mut virtual_overseer, &test_state, test_state.relay_parent) - .await; - - let peer_b = PeerId::random(); - let peer_c = PeerId::random(); - - connect_and_declare_collator( - &mut virtual_overseer, - peer_b, - test_state.collators[0].clone(), - test_state.chain_ids[0], - CollationVersion::V1, - ) - .await; - - connect_and_declare_collator( - &mut virtual_overseer, - peer_c, - test_state.collators[1].clone(), - test_state.chain_ids[0], - CollationVersion::V1, - ) - .await; - - overseer_send( - &mut virtual_overseer, - CollatorProtocolMessage::ReportCollator(test_state.collators[0].public()), - ) - .await; - - assert_matches!( - overseer_recv(&mut virtual_overseer).await, - AllMessages::NetworkBridgeTx( - NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(peer, rep)), - ) => { - assert_eq!(peer, peer_b); - assert_eq!(rep.value, COST_REPORT_BAD.cost_or_benefit()); - } - ); - - virtual_overseer - }); -} - // Test that we verify the signatures on `Declare` and `AdvertiseCollation` messages. #[test] fn collator_authentication_verification_works() { diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 28a3a1ab82ab..b541f9519219 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -48,12 +48,12 @@ use polkadot_primitives::{ CommittedCandidateReceiptV2 as CommittedCandidateReceipt, CoreState, }, ApprovalVotingParams, AuthorityDiscoveryId, BlockNumber, CandidateCommitments, CandidateHash, - CandidateIndex, CollatorId, CoreIndex, DisputeState, ExecutorParams, GroupIndex, - GroupRotationInfo, Hash, HeadData, Header as BlockHeader, Id as ParaId, InboundDownwardMessage, - InboundHrmpMessage, MultiDisputeStatementSet, NodeFeatures, OccupiedCoreAssumption, - PersistedValidationData, PvfCheckStatement, PvfExecKind as RuntimePvfExecKind, SessionIndex, - SessionInfo, SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, - ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, + CandidateIndex, CoreIndex, DisputeState, ExecutorParams, GroupIndex, GroupRotationInfo, Hash, + HeadData, Header as BlockHeader, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, + MultiDisputeStatementSet, NodeFeatures, OccupiedCoreAssumption, PersistedValidationData, + PvfCheckStatement, PvfExecKind as RuntimePvfExecKind, SessionIndex, SessionInfo, + SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, + ValidatorId, ValidatorIndex, ValidatorSignature, }; use polkadot_statement_table::v2::Misbehavior; use std::{ @@ -250,9 +250,6 @@ pub enum CollatorProtocolMessage { /// The core index where the candidate should be backed. core_index: CoreIndex, }, - /// Report a collator as having provided an invalid collation. This should lead to disconnect - /// and blacklist of the collator. - ReportCollator(CollatorId), /// Get a network bridge update. #[from] NetworkBridgeUpdate(NetworkBridgeEvent), diff --git a/polkadot/roadmap/implementers-guide/src/node/collators/collator-protocol.md b/polkadot/roadmap/implementers-guide/src/node/collators/collator-protocol.md index 432d9ab69bab..586a4169b5bc 100644 --- a/polkadot/roadmap/implementers-guide/src/node/collators/collator-protocol.md +++ b/polkadot/roadmap/implementers-guide/src/node/collators/collator-protocol.md @@ -151,12 +151,6 @@ time per relay parent. This reduces the bandwidth requirements and as we can sec the others are probably not required anyway. If the request times out, we need to note the collator as being unreliable and reduce its priority relative to other collators. -As a validator, once the collation has been fetched some other subsystem will inspect and do deeper validation of the -collation. The subsystem will report to this subsystem with a [`CollatorProtocolMessage`][CPM]`::ReportCollator`. In -that case, if we are connected directly to the collator, we apply a cost to the `PeerId` associated with the collator -and potentially disconnect or blacklist it. If the collation is seconded, we notify the collator and apply a benefit to -the `PeerId` associated with the collator. - ### Interaction with [Candidate Backing][CB] As collators advertise the availability, a validator will simply second the first valid parablock candidate per relay diff --git a/polkadot/roadmap/implementers-guide/src/node/subsystems-and-jobs.md b/polkadot/roadmap/implementers-guide/src/node/subsystems-and-jobs.md index a3ca7347eb63..a96f3fa3d4a0 100644 --- a/polkadot/roadmap/implementers-guide/src/node/subsystems-and-jobs.md +++ b/polkadot/roadmap/implementers-guide/src/node/subsystems-and-jobs.md @@ -129,7 +129,6 @@ digraph { cand_sel -> coll_prot [arrowhead = "diamond", label = "FetchCollation"] cand_sel -> cand_back [arrowhead = "onormal", label = "Second"] - cand_sel -> coll_prot [arrowhead = "onormal", label = "ReportCollator"] cand_val -> runt_api [arrowhead = "diamond", label = "Request::PersistedValidationData"] cand_val -> runt_api [arrowhead = "diamond", label = "Request::ValidationCode"] @@ -231,7 +230,7 @@ sequenceDiagram VS ->> CandidateSelection: Collation - Note over CandidateSelection: Lots of other machinery in play here,
but there are only three outcomes from the
perspective of the `CollatorProtocol`: + Note over CandidateSelection: Lots of other machinery in play here,
but there are only two outcomes from the
perspective of the `CollatorProtocol`: alt happy path CandidateSelection -->> VS: FetchCollation @@ -242,10 +241,6 @@ sequenceDiagram NB ->> VS: Collation Deactivate VS - else collation invalid or unexpected - CandidateSelection ->> VS: ReportCollator - VS ->> NB: ReportPeer - else CandidateSelection already selected a different candidate Note over CandidateSelection: silently drop end diff --git a/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md b/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md index 85415e42a11c..cb862440727b 100644 --- a/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md +++ b/polkadot/roadmap/implementers-guide/src/types/overseer-protocol.md @@ -436,9 +436,6 @@ enum CollatorProtocolMessage { DistributeCollation(CandidateReceipt, PoV, Option>), /// Fetch a collation under the given relay-parent for the given ParaId. FetchCollation(Hash, ParaId, ResponseChannel<(CandidateReceipt, PoV)>), - /// Report a collator as having provided an invalid collation. This should lead to disconnect - /// and blacklist of the collator. - ReportCollator(CollatorId), /// Note a collator as having provided a good collation. NoteGoodCollation(CollatorId, SignedFullStatement), /// Notify a collator that its collation was seconded. diff --git a/prdoc/pr_6628.prdoc b/prdoc/pr_6628.prdoc new file mode 100644 index 000000000000..7ea0c4968385 --- /dev/null +++ b/prdoc/pr_6628.prdoc @@ -0,0 +1,12 @@ +title: "Remove ReportCollator message" + +doc: + - audience: Node Dev + description: | + Remove unused message ReportCollator and test related to this message on the collator protocol validator side. + +crates: + - name: polkadot-node-subsystem-types + bump: patch + - name: polkadot-collator-protocol + bump: major \ No newline at end of file From 6d5f8141ad42d7a528a09715c492974550709b92 Mon Sep 17 00:00:00 2001 From: Tarek Mohamed Abdalla Date: Mon, 25 Nov 2024 19:33:32 +0200 Subject: [PATCH 2/8] rpc server: fix subscription id_provider being reset to default one. (#6588) # Description The PR ensures that the id_provider variable is cloned instead of taken, which can help prevent issues related id provider being reset to the default. In [a test in moonbeam](https://github.com/moonbeam-foundation/moonbeam/blob/c6d07d703dfcdd94cc311fa83b553071b7d433ff/test/suites/dev/moonbase/test-subscription/test-subscription.ts#L20-L31) we found that the id_provider is being reset somehow and changed to the default one. Changing .take() to .clone() would fix the issue. # 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. * [ ] I have made corresponding changes to the documentation (if applicable) * [ ] I have added tests that prove my fix is effective or that my feature works (if applicable) --------- Co-authored-by: Niklas Adolfsson --- prdoc/pr_6588.prdoc | 14 ++++++++++++++ substrate/client/rpc-servers/src/lib.rs | 7 +++---- 2 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 prdoc/pr_6588.prdoc diff --git a/prdoc/pr_6588.prdoc b/prdoc/pr_6588.prdoc new file mode 100644 index 000000000000..bf44b2ed3784 --- /dev/null +++ b/prdoc/pr_6588.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "rpc server: fix subscription id_provider being reset to default one" + +doc: + - audience: Node Dev + description: | + The modification ensures that the id_provider variable is cloned instead of taken, which can help prevent issues related id provider being reset to the default. + + +crates: + - name: sc-rpc-server + bump: patch \ No newline at end of file diff --git a/substrate/client/rpc-servers/src/lib.rs b/substrate/client/rpc-servers/src/lib.rs index 31e4042d81f2..ff21e2da768e 100644 --- a/substrate/client/rpc-servers/src/lib.rs +++ b/substrate/client/rpc-servers/src/lib.rs @@ -144,7 +144,7 @@ where local_addrs.push(local_addr); let cfg = cfg.clone(); - let mut id_provider2 = id_provider.clone(); + let id_provider2 = id_provider.clone(); tokio_handle.spawn(async move { loop { @@ -197,10 +197,9 @@ where .set_http_middleware(http_middleware) .set_message_buffer_capacity(max_buffer_capacity_per_connection) .set_batch_request_config(batch_config) - .custom_tokio_runtime(cfg.tokio_handle.clone()) - .set_id_provider(RandomStringIdProvider::new(16)); + .custom_tokio_runtime(cfg.tokio_handle.clone()); - if let Some(provider) = id_provider2.take() { + if let Some(provider) = id_provider2.clone() { builder = builder.set_id_provider(provider); } else { builder = builder.set_id_provider(RandomStringIdProvider::new(16)); From 7f64e66b3b01ff5e391e42a8159f601cbbeb46d8 Mon Sep 17 00:00:00 2001 From: Javier Viola <363911+pepoviola@users.noreply.github.com> Date: Tue, 26 Nov 2024 09:25:18 +0100 Subject: [PATCH 3/8] bump zombienet-sdk version `v0.2.16` (#6633) Fix #6575. cc: @iulianbarbu Co-authored-by: Iulian Barbu <14218860+iulianbarbu@users.noreply.github.com> --- Cargo.lock | 24 ++++++++++++------------ Cargo.toml | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c79adee6f38e..338d5025b662 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -31910,9 +31910,9 @@ dependencies = [ [[package]] name = "zombienet-configuration" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d7a8cc4f8e8bb3f40757b62d3b054da5c95f43321c775eb321edc89d431583e" +checksum = "8ad4fc5b0f1aa54de6bf2d6771c449b41cad47e1cf30559af0a71452686b47ab" dependencies = [ "anyhow", "lazy_static", @@ -31930,9 +31930,9 @@ dependencies = [ [[package]] name = "zombienet-orchestrator" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d32fa87851f41443a78971bd7110274f9a66d139ac834de159adc08f90cf8e3" +checksum = "e4a7dd25842ded75c7f4dc4f38f05fef567bd0b37fd3057c223d4ee34d8fa817" dependencies = [ "anyhow", "async-trait", @@ -31963,9 +31963,9 @@ dependencies = [ [[package]] name = "zombienet-prom-metrics-parser" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9acb9c94bc7c2c83f8eb8e26ed403f757af1632f22b89394d8876412ede990ca" +checksum = "a63e0c6024dd19b0f8b28afa94f78c211e5c163350ecda4a48084532d74d7cfe" dependencies = [ "pest", "pest_derive", @@ -31974,9 +31974,9 @@ dependencies = [ [[package]] name = "zombienet-provider" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc8f3f71d4d974fc4a2262fa9293c2eedc423540378bd7c1dc1b66cc95d1d1af" +checksum = "8d87c29390a342d0f4f62b6796861fb82e0e56c49929a272b689e8dbf24eaab9" dependencies = [ "anyhow", "async-trait", @@ -32005,9 +32005,9 @@ dependencies = [ [[package]] name = "zombienet-sdk" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5dbfddce7a6100cdc930b93301f1b6381e6577ecc013d6802258ea6902a2bebd" +checksum = "829e5111182caf00ba57cd63656cf0bde6ce6add7f6a9747d15821c202a3f27e" dependencies = [ "async-trait", "futures", @@ -32022,9 +32022,9 @@ dependencies = [ [[package]] name = "zombienet-support" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d20567c52b4fd46b600cda254dedb6a6dc30cabf512de91e4f6f78f0f7f4644b" +checksum = "99568384a1d9645458ab9de377b3517cb543a1ece5aba905aeb58d269139df4e" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 533ea4c9e878..57e049885da5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1387,7 +1387,7 @@ xcm-procedural = { path = "polkadot/xcm/procedural", default-features = false } xcm-runtime-apis = { path = "polkadot/xcm/xcm-runtime-apis", default-features = false } xcm-simulator = { path = "polkadot/xcm/xcm-simulator", default-features = false } zeroize = { version = "1.7.0", default-features = false } -zombienet-sdk = { version = "0.2.15" } +zombienet-sdk = { version = "0.2.16" } zstd = { version = "0.12.4", default-features = false } [profile.release] From 86a917f59d04ea3b399b34edcb8effe76b6415e6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 26 Nov 2024 08:49:46 +0000 Subject: [PATCH 4/8] Bump rustls from 0.23.14 to 0.23.18 (#6641) Bumps [rustls](https://github.com/rustls/rustls) from 0.23.14 to 0.23.18.
Commits
  • 33af2c3 Prepare 0.23.18
  • ffe646d Add reproducer for bug 2227
  • 69b6f74 Record and restore the processed cursor in first_handshake_message
  • 4ef3532 Upgrade to mio 1
  • 092a164 Manage dependencies via the workspace
  • a01bd6b rustls-bench: fix warnings with no features
  • 7d74de2 tests: linearize new test code helper
  • 499d797 fix: do not send session_ticket(35) extension for TLS 1.3
  • faca289 chore(deps): lock file maintenance
  • d12f423 fix(deps): update rust crate asn1 to 0.20
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=rustls&package-manager=cargo&previous-version=0.23.14&new-version=0.23.18)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/paritytech/polkadot-sdk/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 28 ++++++++++++++-------------- Cargo.toml | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 338d5025b662..c2d2eb3e9644 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8530,7 +8530,7 @@ dependencies = [ "hyper 1.3.1", "hyper-util", "log", - "rustls 0.23.14", + "rustls 0.23.18", "rustls-native-certs 0.8.0", "rustls-pki-types", "tokio", @@ -9145,7 +9145,7 @@ dependencies = [ "http 1.1.0", "jsonrpsee-core", "pin-project", - "rustls 0.23.14", + "rustls 0.23.18", "rustls-pki-types", "rustls-platform-verifier", "soketto 0.8.0", @@ -9198,7 +9198,7 @@ dependencies = [ "hyper-util", "jsonrpsee-core", "jsonrpsee-types", - "rustls 0.23.14", + "rustls 0.23.18", "rustls-platform-verifier", "serde", "serde_json", @@ -20609,7 +20609,7 @@ dependencies = [ "quinn-proto 0.11.8", "quinn-udp 0.5.4", "rustc-hash 2.0.0", - "rustls 0.23.14", + "rustls 0.23.18", "socket2 0.5.7", "thiserror", "tokio", @@ -20643,7 +20643,7 @@ dependencies = [ "rand", "ring 0.17.7", "rustc-hash 2.0.0", - "rustls 0.23.14", + "rustls 0.23.18", "slab", "thiserror", "tinyvec", @@ -21130,7 +21130,7 @@ dependencies = [ "percent-encoding", "pin-project-lite", "quinn 0.11.5", - "rustls 0.23.14", + "rustls 0.23.18", "rustls-pemfile 2.0.0", "rustls-pki-types", "serde", @@ -21734,9 +21734,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.14" +version = "0.23.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "415d9944693cb90382053259f89fbb077ea730ad7273047ec63b19bc9b160ba8" +checksum = "9c9cc1d47e243d655ace55ed38201c19ae02c148ae56412ab8750e8f0166ab7f" dependencies = [ "log", "once_cell", @@ -21806,9 +21806,9 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.9.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e696e35370c65c9c541198af4543ccd580cf17fc25d8e05c5a242b202488c55" +checksum = "16f1201b3c9a7ee8039bcadc17b7e605e2945b27eee7631788c1bd2b0643674b" [[package]] name = "rustls-platform-verifier" @@ -21821,7 +21821,7 @@ dependencies = [ "jni", "log", "once_cell", - "rustls 0.23.14", + "rustls 0.23.18", "rustls-native-certs 0.7.0", "rustls-platform-verifier-android", "rustls-webpki 0.102.8", @@ -23128,7 +23128,7 @@ dependencies = [ "parity-scale-codec", "parking_lot 0.12.3", "rand", - "rustls 0.23.14", + "rustls 0.23.18", "sc-block-builder", "sc-client-api", "sc-client-db", @@ -29529,7 +29529,7 @@ version = "0.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c7bc40d0e5a97695bb96e27995cd3a08538541b0a846f65bba7a359f36700d4" dependencies = [ - "rustls 0.23.14", + "rustls 0.23.18", "rustls-pki-types", "tokio", ] @@ -30240,7 +30240,7 @@ dependencies = [ "flate2", "log", "once_cell", - "rustls 0.23.14", + "rustls 0.23.18", "rustls-pki-types", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 57e049885da5..53f95406e79a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1136,7 +1136,7 @@ rstest = { version = "0.18.2" } rustc-hash = { version = "1.1.0" } rustc-hex = { version = "2.1.0", default-features = false } rustix = { version = "0.36.7", default-features = false } -rustls = { version = "0.23.14", default-features = false, features = ["logging", "ring", "std", "tls12"] } +rustls = { version = "0.23.18", default-features = false, features = ["logging", "ring", "std", "tls12"] } rustversion = { version = "1.0.17" } rusty-fork = { version = "0.3.0", default-features = false } safe-mix = { version = "1.0", default-features = false } From 8216235b480630c5a69636b172f5146711c88c9a Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:58:48 +0200 Subject: [PATCH 5/8] ci/check-semver: Fix semver failed step (#6535) The semver-check is failing with the following [error](https://github.com/paritytech/polkadot-sdk/actions/runs/11908981132/job/33185572284): ```bash error[E0658]: use of unstable library feature 'error_in_core' --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/frame-decode-0.5.0/src/decoding/extrinsic_decoder.rs:56:6 | 56 | impl core::error::Error for ExtrinsicDecodeError {} | ^^^^^^^^^^^^^^^^^^ | = note: see issue #103765 for more information = help: add `#![feature(error_in_core)]` to the crate attributes to enable = note: this compiler was built on 2024-05-31; consider upgrading it if it is out of date ``` This is related to the toolchain nightly version 1.80. In rust, 1.81 the `core::error::Error` is stable. After updating the rust-toolchain, parity-publish crate must be updated as well. The `cargo-semver-checks` dependency of `parity-publish` crate is updated from 0.34 to 0.38. This update enables rustdoc v36 that fixes the following [issue](https://github.com/paritytech/polkadot-sdk/actions/runs/11912689841/job/33196936011): ```bash validating prdocs... checking file changes... checking semver changes... (1/18) building frame-support-HEAD... (2/18) building frame-support-28.0.0... Error: rustdoc format v36 for file /__w/polkadot-sdk/polkadot-sdk/target/doc/frame_support.new is not supported ``` This PR is pending on a release of parity-publish to version 0.9.0 (fixes already on origin/master) --------- Signed-off-by: Alexandru Vasile --- .github/workflows/check-semver.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-semver.yml b/.github/workflows/check-semver.yml index 78602410cdf6..8d77b6a31b75 100644 --- a/.github/workflows/check-semver.yml +++ b/.github/workflows/check-semver.yml @@ -11,7 +11,7 @@ concurrency: cancel-in-progress: true env: - TOOLCHAIN: nightly-2024-06-01 + TOOLCHAIN: nightly-2024-10-19 jobs: preflight: @@ -74,7 +74,7 @@ jobs: - name: install parity-publish # Set the target dir to cache the build. - run: CARGO_TARGET_DIR=./target/ cargo install parity-publish@0.8.0 --locked -q + run: CARGO_TARGET_DIR=./target/ cargo install parity-publish@0.10.1 --locked -q - name: check semver run: | From 3c003872178b6ce535e9f26ce52e324f36075ffd Mon Sep 17 00:00:00 2001 From: Egor_P Date: Tue, 26 Nov 2024 14:46:51 +0100 Subject: [PATCH 6/8] [Release|CI/CD] Github pipeline to publish polkadot deb package (#6640) This pipeline should replace a manual action done on the `cleamroom` server to publish the `polkadot` deb package to our apt repo with the pipeline triggered from the new paritytech-release org. Right now, this is done manually by running the [add-packages.sh](https://github.com/paritytech/cleanroom/blob/master/ansible/roles/parity-repos/files/add-packages.sh) script on the `cleanroom` machine. What is done under the hood: - Pipeline downloads `polakdot` deb package from S3, that was prebuilt in the [Build release rc pipeline](https://github.com/paritytech/polkadot-sdk/blob/master/.github/workflows/release-build-rc.yml) - Prepares and syncs local apt repository - Adds and signs deb package to it using `reprepro` - Uploads new deb package to the distributed repo Closes: https://github.com/paritytech/release-engineering/issues/239 --- .github/scripts/common/lib.sh | 33 +++- .github/scripts/release/distributions | 39 +++++ .../release-40_publish-deb-package.yml | 152 ++++++++++++++++++ 3 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 .github/scripts/release/distributions create mode 100644 .github/workflows/release-40_publish-deb-package.yml diff --git a/.github/scripts/common/lib.sh b/.github/scripts/common/lib.sh index e3dd6224f29b..6b8f70a26d7e 100755 --- a/.github/scripts/common/lib.sh +++ b/.github/scripts/common/lib.sh @@ -237,15 +237,44 @@ fetch_release_artifacts() { popd > /dev/null } -# Fetch the release artifacts like binary and signatures from S3. Assumes the ENV are set: +# Fetch deb package from S3. Assumes the ENV are set: # - RELEASE_ID # - GITHUB_TOKEN # - REPO in the form paritytech/polkadot -fetch_release_artifacts_from_s3() { +fetch_debian_package_from_s3() { BINARY=$1 echo "Version : $VERSION" echo "Repo : $REPO" echo "Binary : $BINARY" + echo "Tag : $RELEASE_TAG" + OUTPUT_DIR=${OUTPUT_DIR:-"./release-artifacts/${BINARY}"} + echo "OUTPUT_DIR : $OUTPUT_DIR" + + URL_BASE=$(get_s3_url_base $BINARY) + echo "URL_BASE=$URL_BASE" + + URL=$URL_BASE/$RELEASE_TAG/x86_64-unknown-linux-gnu/${BINARY}_${VERSION}_amd64.deb + + mkdir -p "$OUTPUT_DIR" + pushd "$OUTPUT_DIR" > /dev/null + + echo "Fetching deb package..." + + echo "Fetching %s" "$URL" + curl --progress-bar -LO "$URL" || echo "Missing $URL" + + pwd + ls -al --color + popd > /dev/null + +} + +# Fetch the release artifacts like binary and signatures from S3. Assumes the ENV are set: +# - RELEASE_ID +# - GITHUB_TOKEN +# - REPO in the form paritytech/polkadot +fetch_release_artifacts_from_s3() { + BINARY=$1 OUTPUT_DIR=${OUTPUT_DIR:-"./release-artifacts/${BINARY}"} echo "OUTPUT_DIR : $OUTPUT_DIR" diff --git a/.github/scripts/release/distributions b/.github/scripts/release/distributions new file mode 100644 index 000000000000..a430ec76c6ba --- /dev/null +++ b/.github/scripts/release/distributions @@ -0,0 +1,39 @@ +Origin: Parity +Label: Parity +Codename: release +Architectures: amd64 +Components: main +Description: Apt repository for software made by Parity Technologies Ltd. +SignWith: 90BD75EBBB8E95CB3DA6078F94A4029AB4B35DAE + +Origin: Parity +Label: Parity Staging +Codename: staging +Architectures: amd64 +Components: main +Description: Staging distribution for Parity Technologies Ltd. packages +SignWith: 90BD75EBBB8E95CB3DA6078F94A4029AB4B35DAE + +Origin: Parity +Label: Parity stable2407 +Codename: stable2407 +Architectures: amd64 +Components: main +Description: Apt repository for software made by Parity Technologies Ltd. +SignWith: 90BD75EBBB8E95CB3DA6078F94A4029AB4B35DAE + +Origin: Parity +Label: Parity stable2409 +Codename: stable2409 +Architectures: amd64 +Components: main +Description: Apt repository for software made by Parity Technologies Ltd. +SignWith: 90BD75EBBB8E95CB3DA6078F94A4029AB4B35DAE + +Origin: Parity +Label: Parity stable2412 +Codename: stable2412 +Architectures: amd64 +Components: main +Description: Apt repository for software made by Parity Technologies Ltd. +SignWith: 90BD75EBBB8E95CB3DA6078F94A4029AB4B35DAE diff --git a/.github/workflows/release-40_publish-deb-package.yml b/.github/workflows/release-40_publish-deb-package.yml new file mode 100644 index 000000000000..3c5411ab16f0 --- /dev/null +++ b/.github/workflows/release-40_publish-deb-package.yml @@ -0,0 +1,152 @@ +name: Release - Publish polakdot deb package + +on: + workflow_dispatch: + inputs: + tag: + description: Current final release tag in the format polakdot-stableYYMM or polkadot-stable-YYMM-X + default: polkadot-stable2412 + required: true + type: string + + distribution: + description: Distribution where to publish deb package (release, staging, stable2407, etc) + default: staging + required: true + type: string + +jobs: + check-synchronization: + uses: paritytech-release/sync-workflows/.github/workflows/check-syncronization.yml@main + + validate-inputs: + needs: [check-synchronization] + if: ${{ needs.check-synchronization.outputs.checks_passed }} == 'true' + runs-on: ubuntu-latest + outputs: + release_tag: ${{ steps.validate_inputs.outputs.release_tag }} + + steps: + - name: Checkout sources + uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + + - name: Validate inputs + id: validate_inputs + run: | + . ./.github/scripts/common/lib.sh + + RELEASE_TAG=$(validate_stable_tag ${{ inputs.tag }}) + echo "release_tag=${RELEASE_TAG}" >> $GITHUB_OUTPUT + + + fetch-artifacts-from-s3: + runs-on: ubuntu-latest + needs: [validate-inputs] + env: + REPO: ${{ github.repository }} + RELEASE_TAG: ${{ needs.validate-inputs.outputs.release_tag }} + outputs: + VERSION: ${{ steps.fetch_artifacts_from_s3.outputs.VERSION }} + + steps: + - name: Checkout sources + uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + + - name: Fetch rc artifacts or release artifacts from s3 based on version + id: fetch_artifacts_from_s3 + run: | + . ./.github/scripts/common/lib.sh + + VERSION="$(get_polkadot_node_version_from_code)" + echo "VERSION=${VERSION}" >> $GITHUB_OUTPUT + + fetch_debian_package_from_s3 polkadot + + - name: Upload artifacts + uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1 + with: + name: release-artifacts + path: release-artifacts/polkadot/*.deb + + publish-deb-package: + runs-on: ubuntu-latest + needs: [fetch-artifacts-from-s3] + environment: release + env: + AWS_DEFAULT_REGION: ${{ secrets.AWS_DEFAULT_REGION }} + AWS_DEB_PATH: "s3://releases-package-repos/deb" + LOCAL_DEB_REPO_PATH: ${{ github.workspace }}/deb + VERSION: ${{ needs.fetch-artifacts-from-s3.outputs.VERSION }} + + steps: + - name: Install pgpkkms + run: | + # Install pgpkms that is used to sign built artifacts + python3 -m pip install "pgpkms @ git+https://github.com/paritytech-release/pgpkms.git@1f8555426662ac93a3849480a35449f683b1c89f" + echo "PGPKMS_REPREPRO_PATH=$(which pgpkms-reprepro)" >> $GITHUB_ENV + + - name: Install awscli + run: | + python3 -m pip install awscli + which aws + + - name: Checkout sources + uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0 + + - name: Import gpg keys + shell: bash + run: | + . ./.github/scripts/common/lib.sh + + import_gpg_keys + + - name: Download artifacts + uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + with: + name: release-artifacts + path: release-artifacts + + - name: Setup local deb repo + run: | + sudo apt-get install -y reprepro + which reprepro + + sed -i "s|^SignWith:.*|SignWith: ! ${PGPKMS_REPREPRO_PATH}|" ${{ github.workspace }}/.github/scripts/release/distributions + + mkdir -p ${{ github.workspace }}/deb/conf + cp ${{ github.workspace }}/.github/scripts/release/distributions ${{ github.workspace }}/deb/conf/distributions + cat ${{ github.workspace }}/deb/conf/distributions + + - name: Sync local deb repo + env: + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_RELEASE_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_RELEASE_SECRET_ACCESS_KEY }} + run: | + # Download the current state of the deb repo + aws s3 sync "$AWS_DEB_PATH/db" "$LOCAL_DEB_REPO_PATH/db" + aws s3 sync "$AWS_DEB_PATH/pool" "$LOCAL_DEB_REPO_PATH/pool" + aws s3 sync "$AWS_DEB_PATH/dists" "$LOCAL_DEB_REPO_PATH/dists" + + - name: Add deb package to local repo + env: + PGP_KMS_KEY: ${{ secrets.PGP_KMS_KEY }} + PGP_KMS_HASH: ${{ secrets.PGP_KMS_HASH }} + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + run: | + # Add the new deb to the repo + reprepro -b "$LOCAL_DEB_REPO_PATH" includedeb "${{ inputs.distribution }}" "release-artifacts/polkadot_${VERSION}_amd64.deb" + + - name: Upload updated deb repo + env: + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_RELEASE_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_RELEASE_SECRET_ACCESS_KEY }} + run: | + # Upload the updated repo - dists and pool should be publicly readable + aws s3 sync "$LOCAL_DEB_REPO_PATH/pool" "$AWS_DEB_PATH/pool" --acl public-read + aws s3 sync "$LOCAL_DEB_REPO_PATH/dists" "$AWS_DEB_PATH/dists" --acl public-read + aws s3 sync "$LOCAL_DEB_REPO_PATH/db" "$AWS_DEB_PATH/db" + aws s3 sync "$LOCAL_DEB_REPO_PATH/conf" "$AWS_DEB_PATH/conf" + + # Invalidate caches to make sure latest files are served + aws cloudfront create-invalidation --distribution-id E36FKEYWDXAZYJ --paths '/deb/*' From 1c0b610078611dfebc082be52e77109ea4517e48 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 26 Nov 2024 15:52:23 +0100 Subject: [PATCH 7/8] xcm: fix local/remote exports when inner routers return `NotApplicable` (#6645) This PR addresses two small fixes: 1. Fixed a typo ("as as") found on the way. 2. Resolved a bug in the `local/remote exporters` used for bridging. Previously, they consumed `dest` and `msg` without returning them when inner routers/exporters failed with `NotApplicable`. This PR ensures compliance with the [`SendXcm`](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/src/v5/traits.rs#L449-L450) and [`ExportXcm`](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-executor/src/traits/export.rs#L44-L45) traits. --------- Co-authored-by: GitHub Action --- polkadot/grafana/README.md | 2 +- polkadot/grafana/parachains/status.json | 2 +- polkadot/xcm/src/v3/traits.rs | 4 +- polkadot/xcm/src/v4/traits.rs | 4 +- polkadot/xcm/src/v5/traits.rs | 4 +- .../xcm/xcm-builder/src/universal_exports.rs | 265 +++++++++++++++--- .../xcm/xcm-executor/src/traits/export.rs | 4 +- prdoc/pr_6645.prdoc | 14 + .../frame/examples/default-config/src/lib.rs | 4 +- 9 files changed, 248 insertions(+), 55 deletions(-) create mode 100644 prdoc/pr_6645.prdoc diff --git a/polkadot/grafana/README.md b/polkadot/grafana/README.md index e909fdd29a75..0ecb0b70515b 100644 --- a/polkadot/grafana/README.md +++ b/polkadot/grafana/README.md @@ -90,4 +90,4 @@ and issue statement or initiate dispute. - **Assignment delay tranches**. Approval voting is designed such that validators assigned to check a specific candidate are split up into equal delay tranches (0.5 seconds each). All validators checks are ordered by the delay tranche index. Early tranches of validators have the opportunity to check the candidate first before later tranches -that act as as backups in case of no shows. +that act as backups in case of no shows. diff --git a/polkadot/grafana/parachains/status.json b/polkadot/grafana/parachains/status.json index 5942cbdf4479..22250967848d 100644 --- a/polkadot/grafana/parachains/status.json +++ b/polkadot/grafana/parachains/status.json @@ -1405,7 +1405,7 @@ "type": "prometheus", "uid": "$data_source" }, - "description": "Approval voting requires that validators which are assigned to check a specific \ncandidate are split up into delay tranches (0.5s each). Then, all validators checks are ordered by the delay \ntranche index. Early tranches of validators will check the candidate first and later tranches act as as backups in case of no shows.", + "description": "Approval voting requires that validators which are assigned to check a specific \ncandidate are split up into delay tranches (0.5s each). Then, all validators checks are ordered by the delay \ntranche index. Early tranches of validators will check the candidate first and later tranches act as backups in case of no shows.", "gridPos": { "h": 9, "w": 18, diff --git a/polkadot/xcm/src/v3/traits.rs b/polkadot/xcm/src/v3/traits.rs index 1c8620708922..cbf85b454cc6 100644 --- a/polkadot/xcm/src/v3/traits.rs +++ b/polkadot/xcm/src/v3/traits.rs @@ -547,13 +547,13 @@ impl SendXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_send(dest: MultiLocation, msg: Xcm<()>) -> SendResult { T::validate(&mut Some(dest), &mut Some(msg)) } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/polkadot/xcm/src/v4/traits.rs b/polkadot/xcm/src/v4/traits.rs index f32b26fb163d..178093d27177 100644 --- a/polkadot/xcm/src/v4/traits.rs +++ b/polkadot/xcm/src/v4/traits.rs @@ -289,13 +289,13 @@ impl SendXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_send(dest: Location, msg: Xcm<()>) -> SendResult { T::validate(&mut Some(dest), &mut Some(msg)) } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/polkadot/xcm/src/v5/traits.rs b/polkadot/xcm/src/v5/traits.rs index 1f5041ca8d84..dd067b774fcd 100644 --- a/polkadot/xcm/src/v5/traits.rs +++ b/polkadot/xcm/src/v5/traits.rs @@ -502,13 +502,13 @@ impl SendXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_send(dest: Location, msg: Xcm<()>) -> SendResult { T::validate(&mut Some(dest), &mut Some(msg)) } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/polkadot/xcm/xcm-builder/src/universal_exports.rs b/polkadot/xcm/xcm-builder/src/universal_exports.rs index 5c754f01ec0a..aae8438c78d2 100644 --- a/polkadot/xcm/xcm-builder/src/universal_exports.rs +++ b/polkadot/xcm/xcm-builder/src/universal_exports.rs @@ -68,20 +68,28 @@ impl> SendXcm fn validate( dest: &mut Option, - xcm: &mut Option>, + msg: &mut Option>, ) -> SendResult { - let d = dest.take().ok_or(MissingArgument)?; + // This `clone` ensures that `dest` is not consumed in any case. + let d = dest.clone().take().ok_or(MissingArgument)?; let universal_source = UniversalLocation::get(); - let devolved = match ensure_is_remote(universal_source.clone(), d) { - Ok(x) => x, - Err(d) => { - *dest = Some(d); - return Err(NotApplicable) - }, - }; - let (network, destination) = devolved; - let xcm = xcm.take().ok_or(SendError::MissingArgument)?; - validate_export::(network, 0, universal_source, destination, xcm) + let devolved = ensure_is_remote(universal_source.clone(), d).map_err(|_| NotApplicable)?; + let (remote_network, remote_location) = devolved; + let xcm = msg.take().ok_or(MissingArgument)?; + + validate_export::( + remote_network, + 0, + universal_source, + remote_location, + xcm.clone(), + ) + .inspect_err(|err| { + if let NotApplicable = err { + // We need to make sure that msg is not consumed in case of `NotApplicable`. + *msg = Some(xcm); + } + }) } fn deliver(ticket: Exporter::Ticket) -> Result { @@ -95,7 +103,7 @@ pub trait ExporterFor { /// /// The payment is specified from the local context, not the bridge chain. This is the /// total amount to withdraw in to Holding and should cover both payment for the execution on - /// the bridge chain as well as payment for the use of the `ExportMessage` instruction. + /// the bridge chain and payment for the use of the `ExportMessage` instruction. fn exporter_for( network: &NetworkId, remote_location: &InteriorLocation, @@ -205,7 +213,8 @@ impl, msg: &mut Option>, ) -> SendResult { - let d = dest.clone().ok_or(MissingArgument)?; + // This `clone` ensures that `dest` is not consumed in any case. + let d = dest.clone().take().ok_or(MissingArgument)?; let devolved = ensure_is_remote(UniversalLocation::get(), d).map_err(|_| NotApplicable)?; let (remote_network, remote_location) = devolved; let xcm = msg.take().ok_or(MissingArgument)?; @@ -216,7 +225,7 @@ impl(bridge, message) + validate_send::(bridge, message).inspect_err(|err| { + if let NotApplicable = err { + // We need to make sure that msg is not consumed in case of `NotApplicable`. + *msg = Some(xcm); + } + }) } fn deliver(validation: Self::Ticket) -> Result { @@ -272,9 +290,9 @@ impl, msg: &mut Option>, ) -> SendResult { - let d = dest.as_ref().ok_or(MissingArgument)?; - let devolved = - ensure_is_remote(UniversalLocation::get(), d.clone()).map_err(|_| NotApplicable)?; + // This `clone` ensures that `dest` is not consumed in any case. + let d = dest.clone().take().ok_or(MissingArgument)?; + let devolved = ensure_is_remote(UniversalLocation::get(), d).map_err(|_| NotApplicable)?; let (remote_network, remote_location) = devolved; let xcm = msg.take().ok_or(MissingArgument)?; @@ -284,7 +302,7 @@ impl(bridge, message)?; + let (v, mut cost) = validate_send::(bridge, message).inspect_err(|err| { + if let NotApplicable = err { + // We need to make sure that msg is not consumed in case of `NotApplicable`. + *msg = Some(xcm); + } + })?; if let Some(bridge_payment) = maybe_payment { cost.push(bridge_payment); } @@ -476,10 +502,10 @@ impl< let Location { parents, interior: mut junctions } = BridgedNetwork::get(); match junctions.take_first() { Some(GlobalConsensus(network)) => (network, parents), - _ => return Err(SendError::NotApplicable), + _ => return Err(NotApplicable), } }; - ensure!(&network == &bridged_network, SendError::NotApplicable); + ensure!(&network == &bridged_network, NotApplicable); // We don't/can't use the `channel` for this adapter. let dest = destination.take().ok_or(SendError::MissingArgument)?; @@ -496,7 +522,7 @@ impl< }, Err((dest, _)) => { *destination = Some(dest); - return Err(SendError::NotApplicable) + return Err(NotApplicable) }, }; @@ -540,6 +566,10 @@ impl< #[cfg(test)] mod tests { use super::*; + use frame_support::{ + assert_err, assert_ok, + traits::{Contains, Equals}, + }; #[test] fn ensure_is_remote_works() { @@ -564,21 +594,48 @@ mod tests { assert_eq!(x, Err((Parent, Polkadot, Parachain(1000)).into())); } - pub struct OkSender; - impl SendXcm for OkSender { + pub struct OkFor(PhantomData); + impl> SendXcm for OkFor { type Ticket = (); fn validate( - _destination: &mut Option, + destination: &mut Option, _message: &mut Option>, ) -> SendResult { - Ok(((), Assets::new())) + if let Some(d) = destination.as_ref() { + if Filter::contains(&d) { + return Ok(((), Assets::new())) + } + } + Err(NotApplicable) } fn deliver(_ticket: Self::Ticket) -> Result { Ok([0; 32]) } } + impl> ExportXcm for OkFor { + type Ticket = (); + + fn validate( + network: NetworkId, + _: u32, + _: &mut Option, + destination: &mut Option, + _: &mut Option>, + ) -> SendResult { + if let Some(d) = destination.as_ref() { + if Filter::contains(&(network, d.clone())) { + return Ok(((), Assets::new())) + } + } + Err(NotApplicable) + } + + fn deliver(_ticket: Self::Ticket) -> Result { + Ok([1; 32]) + } + } /// Generic test case asserting that dest and msg is not consumed by `validate` implementation /// of `SendXcm` in case of expected result. @@ -598,46 +655,168 @@ mod tests { } #[test] - fn remote_exporters_does_not_consume_dest_or_msg_on_not_applicable() { + fn local_exporters_works() { frame_support::parameter_types! { pub Local: NetworkId = ByGenesis([0; 32]); pub UniversalLocation: InteriorLocation = [GlobalConsensus(Local::get()), Parachain(1234)].into(); pub DifferentRemote: NetworkId = ByGenesis([22; 32]); - // no routers - pub BridgeTable: Vec = vec![]; + pub RemoteDestination: Junction = Parachain(9657); + pub RoutableBridgeFilter: (NetworkId, InteriorLocation) = (DifferentRemote::get(), RemoteDestination::get().into()); } + type RoutableBridgeExporter = OkFor>; + type NotApplicableBridgeExporter = OkFor<()>; + assert_ok!(validate_export::( + DifferentRemote::get(), + 0, + UniversalLocation::get(), + RemoteDestination::get().into(), + Xcm::default() + )); + assert_err!( + validate_export::( + DifferentRemote::get(), + 0, + UniversalLocation::get(), + RemoteDestination::get().into(), + Xcm::default() + ), + NotApplicable + ); - // check with local destination (should be remote) + // 1. check with local destination (should be remote) let local_dest: Location = (Parent, Parachain(5678)).into(); assert!(ensure_is_remote(UniversalLocation::get(), local_dest.clone()).is_err()); + // UnpaidLocalExporter ensure_validate_does_not_consume_dest_or_msg::< - UnpaidRemoteExporter, OkSender, UniversalLocation>, + UnpaidLocalExporter, >(local_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + // 2. check with not applicable from the inner router (using `NotApplicableBridgeSender`) + let remote_dest: Location = + (Parent, Parent, DifferentRemote::get(), RemoteDestination::get()).into(); + assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok()); + + // UnpaidLocalExporter + ensure_validate_does_not_consume_dest_or_msg::< + UnpaidLocalExporter, + >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + + // 3. Ok - deliver + // UnpaidRemoteExporter + assert_ok!(send_xcm::>( + remote_dest, + Xcm::default() + )); + } + + #[test] + fn remote_exporters_works() { + frame_support::parameter_types! { + pub Local: NetworkId = ByGenesis([0; 32]); + pub UniversalLocation: InteriorLocation = [GlobalConsensus(Local::get()), Parachain(1234)].into(); + pub DifferentRemote: NetworkId = ByGenesis([22; 32]); + pub RoutableBridge: Location = Location::new(1, Parachain(9657)); + // not routable + pub NotApplicableBridgeTable: Vec = vec![]; + // routable + pub RoutableBridgeTable: Vec = vec![ + NetworkExportTableItem::new( + DifferentRemote::get(), + None, + RoutableBridge::get(), + None + ) + ]; + } + type RoutableBridgeSender = OkFor>; + type NotApplicableBridgeSender = OkFor<()>; + assert_ok!(validate_send::(RoutableBridge::get(), Xcm::default())); + assert_err!( + validate_send::(RoutableBridge::get(), Xcm::default()), + NotApplicable + ); + + // 1. check with local destination (should be remote) + let local_dest: Location = (Parent, Parachain(5678)).into(); + assert!(ensure_is_remote(UniversalLocation::get(), local_dest.clone()).is_err()); + + // UnpaidRemoteExporter + ensure_validate_does_not_consume_dest_or_msg::< + UnpaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, + >(local_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + // SovereignPaidRemoteExporter ensure_validate_does_not_consume_dest_or_msg::< SovereignPaidRemoteExporter< - NetworkExportTable, - OkSender, + NetworkExportTable, + RoutableBridgeSender, UniversalLocation, >, >(local_dest, |result| assert_eq!(Err(NotApplicable), result)); - // check with not applicable destination + // 2. check with not applicable destination (`NotApplicableBridgeTable`) let remote_dest: Location = (Parent, Parent, DifferentRemote::get()).into(); assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok()); + // UnpaidRemoteExporter ensure_validate_does_not_consume_dest_or_msg::< - UnpaidRemoteExporter, OkSender, UniversalLocation>, + UnpaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); - + // SovereignPaidRemoteExporter ensure_validate_does_not_consume_dest_or_msg::< SovereignPaidRemoteExporter< - NetworkExportTable, - OkSender, + NetworkExportTable, + RoutableBridgeSender, UniversalLocation, >, >(remote_dest, |result| assert_eq!(Err(NotApplicable), result)); + + // 3. check with not applicable from the inner router (using `NotApplicableBridgeSender`) + let remote_dest: Location = (Parent, Parent, DifferentRemote::get()).into(); + assert!(ensure_is_remote(UniversalLocation::get(), remote_dest.clone()).is_ok()); + + // UnpaidRemoteExporter + ensure_validate_does_not_consume_dest_or_msg::< + UnpaidRemoteExporter< + NetworkExportTable, + NotApplicableBridgeSender, + UniversalLocation, + >, + >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + // SovereignPaidRemoteExporter + ensure_validate_does_not_consume_dest_or_msg::< + SovereignPaidRemoteExporter< + NetworkExportTable, + NotApplicableBridgeSender, + UniversalLocation, + >, + >(remote_dest.clone(), |result| assert_eq!(Err(NotApplicable), result)); + + // 4. Ok - deliver + // UnpaidRemoteExporter + assert_ok!(send_xcm::< + UnpaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, + >(remote_dest.clone(), Xcm::default())); + // SovereignPaidRemoteExporter + assert_ok!(send_xcm::< + SovereignPaidRemoteExporter< + NetworkExportTable, + RoutableBridgeSender, + UniversalLocation, + >, + >(remote_dest, Xcm::default())); } #[test] diff --git a/polkadot/xcm/xcm-executor/src/traits/export.rs b/polkadot/xcm/xcm-executor/src/traits/export.rs index b356e0da7df7..3e9275edab37 100644 --- a/polkadot/xcm/xcm-executor/src/traits/export.rs +++ b/polkadot/xcm/xcm-executor/src/traits/export.rs @@ -108,7 +108,7 @@ impl ExportXcm for Tuple { } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. pub fn validate_export( network: NetworkId, channel: u32, @@ -120,7 +120,7 @@ pub fn validate_export( } /// Convenience function for using a `SendXcm` implementation. Just interprets the `dest` and wraps -/// both in `Some` before passing them as as mutable references into `T::send_xcm`. +/// both in `Some` before passing them as mutable references into `T::send_xcm`. /// /// Returns either `Ok` with the price of the delivery, or `Err` with the reason why the message /// could not be sent. diff --git a/prdoc/pr_6645.prdoc b/prdoc/pr_6645.prdoc new file mode 100644 index 000000000000..f033cadc0b6e --- /dev/null +++ b/prdoc/pr_6645.prdoc @@ -0,0 +1,14 @@ +title: 'xcm: fix local/remote exports when inner routers return `NotApplicable`' +doc: +- audience: Runtime Dev + description: |- + Resolved a bug in the `local/remote exporters` used for bridging. Previously, they consumed `dest` and `msg` without returning them when inner routers/exporters failed with `NotApplicable`. This PR ensures compliance with the [`SendXcm`](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/src/v5/traits.rs#L449-L450) and [`ExportXcm`](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-executor/src/traits/export.rs#L44-L45) traits. +crates: +- name: staging-xcm-builder + bump: patch +- name: polkadot + bump: none +- name: staging-xcm + bump: none +- name: staging-xcm-executor + bump: none diff --git a/substrate/frame/examples/default-config/src/lib.rs b/substrate/frame/examples/default-config/src/lib.rs index ccdcd4968598..f690bffe0998 100644 --- a/substrate/frame/examples/default-config/src/lib.rs +++ b/substrate/frame/examples/default-config/src/lib.rs @@ -62,10 +62,10 @@ pub mod pallet { type OverwrittenDefaultValue: Get; /// An input parameter that relies on `::AccountId`. This can - /// too have a default, as long as as it is present in `frame_system::DefaultConfig`. + /// too have a default, as long as it is present in `frame_system::DefaultConfig`. type CanDeriveDefaultFromSystem: Get; - /// We might chose to declare as one that doesn't have a default, for whatever semantical + /// We might choose to declare as one that doesn't have a default, for whatever semantical /// reason. #[pallet::no_default] type HasNoDefault: Get; From f520adb0aacd51247ed548c7db9bef874c2cca9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dino=20Pa=C4=8Dandi?= <3002868+Dinonard@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:56:02 +0100 Subject: [PATCH 8/8] Zero refund check for FungibleAdapter (#6506) `FungibleAdapter` will now check if the _refund amount_ is zero before calling deposit & emitting an event. Fixes https://github.com/paritytech/polkadot-sdk/issues/6469. --------- Co-authored-by: GitHub Action --- prdoc/pr_6506.prdoc | 10 +++++ .../frame/transaction-payment/src/payment.rs | 17 +++++---- .../frame/transaction-payment/src/tests.rs | 37 +++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 prdoc/pr_6506.prdoc diff --git a/prdoc/pr_6506.prdoc b/prdoc/pr_6506.prdoc new file mode 100644 index 000000000000..7c6164a9959a --- /dev/null +++ b/prdoc/pr_6506.prdoc @@ -0,0 +1,10 @@ +title: Zero refund check for FungibleAdapter +doc: +- audience: Runtime User + description: |- + `FungibleAdapter` will now check if the _refund amount_ is zero before calling deposit & emitting an event. + + Fixes https://github.com/paritytech/polkadot-sdk/issues/6469. +crates: +- name: pallet-transaction-payment + bump: patch diff --git a/substrate/frame/transaction-payment/src/payment.rs b/substrate/frame/transaction-payment/src/payment.rs index 4b39cd3fe53b..b8a047fee3e6 100644 --- a/substrate/frame/transaction-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/src/payment.rs @@ -155,14 +155,15 @@ where if let Some(paid) = already_withdrawn { // Calculate how much refund we should return let refund_amount = paid.peek().saturating_sub(corrected_fee); - // refund to the the account that paid the fees if it exists. otherwise, don't refind - // anything. - let refund_imbalance = if F::total_balance(who) > F::Balance::zero() { - F::deposit(who, refund_amount, Precision::BestEffort) - .unwrap_or_else(|_| Debt::::zero()) - } else { - Debt::::zero() - }; + // Refund to the the account that paid the fees if it exists & refund is non-zero. + // Otherwise, don't refund anything. + let refund_imbalance = + if refund_amount > Zero::zero() && F::total_balance(who) > F::Balance::zero() { + F::deposit(who, refund_amount, Precision::BestEffort) + .unwrap_or_else(|_| Debt::::zero()) + } else { + Debt::::zero() + }; // merge the imbalance caused by paying the fees and refunding parts of it again. let adjusted_paid: Credit = paid .offset(refund_imbalance) diff --git a/substrate/frame/transaction-payment/src/tests.rs b/substrate/frame/transaction-payment/src/tests.rs index 572c1d4961dd..bde1bf64728e 100644 --- a/substrate/frame/transaction-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/src/tests.rs @@ -877,3 +877,40 @@ fn no_fee_and_no_weight_for_other_origins() { assert_eq!(post_info.actual_weight, Some(info.call_weight)); }) } + +#[test] +fn fungible_adapter_no_zero_refund_action() { + type FungibleAdapterT = payment::FungibleAdapter; + + ExtBuilder::default().balance_factor(10).build().execute_with(|| { + System::set_block_number(10); + + let dummy_acc = 1; + let (actual_fee, no_tip) = (10, 0); + let already_paid = >::withdraw_fee( + &dummy_acc, + CALL, + &CALL.get_dispatch_info(), + actual_fee, + no_tip, + ).expect("Account must have enough funds."); + + // Correction action with no expected side effect. + assert!(>::correct_and_deposit_fee( + &dummy_acc, + &CALL.get_dispatch_info(), + &default_post_info(), + actual_fee, + no_tip, + already_paid, + ).is_ok()); + + // Ensure no zero amount deposit event is emitted. + let events = System::events(); + assert!(!events + .iter() + .any(|record| matches!(record.event, RuntimeEvent::Balances(pallet_balances::Event::Deposit { amount, .. }) if amount.is_zero())), + "No zero amount deposit amount event should be emitted.", + ); + }); +}