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

implement collation generation subsystem #1557

Merged
merged 25 commits into from
Aug 17, 2020
Merged

implement collation generation subsystem #1557

merged 25 commits into from
Aug 17, 2020

Conversation

coriolinus
Copy link
Contributor

Closes #1464.

@coriolinus coriolinus added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Aug 10, 2020
@coriolinus coriolinus self-assigned this Aug 10, 2020
@coriolinus coriolinus changed the title start sketching out a collation generation subsystem implement collation generation subsystem Aug 11, 2020
coriolinus
coriolinus commented Aug 11, 2020
View reviewed changes
Copy link
Contributor Author

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

@bkchr @rphmeier I think this subsystem is ready for a first look now. I had several questions resulting from some ambiguities in the guide, listed below. If everything's good, then I'll just need to add some tests before it's ready for real review.

node/collation-generation/src/lib.rs Outdated Show resolved Hide resolved
node/collation-generation/src/lib.rs Outdated Show resolved Hide resolved
node/collation-generation/src/lib.rs Outdated Show resolved Hide resolved
node/subsystem/src/messages.rs Outdated Show resolved Hide resolved
primitives/src/v1.rs Outdated Show resolved Hide resolved
@coriolinus coriolinus mentioned this pull request Aug 12, 2020
Closed
@coriolinus coriolinus mentioned this pull request Aug 13, 2020
Closed
@coriolinus
rustfmt
b306de3
@coriolinus
broken: work on fixing sender test
b40f639 
Unfortunately, for reasons that are not yet clear, despite the public key
and checked data being identical, the signer is not producing an identical
signature. This commit produces this output (among more):

signing with  Public(c4733ab0bbe3ba4c096685d1737a7f498cdbdd167a767d04a21dc7df12b8c858 (5GWHUNm5...))
checking with Public(c4733ab0bbe3ba4c096685d1737a7f498cdbdd167a767d04a21dc7df12b8c858 (5GWHUNm5...))
signed payload:  [4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 10, 0, 0, 0, c7, e5, c0, 64, 7a, db, fe, 44, 81, e5, 51, 11, 79, 9f, a5, 63, 93, 94, 3c, c4, 36, c6, 30, 36, c2, c5, 44, a2, 1b, db, b7, 82, 3, 17, a, 2e, 75, 97, b7, b7, e3, d8, 4c, 5, 39, 1d, 13, 9a, 62, b1, 57, e7, 87, 86, d8, c0, 82, f2, 9d, cf, 4c, 11, 13, 14]
checked payload: [4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 10, 0, 0, 0, c7, e5, c0, 64, 7a, db, fe, 44, 81, e5, 51, 11, 79, 9f, a5, 63, 93, 94, 3c, c4, 36, c6, 30, 36, c2, c5, 44, a2, 1b, db, b7, 82, 3, 17, a, 2e, 75, 97, b7, b7, e3, d8, 4c, 5, 39, 1d, 13, 9a, 62, b1, 57, e7, 87, 86, d8, c0, 82, f2, 9d, cf, 4c, 11, 13, 14]
@coriolinus
fix broken test
b87c4d6
@coriolinus
collation function returns commitments hash
ddbf460 
It doesn't look like we use the actual commitments data anywhere, and
it's not obvious if there are any fields of `CandidateCommitments`
not available to the collator, so this commit just assigns them the
entire responsibility of generating the hash.
@coriolinus coriolinus marked this pull request as ready for review August 14, 2020 12:54
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Aug 14, 2020
@coriolinus coriolinus mentioned this pull request Aug 14, 2020
Closed
@ordian ordian mentioned this pull request Aug 14, 2020
Merged
@coriolinus
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Aug 17, 2020

Missing process info; check that the PR belongs to a project column.

@coriolinus
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Aug 17, 2020

Missing process info; check that the PR belongs to a project column.

@coriolinus coriolinus merged commit e750a82 into master Aug 17, 2020
@coriolinus coriolinus deleted the prgn-collation-generation branch August 17, 2020 12:27
ordian added a commit that referenced this pull request Aug 17, 2020
@ordian
Merge branch 'master' into ao-prometheus-metrics
1dacee4 
* master:
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
ordian added a commit that referenced this pull request Aug 17, 2020
@ordian
Merge branch 'master' into ao-job-manager-dont-bring-down-subsystem-o…
91f1024 
…n-race-condition

* master:
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
@ordian ordian mentioned this pull request Aug 17, 2020
Merged
ordian added a commit that referenced this pull request Aug 17, 2020
@ordian
Merge branch 'master' into ao-refactor-primitives
32fefa6 
* master:
  overseer: fix build (#1596)
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
  Update .editorconfig to what we have in practice (#1545)
  Companion PR for substrate #6672 (#1560)
  pre-redenomination tockenSymbol change (#1561)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ordian ordian ordian left review comments

@rphmeier rphmeier rphmeier approved these changes

@bkchr bkchr Awaiting requested review from bkchr

Assignees

@coriolinus coriolinus

Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement: Collation Generation Subsystem
3 participants
@coriolinus @ordian @rphmeier