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

Adding token factory specific denom endpoint #457

Merged
merged 9 commits into from
Mar 14, 2024

Conversation

dssei
Copy link
Contributor

@dssei dssei commented Mar 12, 2024

Describe your changes and provide context

When we create tokens using token factory, denoms are created in the factory/{owner_walltet_id}/{denom} format
E.g. factory/sei1gxskuzvhr4s8sdm2rpruaf7yx2dnmjn0zfdu9q/NEWCOIN

When querying metadata via /cosmos/bank/v1beta1/denoms_metadata/{denom} endpoint for such a denom we are getting error like

{
    "code": 12,
    "message": "Not Implemented",
    "details": []
}

This happens, because routing logic splits the denom uri param into 3 components rather then one and hence tries to router request to a non-existing handler. Url encoding does not help either as it still results in same issue. We would like to add additional endpoint to handle token factory created metadata retrieval, and move denom for the endpoint from uri param to query param.

so then for token factory created tokens the query would be
/cosmos/bank/v1beta1/token_factory/denoms_metadata?denom=factory/{owner_walltet_id}/{denom}

Testing performed to validate your change

Note: some demom tests were disables, and I have enabled them.

  • Functional testing on local node with updated source
  • unit/integration tests

…ormat factory/<owner_wallet>/<denom> in a separate query param rather than URI param.
@dssei dssei self-assigned this Mar 12, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.52%. Comparing base (0320b34) to head (5411a07).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #457      +/-   ##
==========================================
+ Coverage   55.46%   55.52%   +0.05%     
==========================================
  Files         629      629              
  Lines       53541    53543       +2     
==========================================
+ Hits        29695    29728      +33     
+ Misses      21733    21700      -33     
- Partials     2113     2115       +2     
Files Coverage Δ
x/bank/keeper/grpc_query.go 70.00% <100.00%> (+21.44%) ⬆️

... and 2 files with indirect coverage changes

@dssei dssei marked this pull request as ready for review March 13, 2024 17:29
…41_additional_denom_ednpoint_for_tokenfoundry
@dssei dssei merged commit ca54bb7 into main Mar 14, 2024
13 checks passed
@dssei dssei deleted the SEI-5441_additional_denom_ednpoint_for_tokenfoundry branch March 14, 2024 16:32
dssei added a commit that referenced this pull request Mar 14, 2024
dssei added a commit that referenced this pull request Mar 19, 2024
Reverts #457 in favor of
sei-protocol/sei-chain#1444 as tokenfactory is
more appropriate module for the endpoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants