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

enable disputes for known chains, except for polkadot #4464

Merged
merged 20 commits into from
Dec 17, 2021
Merged

enable disputes for known chains, except for polkadot #4464

merged 20 commits into from
Dec 17, 2021

Conversation

drahnr
Copy link
Contributor

@drahnr drahnr commented Dec 3, 2021

Ref #4453

@drahnr drahnr requested a review from a team as a code owner December 3, 2021 17:47
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 3, 2021
@drahnr drahnr added B0-silent Changes should not be mentioned in any release notes B1-releasenotes C1-low PR touches the given topic and has a low impact on builders. labels Dec 3, 2021
@drahnr drahnr force-pushed the bernhard-remove-disputes-feature branch from 032483d to f27e126 Compare December 3, 2021 18:01
eskimor
eskimor reviewed Dec 3, 2021
View reviewed changes
Copy link
Member

@eskimor eskimor left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Apart from ignoring disputes for chain selection, we should probably also not to try to pass disputes to the runtime on block creation. Having all the other machinery still in place is probably a good idea, as then we would see any Polkadot specific issues before false disputes would actually be able to hit the chain for real.

@drahnr drahnr requested a review from eskimor December 3, 2021 18:27
@rphmeier
Copy link
Contributor

rphmeier commented Dec 3, 2021

The one thing I'm a little confused by is the removal of the dummy dispute coordinator. I think it should be fine since we intentionally avoid invoking the dispute coordinator in chain selection and in the provisioner. We might end up in some weird middle ground where we participate in disputes off-chain but they have absolutely no effect. It's weird, but not a problem except wasted resources.

@drahnr
Copy link
Contributor Author

drahnr commented Dec 3, 2021

The one thing I'm a little confused by is the removal of the dummy dispute coordinator. I think it should be fine since we intentionally avoid invoking the dispute coordinator in chain selection and in the provisioner. We might end up in some weird middle ground where we participate in disputes off-chain but they have absolutely no effect. It's weird, but not a problem except wasted resources.

Agreed, I'll resurrect it on Monday, I forgot that there is the builder pattern with the overseer now which enables to replace this easily

eskimor
eskimor reviewed Dec 4, 2021
View reviewed changes
node/core/provisioner/src/lib.rs Outdated Show resolved Hide resolved
node/service/src/relay_chain_selection.rs Outdated Show resolved Hide resolved
@drahnr drahnr removed the B0-silent Changes should not be mentioned in any release notes label Dec 4, 2021
@chevdor chevdor added this to the v0.9.14 milestone Dec 4, 2021
@drahnr drahnr changed the title enable disputes enable disputes for known chains, except for polkadot Dec 5, 2021
@drahnr drahnr self-assigned this Dec 10, 2021
@drahnr
Copy link
Contributor Author

drahnr commented Dec 10, 2021 •
edited
Loading

TODO:

  • add versi (internal fast breaking testnet) to enabled chains

@ordian ordian modified the milestones: v0.9.14, v0.9.15 Dec 15, 2021
@drahnr
Copy link
Contributor Author

drahnr commented Dec 16, 2021 •
edited
Loading

Versi was already enabled in #4485

@drahnr drahnr force-pushed the bernhard-remove-disputes-feature branch from 78f52f5 to e063890 Compare December 17, 2021 16:30
@drahnr drahnr requested a review from sandreim December 17, 2021 16:57
@drahnr
Copy link
Contributor Author

drahnr commented Dec 17, 2021

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

sandreim
sandreim approved these changes Dec 17, 2021
View reviewed changes
node/core/provisioner/src/lib.rs Outdated Show resolved Hide resolved
@drahnr
Copy link
Contributor Author

drahnr commented Dec 17, 2021

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Checks failed for a8d2d01

@drahnr drahnr force-pushed the bernhard-remove-disputes-feature branch from fa63146 to 8b55807 Compare December 17, 2021 17:22
@drahnr
Copy link
Contributor Author

