-
Notifications
You must be signed in to change notification settings - Fork 680
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
im-online
removal cleanup: remove off-chain storage
#2290
Conversation
im-online
removal cleanup: remove off-chain storageim-online
removal cleanup: remove off-chain storage
Tested, it and it seems to work, but maybe I'm over-engineering it... Are there any better ideas on how to implement it without sticking a whole fake pallet into the runtime? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests?
I've done manual testing. I could automate that, but that seems to be overkill for a disposable fake pallet that will only fire once. Besides, I'm not even sure that this is the right way to do what I want to do, and I'm still waiting for someone to come and say, "you're doing it totally wrong, see how it should be" 🙂 |
What was your process for testing it, could you post a summary of the results? Regarding the approach, I wonder if it's possible to use a runtime migration rather than a pallet? If not, probably deserves an issue. |
I've spun up a zombienet, then applied runtime upgrade with However, this approach has some significant drawbacks. First, I'm sticking a whole fake pallet into the runtime just to remove several values from offchain storage. I'd expect an easier mechanism for that, but I didn't find one. That pallet has to be removed afterward. Second, until it is removed, it will fire an offchain worker every next block. It will do close to nothing (get the block number from on-chain storage; check we're not at that block; exit), but still, it's completely unneeded overhead. Unfortunately, regular migration doesn't work here, as the offchain storage can only be accessed from an offchain worker, not from the runtime itself. That's the problem I'm trying to overcome with this fake pallet, and I'd be glad to hear that there's some cleaner solution for that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a FRAME expert, but if it's tested to work, sounds good to me :)
@bkchr Rococo and Westend have been running without |
…ch/polkadot-sdk into s0me0ne/retire-im-online-part-3
* master: (41 commits) Add Coretime to Westend (#3319) removed `pallet::getter` from `pallet-sudo` (#3370) gossip-support: add unittests for update authorities (#3258) [FRAME Core] remove unnecessary overrides while using derive_impl for frame_system (#3317) Update coretime-westend bootnodes (#3380) `im-online` removal cleanup: remove off-chain storage (#2290) Bump the known_good_semver group with 1 update (#3379) Fix documentation dead link (#3372) Make `sp-keystore` `no_std`-compatible and fix the `build-runtimes-polkavm` CI job (#3363) Remove unused `im-online` weights (#3373) Ensure referenda `TracksInfo` is sorted (#3325) rpc server: add rate limiting middleware (#3301) do not block finality for "disabled" disputes (#3358) fix(zombienet): docker `img` version to use in merge queues for bridges (#3337) Various nits and alignments for SP testnets found during bumping `polkadot-fellows` repo (#3359) Add broker pallet to `coretime-westend` (#3272) remove recursion limit (#3348) Update subkey README.md (#3355) Bump the known_good_semver group with 6 updates (#3347) Document LocalKeystore insert method (#3336) ...
This is a follow-up for `im-online` pallet removal that is cleaning up its off-chain storage. Must be merged no earlier than paritytech#2265 is enacted. Related: paritytech#1964 --------- Co-authored-by: Bastian Köcher <[email protected]>
Removes transient code introduced to clean up offchain database after `im-online` pallet removal. Should be merged after #2290 has been enacted.
Removes transient code introduced to clean up offchain database after `im-online` pallet removal. Should be merged after #2290 has been enacted.
Removes transient code introduced to clean up offchain database after `im-online` pallet removal. Should be merged after paritytech#2290 has been enacted.
This is a follow-up for
im-online
pallet removal that is cleaning up its off-chain storage. Must be merged no earlier than #2265 is enacted. Related: #1964