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.

Sign up for GitHub

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

Jump to bottom

Improve try-state developer experience & fix bug #2019

Merged
merged 8 commits into from
Oct 30, 2023
Merged

Improve try-state developer experience & fix bug #2019

merged 8 commits into from
Oct 30, 2023

Conversation

liamaharon
Copy link
Contributor

Making some devex improvements as I audit our chains adherence to try-state invariants, in preparation for automated try-state checks and alerting.

Note to reviewer: while you're here, if you have time would be great to get your eyes on #1297 also since it touches a similar file and I'd like to avoid merge conflicts :P

Devex Improvements

  • Changes the log level of logs informing the user that try-state checks are being run for a pallet from debug to info
  • Improves how errors are communicated
    • Errors are logged when they are encountered, rather than after everything has been executed
    • Exact pallet the error originated from is included with the error log
    • Clearly see all errors and how many there are, rather than only one
    • Closes try-state: should indicate which pallet is failing #136

Example of new logs

Screenshot 2023-10-25 at 15 44 44

Same but with old logs (run with RUST_LOG=debug)

Notice only informed of one of the errors, and it's unclear which pallet it originated

Screenshot 2023-10-25 at 15 39 01

Bug fix

When dry-running migrations and checks.try_state() is true, only run try_state checks after migrations have been executed. Otherwise, try_state checks that expect state to be in at a HIGHER storage version than is on-chain could incorrectly fail.

@liamaharon liamaharon added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Oct 25, 2023
@liamaharon liamaharon requested review from kianenigma, bkchr and a team October 25, 2023 04:46
@liamaharon liamaharon mentioned this pull request Oct 25, 2023
Closed
4 tasks
@liamaharon
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Oct 26, 2023 •
edited
Loading

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

Comment bot cancel 11-f4828636-049c-4636-b626-b73a4585b353 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Oct 26, 2023

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

@liamaharon liamaharon mentioned this pull request Oct 26, 2023
Closed
11 tasks
@liamaharon liamaharon enabled auto-merge (squash) October 30, 2023 07:46
@liamaharon liamaharon merged commit d715caa into master Oct 30, 2023
103 of 106 checks passed
@liamaharon liamaharon deleted the liam-improve-try-state-tuple-internals branch October 30, 2023 08:43
@4meta5 4meta5 mentioned this pull request Feb 1, 2024
Merged
@github-actions github-actions bot mentioned this pull request Feb 19, 2024
Closed
@github-actions github-actions bot mentioned this pull request Mar 13, 2024
Closed
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@liamaharon
Improve try-state developer experience & fix bug (paritytech#2019)
6819591 
Making some devex improvements as I audit our chains adherence to
try-state invariants, in preparation for automated try-state checks and
alerting.

Note to reviewer: while you're here, if you have time would be great to
get your eyes on paritytech#1297
also since it touches a similar file and I'd like to avoid merge
conflicts :P

## Devex Improvements

- Changes the log level of logs informing the user that try-state checks
are being run for a pallet from debug to info
- Improves how errors are communicated
- Errors are logged when they are encountered, rather than after
everything has been executed
- Exact pallet the error originated from is included with the error log
  - Clearly see all errors and how many there are, rather than only one
  - Closes paritytech#136 

### Example of new logs

<img width="1185" alt="Screenshot 2023-10-25 at 15 44 44"
src="https://github.com/paritytech/polkadot-sdk/assets/16665596/b75588a2-1c64-45df-bbc8-bcb8bf8b0fe0">

### Same but with old logs (run with RUST_LOG=debug)

Notice only informed of one of the errors, and it's unclear which pallet
it originated

<img width="1185" alt="Screenshot 2023-10-25 at 15 39 01"
src="https://github.com/paritytech/polkadot-sdk/assets/16665596/e3429cb1-489e-430a-9716-77c052e5dae6">
 

## Bug fix

When dry-running migrations and `checks.try_state()` is `true`, only run
`try_state` checks after migrations have been executed. Otherwise,
`try_state` checks that expect state to be in at a HIGHER storage
version than is on-chain could incorrectly fail.

---------

Co-authored-by: command-bot <>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 26, 2024
@svyatonik
only refund if all bundled messages have been delivered (paritytech#2019
4bb1a64 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Mar 27, 2024
@svyatonik
only refund if all bundled messages have been delivered (paritytech#2019
172830a 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik
only refund if all bundled messages have been delivered (paritytech#2019
7b2aa04 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
ede3e24 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
18c8655 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
2744e7a 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
7302851 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
d616833 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
526d002 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
f65fec6 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
18f302d 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
9a1a1b8 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
016b6a5 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
6a3d001 
)
serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024
@svyatonik @serban300
only refund if all bundled messages have been delivered (paritytech#2019
2768b1c 
)
bkchr pushed a commit that referenced this pull request Apr 10, 2024
@svyatonik @bkchr
only refund if all bundled messages have been delivered (#2019)
4d29753
This was referenced Jun 5, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@franciscoaguirre franciscoaguirre franciscoaguirre approved these changes

@gupnik gupnik gupnik approved these changes

@ggwpez ggwpez ggwpez approved these changes

@kianenigma kianenigma kianenigma approved these changes

@bkchr bkchr Awaiting requested review from bkchr

Assignees
No one assigned
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

try-state: should indicate which pallet is failing
5 participants
@liamaharon @franciscoaguirre @kianenigma @ggwpez @gupnik