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.

Sign up for GitHub

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

Jump to bottom

Query osmo equivilent is staked via superfluid #1632

Merged
merged 16 commits into from
Jun 6, 2022
Merged

Query osmo equivilent is staked via superfluid #1632

merged 16 commits into from
Jun 6, 2022

Conversation

hieuvubk
Copy link
Contributor

Closes: #1525

What is the purpose of the change

Cli command
Rpc Query: SuperfluidOSMODelegationsByDelegator same as SuperfluidDelegationsByDelegator but return osmo value
Add test

@hieuvubk hieuvubk requested a review from a team May 31, 2022 21:15
@hieuvubk hieuvubk mentioned this pull request May 31, 2022
Merged
alexanderbez
alexanderbez reviewed May 31, 2022
View reviewed changes
proto/osmosis/superfluid/query.proto Outdated
@@ -70,6 +70,15 @@ service Query {
"superfluid_delegations/{delegator_address}";
}

// Returns all the superfluid poistions for a specific delegator
rpc SuperfluidOSMODelegationsByDelegator(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a mouthful haha. Any chance we can think of more abbreviated nomenclature.

Copy link
Contributor Author

@hieuvubk hieuvubk Jun 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

heee we can think of better names. How do u think about the logic

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to understand what adding OSMO to the query means? I read the original issue and I don't understand why we can't augment the current query (SuperfluidDelegationsByDelegator)?

Copy link
Contributor Author

@hieuvubk hieuvubk Jun 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@mattverse want to create a separate query (as i understant). Refer to #1539 (review)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should change to SuperfluidEquivilentDelegationsByDelegator?

@hieuvubk hieuvubk marked this pull request as draft June 1, 2022 15:04
@github-actions github-actions bot removed the C:CLI label Jun 2, 2022
@hieuvubk hieuvubk requested a review from alexanderbez June 2, 2022 15:47
alexanderbez
alexanderbez reviewed Jun 2, 2022
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

Looks much better :P

proto/osmosis/superfluid/query.proto Outdated
@@ -187,6 +187,10 @@ message SuperfluidDelegationsByDelegatorResponse {
(gogoproto.nullable) = false,
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins"
];
cosmos.base.v1beta1.Coin total_equivilent_staked_amount = 3 [
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cosmos.base.v1beta1.Coin total_equivilent_staked_amount = 3 [
cosmos.base.v1beta1.Coin total_equivalent_staked_amount = 3 [

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank u xD i changed that

@hieuvubk hieuvubk requested a review from alexanderbez June 3, 2022 04:01
@hieuvubk hieuvubk marked this pull request as ready for review June 4, 2022 16:10
ValarDragon
ValarDragon reviewed Jun 6, 2022
View reviewed changes
x/superfluid/keeper/grpc_query.go Outdated
Comment on lines 222 to 223
equivilentAmount := q.Keeper.GetSuperfluidOSMOTokens(ctx, baseDenom, lockedCoins.Amount)
coin := sdk.NewCoin(appparams.BaseCoinUnit, equivilentAmount)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
equivilentAmount := q.Keeper.GetSuperfluidOSMOTokens(ctx, baseDenom, lockedCoins.Amount)
coin := sdk.NewCoin(appparams.BaseCoinUnit, equivilentAmount)
equivalentAmount := q.Keeper.GetSuperfluidOSMOTokens(ctx, baseDenom, lockedCoins.Amount)
coin := sdk.NewCoin(appparams.BaseCoinUnit, equivalentAmount)

minor nit

ValarDragon
ValarDragon reviewed Jun 6, 2022
View reviewed changes
docs/core/proto-docs.md Outdated
@@ -3691,6 +3692,7 @@ assets
| ----- | ---- | ----- | ----------- |
| `superfluid_delegation_records` | [SuperfluidDelegationRecord](#osmosis.superfluid.SuperfluidDelegationRecord) | repeated | |
| `total_delegated_coins` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | |
| `total_equivilent_staked_amount` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | | |
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs to be regenerated due to the typo

ValarDragon
ValarDragon approved these changes Jun 6, 2022
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

LGTM once proto-doc / typo is sorted out.

Thanks for adding this feature! Want to make a Changelog update? (Feature in unreleased)

@ValarDragon ValarDragon added the A:backport/v9.x Do not use. backport patches to v9.x branch label Jun 6, 2022
@hieuvubk
Copy link
Contributor Author

hieuvubk commented Jun 6, 2022 •
edited
Loading

LGTM once proto-doc / typo is sorted out.

Thanks for adding this feature! Want to make a Changelog update? (Feature in unreleased)

Thank u, i missed. Also added entry to changelog.
Btw i dont know why running make proto-all, it's not rebuild the proto-docs 🤔

@hieuvubk hieuvubk merged commit 19fedeb into osmosis-labs:main Jun 6, 2022
mergify bot pushed a commit that referenced this pull request Jun 6, 2022
@hieuvubk @mergify
Query osmo equivilent is staked via superfluid (#1632)
134f56f 
* fix proto

* get osmo equivilent is staked via superfluid

* format

* fix go_fmt

* Revert "fix go_fmt"

This reverts commit 5407105.

* augment SuperfluidDelegationsByDelegator query

* remove fmt

* format

* fix spell

* fix spell, changelog

* query

(cherry picked from commit 19fedeb)

# Conflicts:
#	x/superfluid/keeper/grpc_query.go
@mergify mergify bot mentioned this pull request Jun 6, 2022
Merged
hieuvubk added a commit that referenced this pull request Jun 7, 2022
@mergify @hieuvubk
Query osmo equivilent is staked via superfluid (backport #1632) (#1676)
ddd75fe 
* Query osmo equivilent is staked via superfluid (#1632)

* fix proto

* get osmo equivilent is staked via superfluid

* format

* fix go_fmt

* Revert "fix go_fmt"

This reverts commit 5407105.

* augment SuperfluidDelegationsByDelegator query

* remove fmt

* format

* fix spell

* fix spell, changelog

* query

(cherry picked from commit 19fedeb)

# Conflicts:
#	x/superfluid/keeper/grpc_query.go

* resolved

* fix lint

Co-authored-by: Hieu Vu <[email protected]>
@github-actions github-actions bot mentioned this pull request Mar 15, 2024
Closed
@github-actions github-actions bot mentioned this pull request Nov 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ValarDragon ValarDragon ValarDragon approved these changes

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

Assignees
No one assigned
Labels
A:backport/v9.x Do not use. backport patches to v9.x branch C:x/superfluid
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support superfluid staking queries
3 participants
@hieuvubk @alexanderbez @ValarDragon