-
Notifications
You must be signed in to change notification settings - Fork 586
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
feat: add metadata for IBC tokens #3104
Merged
Merged
Changes from 5 commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
b1359b2
add metadata after mint
0xmuralik 4070c24
set base denom as ibc hash denom
0xmuralik d0c2b83
comment
0xmuralik d0662a4
remove unsed funcs from keeper
0xmuralik ef8310a
Merge branch 'main' into murali/ibc-denom
0xmuralik e460e35
genesis setmetadata
0xmuralik 850ded9
set metadata in migration
0xmuralik 9e7924a
Merge branch 'main' into murali/ibc-denom
0xmuralik e257dcb
Merge branch 'main' into murali/ibc-denom
0xmuralik 21c81e5
new migration
0xmuralik dafd5f1
test new migration
0xmuralik 7763f74
Merge branch 'main' into murali/ibc-denom
0xmuralik a2b159f
Update modules/apps/transfer/keeper/migrations.go
0xmuralik 663a0fd
import seperation
0xmuralik 737cba5
replace MetaData with Metadata
0xmuralik c30d23d
Merge branch 'main' of https://github.com/0xmuralik/ibc-go into mural…
0xmuralik d894142
Merge branch 'murali/ibc-denom' of https://github.com/0xmuralik/ibc-g…
0xmuralik 1640753
Merge branch 'main' into murali/ibc-denom
crodriguezvega 9501c78
fix typo
crodriguezvega de08338
add initial migration documentation
crodriguezvega 324986c
fixed tests
crodriguezvega e4e24d2
add logging message
crodriguezvega 8001ff5
Merge branch 'main' into murali/ibc-denom
crodriguezvega ff7e7ff
Merge branch 'main' into murali/ibc-denom
DimitrisJim dac13bb
Bump consensus version.
DimitrisJim d16383b
Nits:
DimitrisJim c29568a
Merge branch 'main' into murali/ibc-denom
crodriguezvega 17f23ae
Merge branch 'main' into murali/ibc-denom
DimitrisJim 771dd3c
addressing some of the feedback
crodriguezvega e94f138
add more test cases
crodriguezvega 2794a64
rename variables
crodriguezvega b97b8ee
check metadata in onrecvpacket tests
crodriguezvega bcfe839
fix: remove test assertion on errors, state will be reverted by basea…
colin-axner b8e0f63
Merge branch 'main' into murali/ibc-denom
crodriguezvega 7091998
address review comment
crodriguezvega ab7c94f
lint, lint, lint
crodriguezvega a433aad
Merge branch 'main' into murali/ibc-denom
crodriguezvega ca1304b
Merge branch 'main' into murali/ibc-denom
colin-axner 1c2626d
fix test
crodriguezvega 0e1c34e
Merge branch 'main' into murali/ibc-denom
crodriguezvega b22484a
address review comments
crodriguezvega ec3d294
Merge branch 'main' into murali/ibc-denom
crodriguezvega aaa6786
Merge branch 'main' into murali/ibc-denom
crodriguezvega File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
if we are running migrations such that all traces have metadata set, shouldn't we just make setting metadata a functionality of
SetDenomTrace
? This avoids accidental incidents where the trace is set but the metadata isn't. There should never be an instance when you want to set the denom trace without setting the metadata as well?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 guess we could do that, although
SetDenomTrace
is a setter function for a specific key in the store, so it would squeak to me a bit if we addSetDenomMetadata
there as well. Maybe we could have a helper function that wraps bothSetDenomTrace
andSetDenomMetadata
. Happy to hear other people's thoughts.