-
Notifications
You must be signed in to change notification settings - Fork 589
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 indirect incentivized pools to incentivized pools query #5589
Changes from 5 commits
5431164
ceccc51
81ce026
ea3788a
28f7ee9
bba673d
42a2e17
e70ac41
18ee857
1ebe6dc
b7e3225
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ package keeper | |
|
||
import ( | ||
"context" | ||
"time" | ||
|
||
"google.golang.org/grpc/codes" | ||
"google.golang.org/grpc/status" | ||
|
@@ -157,6 +158,45 @@ func (q Querier) IncentivizedPools(ctx context.Context, _ *types.QueryIncentiviz | |
} | ||
} | ||
|
||
// Retrieve the migration records between balancer pools and concentrated liquidity pools. | ||
// This comes from the superfluid keeper, since superfluid is the only pool incentives connected | ||
// module that has access to the gamm modules store. | ||
migrationRecords, err := q.superfluidKeeper.GetAllMigrationInfo(sdkCtx) | ||
if err != nil { | ||
return nil, status.Error(codes.Internal, err.Error()) | ||
} | ||
|
||
// Iterate over all migration records. | ||
for _, record := range migrationRecords.BalancerToConcentratedPoolLinks { | ||
linkedClPoolIsIncentivized := false | ||
var duration time.Duration | ||
|
||
// If the linked concentrated liquidity pool is in the list of incentivized pools, | ||
// note this and add its balancer counterpart to the list of incentivized pools. | ||
for _, incentivizedPool := range incentivizedPools { | ||
if incentivizedPool.PoolId == record.ClPoolId { | ||
linkedClPoolIsIncentivized = true | ||
duration = incentivizedPool.LockableDuration | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it correct to use this for duration? AFAIR, we update the largest lockable duration gauge for the balancer pool while using incentivized epoch duration for CL pool which might not necessarily match There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its just a holder value, but we can set this as the longest duration just in case, because I agree that is "default" Done here 18ee857 |
||
continue | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider creating a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, added here bba673d |
||
|
||
if !linkedClPoolIsIncentivized { | ||
continue | ||
} | ||
|
||
gaugeId, err := q.Keeper.GetPoolGaugeId(sdkCtx, record.BalancerPoolId, duration) | ||
if err == nil { | ||
incentivizedPool := types.IncentivizedPool{ | ||
PoolId: record.BalancerPoolId, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we never expect PoolId to be a CL pool Id? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In every case the balancer pool will be the pool that is getting indirectly incentivized. This means that there is never a case in which a CL pool does not appear in the incentivized pools list that should be there. That is why we only retrieve the balancer pools at the end, since they are the ones that are getting replaced and funded via the CL gauge |
||
LockableDuration: duration, | ||
GaugeId: gaugeId, | ||
} | ||
|
||
incentivizedPools = append(incentivizedPools, incentivizedPool) | ||
} | ||
} | ||
|
||
return &types.QueryIncentivizedPoolsResponse{ | ||
IncentivizedPools: incentivizedPools, | ||
}, nil | ||
|
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.
This was sadly the cleanest way knew to get passed the circular dependency issue. The only module that pool-incentives had access to that could access gamm's store was superfluid. So I basically called GetAllMigrationInfo in pool-incentives via superfluid and parsed the result into an identical struct. Please lmk if there is a better way to achieve this without a major refactor.
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.
this might sound a bit unorthodox but how about we create a separate file that store shared interfaces between modules
for instance;
gamm
pool-incentives
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 agree with this! I think it makes it simpler to reason about when we come back around and resolve this with the solution Roman recommended. Added here e70ac41