Closed SpicyLemon closed 3 weeks ago
This pull request introduces changes to suppress event emissions during the metadata migration process and implements a no-operation event manager. The Migrate3To4
function in the keeper
package is modified to utilize this no-op event manager unless the context indicates a testnet environment. Additionally, new interfaces and structs are defined to facilitate the migration process, ensuring proper handling of value owner data. The changes focus on enhancing the migration logic while maintaining the overall structure of the migration process.
File Path | Change Summary |
---|---|
.changelog/unreleased/improvements/2195-hide-md-mig-events.md | Documented the suppression of events during metadata migration. |
internal/sdk/events.go | Introduced NoOpEventManager struct implementing sdk.EventManagerI with various no-op methods. |
x/metadata/keeper/migrations_v4.go | Modified Migrate3To4 to use a no-op event manager; added keeper3To4I interface and struct. |
🐇 In the meadow where events once danced,
A hush now falls, as silence pranced.
With no-op magic, we softly tread,
Migrating data, where once it led.
A hop, a skip, through versions we roam,
In quietude, we find our home. 🌼
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Description
This PR updates the metadata module migration from v3 to v4 (to move value-owner records into the bank module). It will no longer emit any events when run on a mainnet node. Any events for other parts of the upgrade will still be included, though, just not the events from the metadata module migration.
For each scope value owner, there is at least 6 events (more if they've opted into quarantine). Both mainnet and testnet have around 300,000 scopes and most (if not all) have a value owner. That's around 1.8M events that would be emitted.
On testnet, when we ran the
viridian-rc1
upgrade, it included all of those events in the block-results for the upgrade height. I had to write a custom executable to extract it, and as JSON, it was almost 400MB. We could not find a way to allow a node to actually return that info via the block-results query.In some unit tests I wrote and played with, reading the block result from state and unmarshalling it always took less than half a second. But that's a small fraction of the amount of time between request and timeout. My theory is that some limit is being hit, causing a panic in a sub-process that isn't being properly handled. When the panic happens, the request processing is halted and the node never sends any sort of response, leading ultimately to a timeout from something. Further, I believe that when that panic happens, there's memory that has been allocated, but not released, leading to a memory leak. This might explain why some modes had memory problems (some causing system crashes) after the upgrade.
So, to be on the safe side, we'll omit the events from the metadata module migration.
Also, so that
v1.20.0-rc2
andv1.20.0
can be state compatible, this change only affects non-testnet nodes. That way, if you're recreating a node, playing through all the blocks, then, on theviridian-rc1
upgrade height, you can switch tov1.20.0
and get the correct result. I.e. for a testnet node, all the events will still be emitted during the metadata v3 to v4 module migration.Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why.
docs/
) or specification (x/<module>/spec/
).godoc
comments..changelog/unreleased
(see Adding Changes).Files changed
in the Github PR explorer.Codecov Report
in the comment section below once CI passes.Summary by CodeRabbit
New Features
Bug Fixes
Documentation