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

fix: add old supply offset functions #629

Merged
merged 4 commits into from
Oct 15, 2024
Merged

Conversation

PaddyMc
Copy link

@PaddyMc PaddyMc commented Oct 2, 2024

Description

Exposing functions to allow Osmosis to access the old supply offsets

Osmosis PR => osmosis-labs/osmosis#8751

This comment has been minimized.

Comment on lines +38 to +41

// SupplyOffKeyOld is a Osmosis specific key that handles supply offsets pre v0.50
// TODO: Remove in the v28 upgrade
SupplyOffsetKeyOld = []byte{0x88}
Copy link
Member

Choose a reason for hiding this comment

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

I just want to verify that you checked NewPrefix 88 does not collide with []byte{0x88}

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I did, this is the bug

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.

ACK, we might want to add this to Osmosis specific changes on changelog

Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

LGTM

x/bank/keeper/supply_offset_old.go Outdated Show resolved Hide resolved
@PaddyMc PaddyMc merged commit 2c466dc into osmo-v27/0.50.10 Oct 15, 2024
44 of 45 checks passed
@PaddyMc PaddyMc deleted the fix/supply-offsets branch October 15, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants