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

Removes ReportsByKindIndex #13936

Merged
merged 17 commits into from
Apr 25, 2023
Merged

Removes ReportsByKindIndex #13936

merged 17 commits into from
Apr 25, 2023

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Apr 18, 2023 •
edited
Loading

Relates to https://github.com/paritytech/srlabs_findings/issues/224. I don't think we actually need ReportsByKind. We already store ConcurrentReportsIndex which could be iterated off-chain if we want to get all reports.

polkadot companion: paritytech/polkadot#7114

@gupnik gupnik added A0-please_review Pull request needs code review. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited B1-note_worthy Changes should be noted in the release notes C5-high PR touches the given topic and has a high impact on builders. labels Apr 18, 2023
@gupnik gupnik added the T1-runtime This PR/Issue is related to the topic “runtime”. label Apr 18, 2023
bkchr
bkchr approved these changes Apr 21, 2023
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Yeah, I also don't see any reason for this storage item. ConcurrentReportsIndex can be used to create the same structure any way.

@bkchr
Copy link
Member

bkchr commented Apr 21, 2023

Ahh and that will require a migration to delete the old storage entries.

@bkchr bkchr added the E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. label Apr 21, 2023
@gupnik
Copy link
Contributor Author

gupnik commented Apr 22, 2023

Ahh and that will require a migration to delete the old storage entries.

@bkchr Added the migration. Could you please take a look?

@gupnik gupnik mentioned this pull request Apr 22, 2023
Merged
bkchr
bkchr approved these changes Apr 22, 2023
View reviewed changes
frame/offences/src/migration.rs Outdated Show resolved Hide resolved
frame/offences/src/migration.rs Outdated Show resolved Hide resolved
frame/offences/src/migration.rs Outdated Show resolved Hide resolved
@gupnik
Copy link
Contributor Author

gupnik commented Apr 25, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Apr 25, 2023 •
edited
Loading

@gupnik https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2724252 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 52-ead506d2-e4a0-4681-b87a-05d3b9150cac to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 25, 2023

@gupnik Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2724252 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2724252/artifacts/download.

@gupnik
Copy link
Contributor Author

gupnik commented Apr 25, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Apr 25, 2023 •
edited
Loading

@gupnik https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2727997 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 59-3714f044-78cb-4f83-8d71-21f58c317047 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Apr 25, 2023

@gupnik Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2727997 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2727997/artifacts/download.

@gupnik
Copy link
Contributor Author

gupnik commented Apr 25, 2023

bot merge

@paritytech-processbot
Copy link

Error: Github API says paritytech/polkadot#7114 is not mergeable

@gupnik
Copy link
Contributor Author

gupnik commented Apr 25, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit db6ebf5 into master Apr 25, 2023
@paritytech-processbot paritytech-processbot bot deleted the gupnik/offences-pallet-updates branch April 25, 2023 15:21
@ggwpez
Copy link
Member

ggwpez commented Apr 28, 2023

This is in release .42 without an audit. Please be careful when merging stuff that needs audit before that is done.

@ggwpez ggwpez added C3-medium PR touches the given topic and has a medium impact on builders. and removed C5-high PR touches the given topic and has a high impact on builders. labels Apr 28, 2023
@gupnik
Copy link
Contributor Author

gupnik commented Apr 29, 2023

This is in release .42 without an audit. Please be careful when merging stuff that needs audit before that is done.

Yes, I have raised it for audit and it is being looked at priority.

@gupnik gupnik mentioned this pull request Apr 30, 2023
Open
4 tasks
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels May 2, 2023
gpestana pushed a commit that referenced this pull request May 4, 2023
@gupnik @gpestana
Removes ReportsByKindIndex (#13936)
4286a74 
* Removes ReportsByKind

* Minor build fixes

* adds migration

* Addresses review comment

* Uses clear but weight check fails

* Uses unique

* Updates test to commit the change before migration

* Uses reads_writes

* ".git/.scripts/commands/fmt/fmt.sh"

* Fixes build

* Addresses review comments

* ".git/.scripts/commands/fmt/fmt.sh"

* fixes typo

---------

Co-authored-by: command-bot <>
@mattheworris mattheworris mentioned this pull request Jun 12, 2023
Merged
16 tasks
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
Closed
mattheworris added a commit to frequency-chain/frequency that referenced this pull request Jun 15, 2023
@mattheworris @rlaferla
Polkadot 0939 to 0942 upgrade (#1590)
6d9e542 
# Goal
The goal of this PR is upgrade Polkadot to v0.9.42

Polkadot Release Notes:
https://github.com/paritytech/polkadot/releases/tag/v0.9.42
https://github.com/paritytech/polkadot/releases/tag/v0.9.41
https://github.com/paritytech/polkadot/releases/tag/v0.9.40

Polkadot Release Analysis:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-41-v0-9-42/2828/1

Closes #1472 
Closes #1270 
Closes #1332 

# Discussion
- v0.9.40 was not working and there was evidence that changes in v0.9.42
were related, so we decided to jump ahead to v0.9.42
<!-- List discussion items -->
# Runtime Migrations Included
- [x] paritytech/polkadot#6937
- [x] paritytech/substrate#13715
- [x] paritytech/substrate#13936
- [x] paritytech/polkadot#7114
- [x] Further all migrations from 9.38+ are included:
paritytech/polkadot#7162)
# Checklist
- [x] Chain spec updated
- [ ] Custom RPC OR Runtime API added/changed? Updated js/api-augment.
- [ ] Design doc(s) updated
- [ ] Tests added
- [ ] Benchmarks added
- [x] Weights updated
# Tests Performed
- [x] `make ci-local` -- Passing (includes lint, docs, unit-test and
integration-test)
- [x] `make start-native` -- Successfully attached debugger when
creating MSA with Polkadot UI
- [x] `make start-relay` -- Docker Containers successfully started, but
too slow in emulation on Apple Silicon M2.
- [x] `make upgrade-local` -- Successfully started local relay network
and upgraded to a newer test runtime.
- [x] `make upgrade-local` -- Successfully updated a node running the
current version on branch `main`

---------

Co-authored-by: Frequency CI [bot] <do-not-reply@users.noreply.github.com>
Co-authored-by: Matthew Orris <--help>
Co-authored-by: Robert La Ferla <robert@onemsg.co>
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@gupnik @nathanwhit
Removes ReportsByKindIndex (paritytech#13936)
bbecda8 
* Removes ReportsByKind

* Minor build fixes

* adds migration

* Addresses review comment

* Uses clear but weight check fails

* Uses unique

* Updates test to commit the change before migration

* Uses reads_writes

* ".git/.scripts/commands/fmt/fmt.sh"

* Fixes build

* Addresses review comments

* ".git/.scripts/commands/fmt/fmt.sh"

* fixes typo

---------

Co-authored-by: command-bot <>
@ukint-vs ukint-vs mentioned this pull request Aug 26, 2023
Closed
2 tasks
@ukint-vs ukint-vs mentioned this pull request Oct 24, 2023
Closed
2 tasks
@ukint-vs ukint-vs mentioned this pull request Dec 12, 2023
Merged
@surangap surangap mentioned this pull request May 28, 2024
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ggwpez ggwpez ggwpez approved these changes

@bkchr bkchr bkchr approved these changes

@andresilva andresilva andresilva approved these changes

@kianenigma kianenigma Awaiting requested review from kianenigma

@davxy davxy Awaiting requested review from davxy

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C3-medium PR touches the given topic and has a medium impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gupnik @bkchr @ggwpez @andresilva @jakoblell @codekitz