Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filter backing votes for disputes concluded in the past #4690

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

ordian
Copy link
Member

@ordian ordian commented Jun 4, 2024

This adds an additional layer of protection (defense in depth) for the case when a dispute has concluded, but the candidate is attempted to be included again (causing a freeze and reversion again).

ordian added 16 commits May 14, 2024 17:16
* master: (106 commits)
  [ci] Delete unused flow (#4676)
  Fix umbrella CI check and fix the C&P message (#4670)
  Add Dockerfiles to the templates (#4637)
  Revamp the Readme of the minimal template (#4649)
  Add chain-spec-builder docker image (#4655)
  Update Amforc bootnodes for Kusama and Polkadot (#4668)
  make all storage items in parachain-system public (#4645)
  [Pools] Refactors and runtime apis for DelegateStake (#4537)
  update amforc westend and its parachain bootnodes (#4641)
  Better error for missing index in CRV2 (#4643)
  Implement `XcmPaymentApi` and `DryRunApi` on all system parachains (#4634)
  Use Unlicense for templates (#4628)
  collator-protocol: remove `elastic-scaling-experimental` feature (#4595)
  Update `runtime_type` ref doc with the new "Associated Type Bounds" (#4624)
  Adds ability to specify chain type in chain-spec-builder (#4542)
  Fix broken windows build (#4636)
  Beefy client generic on aduthority Id (#1816)
  pallet-staking: Put tests behind `cfg(debug_assertions)` (#4620)
  Broker new price adapter (#4521)
  Change `XcmDryRunApi::dry_run_extrinsic` to take a call instead (#4621)
  ...
* master:
  Add READ_ONLY flag to contract call function  (#4418)
  Format the README.md files (#4688)
  Typos in template README (#4687)
  [ci] Increase timeout for check-runtime-migration workflow (#4674)
@ordian ordian added T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Jun 4, 2024
@alindima alindima self-requested a review June 5, 2024 15:23
ordian added 4 commits June 13, 2024 23:42
* master: (29 commits)
  Append overlay optimization. (#1223)
  finalization: Skip tree route calculation if no forks present (#4721)
  Remove unncessary call remove_from_peers_set (#4742)
  add pov-recovery unit tests and support for elastic scaling (#4733)
  approval-voting: Add no shows debug information (#4726)
  Revamp the Readme of the parachain template (#4713)
  Update README.md to move the PSVM link under a "Tooling" section under the "Releases" section (#4734)
  frame/proc-macro: Refactor code for better readability (#4712)
  Contracts:  update wasmi to 0.32 (#3679)
  Backport style changes from P<>K bridge to R<>W bridge (#4732)
  New reference doc for Custom RPC V2 (#4654)
  Frame Pallets: Clean a lot of test setups (#4642)
  Fix occupied core handling (#4691)
  statement-distribution: Fix false warning (#4727)
  Update the README to include a link to the Polkadot SDK Version Manager (#4718)
  Cleanup PVF artifact by cache limit and stale time (#4662)
  Update link to a latest polkadot release (#4711)
  [CI] Delete cargo-deny config (#4677)
  fix build on MacOS: bump secp256k1 and secp256k1-sys to patched versions (#4709)
  Unify dependency aliases (#4633)
  ...
* master:
  [Backport] Version bumps and prdoc reorg from 1.13.0 (#4784)
  Add `--version` to the `ChainSpecBuilder` command for `staging-chain-spec-builder` (#4752)
  Use aggregated types for `RuntimeFreezeReason` and better examples of `MaxFreezes` (#4615)
  Update the pallet guide in `sdk-docs` (#4735)
  Hide `tuplex` dependency and re-export by macro (#4774)
* master: (125 commits)
  add elastic scaling MVP guide (#4663)
  Send PeerViewChange with high priority (#4755)
  [ci] Update forklift in CI image (#5032)
  Adjust base value for statement-distribution regression tests (#5028)
  [pallet_contracts] Add support for transient storage in contracts host functions (#4566)
  [1 / 5] Optimize logic for gossiping assignments (#4848)
  Remove `pallet-getter` usage from pallet-session (#4972)
  command-action: added scoped permissions to the github tokens (#5016)
  net/litep2p: Propagate ValuePut events to the network backend (#5018)
  rpc: add back rpc logger (#4952)
  Updated substrate-relay version for tests (#5017)
  Remove most all usage of `sp-std` (#5010)
  Use sp_runtime::traits::BadOrigin (#5011)
  network/tx: Ban peers with tx that fail to decode (#5002)
  Try State Hook for Bounties (#4563)
  [statement-distribution] Add metrics for distributed statements in V2 (#4554)
  added sync command (#4818)
  Bridges V2 refactoring backport and `pallet_bridge_messages` simplifications (#4935)
  xcm-executor: Improve logging (#4996)
  Remove usage of `sp-std` on templates (#5001)
  ...
prdoc/pr_4690.prdoc Outdated Show resolved Hide resolved
Copy link
Contributor

@sandreim sandreim left a comment

Choose a reason for hiding this comment

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

Looks good! I just have a few questions

polkadot/runtime/parachains/src/paras_inherent/mod.rs Outdated Show resolved Hide resolved
polkadot/runtime/parachains/src/disputes.rs Outdated Show resolved Hide resolved

if let Some(state) = Disputes::<T>::get(&session, candidate_hash) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we do this initially if we are already doing it before in process_checked_multi_dispute_data ?

Copy link
Member Author

Choose a reason for hiding this comment

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

As opposed to process_checked_multi_dispute_data, here we used to check for all past concluding disputes, not just the ones that concluded in this block. And we needed to do this exactly because of possibility of re-including a candidate disputed in the past.

ordian added 5 commits August 14, 2024 22:02
* master: (167 commits)
  Upgrade accidentally downgraded deps (#5365)
  [Pools] Fix issues with member migration to `DelegateStake` (#4822)
  Unify `no_genesis` check (#5360)
  [CI] Fix prdoc command (#5358)
  Beefy: add benchmarks for `report_fork_voting()` (#5188)
  Fix OurViewChange small race (#5356)
  Make ticket non-optional and add ensure_successful method to Consideration trait (#5359)
  [tests] dedup test code, add more tests, improve naming and docs (#5338)
  Stop running the wishlist workflow on forks (#5297)
  Migrate foreign assets v3::Location to v4::Location (#4129)
  Minor clean up (#5284)
  [Pools] Ensure members can always exit the pool gracefully (#4998)
  StorageWeightReclaim: set to node pov size if higher (#5281)
  [Bot] Add prdoc generation (#5331)
  Small nits found accidentally along the way (#5341)
  Create subsystem-benchmarks.yml (#5325)
  Bump libp2p-identity from 0.2.8 to 0.2.9 (#5232)
  Bump authoring duration for async backing to 2s. (#5195)
  Fix spelling issues (#5206)
  Bump the known_good_semver group across 1 directory with 3 updates (#5315)
  ...
…itytech/polkadot-sdk into ao-fix-backing-filtering-on-disputes

* 'ao-fix-backing-filtering-on-disputes' of github.com:paritytech/polkadot-sdk:
  Apply suggestions from code review
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7026778

ordian added 2 commits August 16, 2024 14:24
* master:
  Remove redundant minimal template workspace (#5330)
  approval-distribution: Fix handling of conclude (#5375)
  More logs in `is_potential_spam` from `dispute-coordinator` (#5252)
  Fix zombienet bridges test (#5373)
  Update Readme of the `polkadot` crate (#5326)
  allow for `u8` to be used as hold/freeze reason (#5348)
  Moving cargo check for runtimes to GHA (#5340)
  Update links in the documentation (#5175)
  fix visibility for `pallet_nfts` types used as call arguments (#3634)
  Correct some typos in crates' descriptions (#5262)
  Aura: Ensure we are building on each relay chain fork (#5352)
  Update Identity pallet README.md (#5183)
  Bump trie-db from 0.29.0 to 0.29.1 (#5231)
  [Coretime] Always include UnpaidExecution, not just when revenue is > 0 (#5369)
  [Pools] fix derivation of pool account (#4999)
@ordian ordian requested a review from sandreim August 16, 2024 12:29
Copy link
Contributor

@alindima alindima left a comment

Choose a reason for hiding this comment

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

Looking good

ordian added 2 commits August 27, 2024 12:35
* master: (36 commits)
  Bump the ci_dependencies group across 1 directory with 2 updates (#5401)
  Remove deprecated calls in cumulus-parachain-system (#5439)
  Make the PR template a default for new PRs (#5462)
  Only log the propagating transactions when they are not empty (#5424)
  [CI] Fix SemVer check base commit (#5361)
  Sync status refactoring (#5450)
  Add build options to the srtool build step (#4956)
  `MaybeConsideration` extension trait for `Consideration` (#5384)
  Skip slot before creating inherent data providers during major sync (#5344)
  Add symlinks for code of conduct and contribution guidelines (#5447)
  pallet-collator-selection: correctly register weight in `new_session` (#5430)
  Derive `Clone` on `EncodableOpaqueLeaf` (#5442)
  Moving `Find FAIL-CI` check to GHA (#5377)
  Remove panic, as proof is invalid. (#5427)
  Reactive syncing metrics (#5410)
  [bridges] Prune messages from confirmation tx body, not from the on_idle (#5006)
  Change the chain to Rococo in the parachain template Zombienet config (#5279)
  Improve the appearance of crates on `crates.io` (#5243)
  Add initial version of `pallet_revive` (#5293)
  Update OpenZeppelin template documentation (#5398)
  ...
@ordian ordian requested review from eskimor and Overkillus August 27, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet. T8-polkadot This PR/Issue is related to/affects the Polkadot network.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants