Skip to content

Commit 246dbc7

Browse files
fix(poolmanager): only write to cache in finalize execution mode (#9332) (#9337)
* fix(poolmanager): only write to cache in finalize execution mode Prevents the cache from being written to during other execution modes like CheckTx, which can lead to inconsistent state during transaction processing. The cache should only be populated during the final execution phase. * docs: add changelog entry for PR #9332 * fix(poolmanager): refactor GetPoolType to directly use state storage - Add getPoolRouteRaw helper to directly get pool route from storage - Modify GetPoolType to not rely on the cache via GetPoolModule - Update tests to verify behavior in both Check and Finalize execution modes - Fix test setup to ensure consistent state across execution modes --------- Co-authored-by: ghost <paddymchale@hotmail.com> (cherry picked from commit 140a57d) Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
1 parent 2321b09 commit 246dbc7

File tree

3 files changed

+69
-46
lines changed

3 files changed

+69
-46
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5656
### State Compatible
5757

5858
* [#9334](https://github.com/osmosis-labs/osmosis/pull/9334) fix: iavl pruning issue move onto next version if stuck
59+
* [#9332](https://github.com/osmosis-labs/osmosis/pull/9332) fix(poolmanager): only write to cache in finalize execution mode
5960

6061
## v28.0.5
6162

@@ -64,7 +65,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6465
* [#9050](https://github.com/osmosis-labs/osmosis/pull/9050) chore: bump ibc-go to v8.7.0
6566
* [#9006](https://github.com/osmosis-labs/osmosis/pull/9006) feat: CosmWasm Pool raw state query
6667

67-
6868
## v28.0.4
6969

7070
### State Breaking

x/poolmanager/create_pool.go

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,19 @@ func (k Keeper) getNextPoolIdAndIncrement(ctx sdk.Context) uint64 {
150150
return nextPoolId
151151
}
152152

153+
func (k *Keeper) getPoolRouteRaw(ctx sdk.Context, poolId uint64) (*types.ModuleRoute, error) {
154+
store := ctx.KVStore(k.storeKey)
155+
moduleRoute := &types.ModuleRoute{}
156+
found, err := osmoutils.Get(store, types.FormatModuleRouteKey(poolId), moduleRoute)
157+
if err != nil {
158+
return nil, err
159+
}
160+
if !found {
161+
return nil, types.FailedToFindRouteError{PoolId: poolId}
162+
}
163+
return moduleRoute, nil
164+
}
165+
153166
func (k *Keeper) SetPoolRoute(ctx sdk.Context, poolId uint64, poolType types.PoolType) {
154167
store := ctx.KVStore(k.storeKey)
155168
osmoutils.MustSet(store, types.FormatModuleRouteKey(poolId), &types.ModuleRoute{PoolType: poolType})
@@ -167,11 +180,11 @@ type poolModuleCacheValue struct {
167180
func (k *Keeper) GetPoolType(ctx sdk.Context, poolId uint64) (types.PoolType, error) {
168181
poolModuleCandidate, cacheHit := k.cachedPoolModules.Load(poolId)
169182
if !cacheHit {
170-
_, err := k.GetPoolModule(ctx, poolId)
183+
moduleRoute, err := k.getPoolRouteRaw(ctx, poolId)
171184
if err != nil {
172185
return 0, err
173186
}
174-
poolModuleCandidate, _ = k.cachedPoolModules.Load(poolId)
187+
return moduleRoute.PoolType, nil
175188
}
176189
v, _ := poolModuleCandidate.(poolModuleCacheValue)
177190
if cacheHit {
@@ -212,13 +225,15 @@ func (k *Keeper) GetPoolModule(ctx sdk.Context, poolId uint64) (types.PoolModule
212225
return nil, types.UndefinedRouteError{PoolType: moduleRoute.PoolType, PoolId: poolId}
213226
}
214227

215-
k.cachedPoolModules.Store(poolId, poolModuleCacheValue{
216-
pooltype: moduleRoute.PoolType,
217-
module: swapModule,
218-
gasFlat: gasFlat,
219-
gasKey: gasKey,
220-
gasValue: gasVal,
221-
})
228+
if ctx.ExecMode() == sdk.ExecModeFinalize {
229+
k.cachedPoolModules.Store(poolId, poolModuleCacheValue{
230+
pooltype: moduleRoute.PoolType,
231+
module: swapModule,
232+
gasFlat: gasFlat,
233+
gasKey: gasKey,
234+
gasValue: gasVal,
235+
})
236+
}
222237

223238
return swapModule, nil
224239
}

x/poolmanager/create_pool_test.go

Lines changed: 44 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -226,42 +226,50 @@ func (s *KeeperTestSuite) TestCreatePool() {
226226
},
227227
}
228228

229-
for i, tc := range tests {
230-
s.Run(tc.name, func() {
231-
if tc.isPermissionlessPoolCreationDisabled {
232-
params := s.App.ConcentratedLiquidityKeeper.GetParams(s.Ctx)
233-
params.IsPermissionlessPoolCreationEnabled = false
234-
s.App.ConcentratedLiquidityKeeper.SetParams(s.Ctx, params)
235-
}
236-
237-
if tc.expectedModuleType == cosmwasmKeeperType {
238-
codeId := s.StoreCosmWasmPoolContractCode(apptesting.TransmuterContractName)
239-
s.Require().Equal(validTransmuterCodeId, codeId)
240-
s.App.CosmwasmPoolKeeper.WhitelistCodeId(s.Ctx, codeId)
241-
}
242-
243-
poolmanagerKeeper := s.App.PoolManagerKeeper
244-
ctx := s.Ctx
245-
246-
poolCreationFee := poolmanagerKeeper.GetParams(s.Ctx).PoolCreationFee
247-
s.FundAcc(s.TestAccs[0], append(tc.creatorFundAmount, poolCreationFee...))
248-
249-
poolId, err := poolmanagerKeeper.CreatePool(ctx, tc.msg)
250-
251-
if tc.expectError {
252-
s.Require().Error(err)
253-
return
254-
}
255-
256-
// Validate pool.
257-
s.Require().NoError(err)
258-
s.Require().Equal(uint64(i+1), poolId)
259-
260-
// Validate that mapping pool id -> module type has been persisted.
261-
swapModule, err := poolmanagerKeeper.GetPoolModule(ctx, poolId)
262-
s.Require().NoError(err)
263-
s.Require().Equal(tc.expectedModuleType, reflect.TypeOf(swapModule))
264-
})
229+
// setup cosmwasm pool
230+
codeId := s.StoreCosmWasmPoolContractCode(apptesting.TransmuterContractName)
231+
s.Require().Equal(validTransmuterCodeId, codeId)
232+
s.App.CosmwasmPoolKeeper.WhitelistCodeId(s.Ctx, codeId)
233+
234+
execModes := map[string]sdk.ExecMode{
235+
"check": sdk.ExecModeCheck,
236+
"finalize": sdk.ExecModeFinalize,
237+
}
238+
totalTestCount := 0
239+
for _, tc := range tests {
240+
for execModeName, execMode := range execModes {
241+
totalTestCount++
242+
s.Run(fmt.Sprintf("%s-%s", tc.name, execModeName), func() {
243+
s.Ctx = s.Ctx.WithExecMode(execMode)
244+
if tc.isPermissionlessPoolCreationDisabled {
245+
params := s.App.ConcentratedLiquidityKeeper.GetParams(s.Ctx)
246+
params.IsPermissionlessPoolCreationEnabled = false
247+
s.App.ConcentratedLiquidityKeeper.SetParams(s.Ctx, params)
248+
}
249+
250+
poolmanagerKeeper := s.App.PoolManagerKeeper
251+
ctx := s.Ctx
252+
253+
poolCreationFee := poolmanagerKeeper.GetParams(s.Ctx).PoolCreationFee
254+
s.FundAcc(s.TestAccs[0], append(tc.creatorFundAmount, poolCreationFee...))
255+
256+
poolId, err := poolmanagerKeeper.CreatePool(ctx, tc.msg)
257+
258+
if tc.expectError {
259+
s.Require().Error(err)
260+
return
261+
}
262+
263+
// Validate pool.
264+
s.Require().NoError(err)
265+
s.Require().Equal(uint64(totalTestCount), poolId)
266+
267+
// Validate that mapping pool id -> module type has been persisted.
268+
swapModule, err := poolmanagerKeeper.GetPoolModule(ctx, poolId)
269+
s.Require().NoError(err)
270+
s.Require().Equal(tc.expectedModuleType, reflect.TypeOf(swapModule))
271+
})
272+
}
265273
}
266274
}
267275

0 commit comments

Comments
 (0)