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.

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

chore: Remove burn logs from Bank module #432

Merged
merged 2 commits into from
Apr 27, 2023

Conversation

mattverse
Copy link
Member

Closes: osmosis-labs/osmosis#4540

What is the purpose of the change

Removes Burn Logs from Bank module.

We have alot of incidences where we Burn coins, for example, whenever user exits a pool, it would burn the pool shares, triggering this log layer.

We also have future incidences where we will have multiples of simultaneous burns, e,g when we are migrating shares from one pool to other. This would cause log bombs in nodes as shown in the original issue.

This PR removes log bombs caused from burns.

Brief Changelog

  • Remove burn logs from Bank module.

Testing and Verifying

This change does not need tests

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@mattverse mattverse requested a review from a team April 14, 2023 08:33
@czarcas7ic
Copy link
Member

Is there any desire to keep this as a log but for debug mode only?

@mattverse
Copy link
Member Author

@czarcas7ic yes definitely! Nice suggestion, changed!

@mattverse mattverse changed the title misc: Remove burn logs from Bank module chore: Remove burn logs from Bank module Apr 27, 2023
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

LGTM!

@czarcas7ic czarcas7ic merged commit 6ae3ebb into osmosis-main Apr 27, 2023
@czarcas7ic czarcas7ic deleted the mattverse/remove-burn-logs branch April 27, 2023 15:14
mergify bot pushed a commit that referenced this pull request Apr 27, 2023
* Remove burn logs

* Change to debug log

(cherry picked from commit 6ae3ebb)
mattverse added a commit that referenced this pull request Apr 28, 2023
* Remove burn logs

* Change to debug log

(cherry picked from commit 6ae3ebb)

Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[CL Migrations] Remove log bombs
2 participants