fix: add indirect incentivized pools to incentivized pools query #5589
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. 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. 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. Consider creating a There was a problem hiding this comment. 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. do we never expect PoolId to be a CL pool Id? There was a problem hiding this comment. 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