drahnr commented Dec 17, 2021

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 97ec5c5 into master Dec 17, 2021
@paritytech-processbot paritytech-processbot bot deleted the bernhard-remove-disputes-feature branch December 17, 2021 17:50
@drahnr drahnr mentioned this pull request Dec 18, 2021
Merged
drahnr added a commit that referenced this pull request Dec 18, 2021
@drahnr @ordian
enable disputes for known chains, except for polkadot (#4464)
06bae04 
* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>
drahnr added a commit that referenced this pull request Dec 20, 2021
@drahnr @ordian
enable disputes for known chains, except for polkadot (#4464)
0c7bbc4 
* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>
drahnr added a commit that referenced this pull request Dec 20, 2021
@drahnr @ordian
enable disputes for known chains, except for polkadot (#4464)
11e4f83 
* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>
drahnr added a commit that referenced this pull request Dec 20, 2021
@drahnr @ordian
enable disputes for known chains, except for polkadot (#4464)
4e6c608 
* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>
drahnr added a commit that referenced this pull request Dec 20, 2021
@drahnr @ordian
enable disputes for known chains, except for polkadot (#4464)
ad03b26 
* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>
ordian added a commit that referenced this pull request Dec 20, 2021
@ordian
Merge branch 'master' into ao-session-info-migration
cc70c2f 
* master:
  dispute-coordinator: fix underflow panic (#4557)
  Bump futures from 0.3.18 to 0.3.19 (#4567)
  Bump lru from 0.7.0 to 0.7.1 (#4566)
  Update Polkadot (#4561)
  Suggest installing graphviz before book building (#4565)
  [Zombienet] fix test creds (#4562)
  chain-api: stop ancestors lookup at block #0 (#4560)
  use v1.0.2 of zombienet (#4553)
  remove invalid dispute subsystem replace (#4559)
  Bump hyper from 0.14.15 to 0.14.16 (#4550)
  Create a README for XCMv2 detailing notable changes (#4059)
  enable disputes for known chains, except for polkadot (#4464)
  dispute statements node side limiting (#4541)
  Bump serde from 1.0.131 to 1.0.132 (#4551)
  Bump nix from 0.23.0 to 0.23.1 (#4552)
  Dispute coordinator: look for included candidates in non-finalized chain (#4508)
drahnr added a commit that referenced this pull request Jan 4, 2022
@drahnr @ordian
enable disputes for known chains, except for polkadot (#4464)
3d9200e 
* enable disputes, for all known chains but polkadot

* chore: fmt

* don't propagate disputes either

* review

* remove disputes feature

* remove superfluous line

* Update node/service/src/lib.rs

Co-authored-by: Andronik Ordian <[email protected]>

* fixup

* allow being a dummy

* rialto

* add an enum, to make things work better

* overseer

* fix test

* comments

* move condition out

* excess arg

Co-authored-by: Andronik Ordian <[email protected]>
@chevdor chevdor modified the milestones: v0.9.15, v0.9.16 Jan 11, 2022
@librelois librelois mentioned this pull request Feb 7, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ordian ordian ordian left review comments

@sandreim sandreim sandreim approved these changes

@slumber slumber slumber approved these changes

@rphmeier rphmeier rphmeier approved these changes

@eskimor eskimor eskimor approved these changes

@acatangiu acatangiu Awaiting requested review from acatangiu

@adoerr adoerr Awaiting requested review from adoerr

@antonio-dropulic antonio-dropulic Awaiting requested review from antonio-dropulic

@svyatonik svyatonik Awaiting requested review from svyatonik

@tomusdrw tomusdrw Awaiting requested review from tomusdrw

@chevdor chevdor Awaiting requested review from chevdor

Assignees

@drahnr drahnr

Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
v0.9.16
Development

Successfully merging this pull request may close these issues.
7 participants
@drahnr @rphmeier @ordian @eskimor @slumber @sandreim @chevdor