From c29573fec5e27a2c9793339e739f5d885cb21d11 Mon Sep 17 00:00:00 2001 From: mattverse Date: Tue, 5 Mar 2024 16:19:49 +0900 Subject: [PATCH 1/5] Replace to use transient store --- app/keepers/keepers.go | 1 + app/keepers/keys.go | 3 ++- x/protorev/keeper/keeper.go | 9 ++++++--- x/protorev/keeper/posthandler.go | 19 +++++++------------ x/protorev/keeper/protorev.go | 7 +++---- x/protorev/types/keys.go | 2 ++ x/twap/store.go | 2 +- 7 files changed, 22 insertions(+), 21 deletions(-) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 1785674e804..4f0b6f26cfb 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -384,6 +384,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers( protorevKeeper := protorevkeeper.NewKeeper( appCodec, appKeepers.keys[protorevtypes.StoreKey], + appKeepers.tkeys[protorevtypes.TransientStoreKey], appKeepers.GetSubspace(protorevtypes.ModuleName), appKeepers.AccountKeeper, appKeepers.BankKeeper, diff --git a/app/keepers/keys.go b/app/keepers/keys.go index aa8879205c1..b3387a8a7f5 100644 --- a/app/keepers/keys.go +++ b/app/keepers/keys.go @@ -7,6 +7,7 @@ import ( storetypes "github.com/cosmos/cosmos-sdk/store/types" + protorevtypes "github.com/osmosis-labs/osmosis/v23/x/protorev/types" twaptypes "github.com/osmosis-labs/osmosis/v23/x/twap/types" ) @@ -17,7 +18,7 @@ func (appKeepers *AppKeepers) GenerateKeys() { appKeepers.keys = sdk.NewKVStoreKeys(KVStoreKeys()...) // Define transient store keys - appKeepers.tkeys = sdk.NewTransientStoreKeys(paramstypes.TStoreKey, twaptypes.TransientStoreKey) + appKeepers.tkeys = sdk.NewTransientStoreKeys(paramstypes.TStoreKey, twaptypes.TransientStoreKey, protorevtypes.TransientStoreKey) // MemKeys are for information that is stored only in RAM. appKeepers.memKeys = sdk.NewMemoryStoreKeys(capabilitytypes.MemStoreKey) diff --git a/x/protorev/keeper/keeper.go b/x/protorev/keeper/keeper.go index 7c2fb8bf64a..5e78a79ce6e 100644 --- a/x/protorev/keeper/keeper.go +++ b/x/protorev/keeper/keeper.go @@ -15,9 +15,10 @@ import ( type ( Keeper struct { - cdc codec.BinaryCodec - storeKey storetypes.StoreKey - paramstore paramtypes.Subspace + cdc codec.BinaryCodec + storeKey storetypes.StoreKey + transientKey *storetypes.TransientStoreKey + paramstore paramtypes.Subspace accountKeeper types.AccountKeeper bankKeeper types.BankKeeper @@ -32,6 +33,7 @@ type ( func NewKeeper( cdc codec.BinaryCodec, storeKey storetypes.StoreKey, + transientKey *storetypes.TransientStoreKey, ps paramtypes.Subspace, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, @@ -49,6 +51,7 @@ func NewKeeper( return Keeper{ cdc: cdc, storeKey: storeKey, + transientKey: transientKey, paramstore: ps, accountKeeper: accountKeeper, bankKeeper: bankKeeper, diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 42acef2a727..9bd16b16870 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -36,38 +36,33 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu // Create a cache context to execute the posthandler such that // 1. If there is an error, then the cache context is discarded // 2. If there is no error, then the cache context is written to the main context with no gas consumed - cacheCtx, write := ctx.CacheContext() // CacheCtx's by default _share_ their gas meter with the parent. // In our case, the cache ctx is given a new gas meter instance entirely, // so gas usage is not counted towards tx gas usage. // // 50M is chosen as a large enough number to ensure that the posthandler will not run out of gas, // but will eventually terminate in event of an accidental infinite loop with some gas usage. - upperGasLimitMeter := sdk.NewGasMeter(sdk.Gas(50_000_000)) - cacheCtx = cacheCtx.WithGasMeter(upperGasLimitMeter) // Check if the protorev posthandler can be executed - if err := protoRevDec.ProtoRevKeeper.AnteHandleCheck(cacheCtx); err != nil { + if err := protoRevDec.ProtoRevKeeper.AnteHandleCheck(ctx); err != nil { return next(ctx, tx, success, simulate) } // Extract all of the pools that were swapped in the tx - swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(cacheCtx) + swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(ctx) if len(swappedPools) == 0 { return next(ctx, tx, success, simulate) } // Attempt to execute arbitrage trades - if err := protoRevDec.ProtoRevKeeper.ProtoRevTrade(cacheCtx, swappedPools); err == nil { - write() - } else { + if err := protoRevDec.ProtoRevKeeper.ProtoRevTrade(ctx, swappedPools); err != nil { ctx.Logger().Error("ProtoRevTrade failed with error: " + err.Error()) } - // Delete swaps to backrun for next transaction without consuming gas - // from the current transaction's gas meter, but instead from a new gas meter with 50mil gas. - // 50 mil gas was chosen as an arbitrary large number to ensure deletion does not run out of gas. - protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(50_000_000)))) + // // Delete swaps to backrun for next transaction without consuming gas + // // from the current transaction's gas meter, but instead from a new gas meter with 50mil gas. + // // 50 mil gas was chosen as an arbitrary large number to ensure deletion does not run out of gas. + // protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(50_000_000)))) return next(ctx, tx, success, simulate) } diff --git a/x/protorev/keeper/protorev.go b/x/protorev/keeper/protorev.go index 1412cd2d79a..41822b0ed23 100644 --- a/x/protorev/keeper/protorev.go +++ b/x/protorev/keeper/protorev.go @@ -199,8 +199,7 @@ func (k Keeper) DeleteAllPoolsForBaseDenom(ctx sdk.Context, baseDenom string) { // SetSwapsToBackrun sets the swaps to backrun, updated via hooks func (k Keeper) SetSwapsToBackrun(ctx sdk.Context, swapsToBackrun types.Route) error { - store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) - + store := prefix.NewStore(ctx.TransientStore(k.transientKey), types.KeyPrefixSwapsToBackrun) bz, err := swapsToBackrun.Marshal() if err != nil { return err @@ -213,7 +212,7 @@ func (k Keeper) SetSwapsToBackrun(ctx sdk.Context, swapsToBackrun types.Route) e // GetSwapsToBackrun returns the swaps to backrun, updated via hooks func (k Keeper) GetSwapsToBackrun(ctx sdk.Context) (types.Route, error) { - store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) + store := prefix.NewStore(ctx.TransientStore(k.transientKey), types.KeyPrefixSwapsToBackrun) bz := store.Get(types.KeyPrefixSwapsToBackrun) swapsToBackrun := types.Route{} @@ -227,7 +226,7 @@ func (k Keeper) GetSwapsToBackrun(ctx sdk.Context) (types.Route, error) { // DeleteSwapsToBackrun deletes the swaps to backrun func (k Keeper) DeleteSwapsToBackrun(ctx sdk.Context) { - store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) + store := prefix.NewStore(ctx.TransientStore(k.transientKey), types.KeyPrefixSwapsToBackrun) store.Delete(types.KeyPrefixSwapsToBackrun) } diff --git a/x/protorev/types/keys.go b/x/protorev/types/keys.go index 4630d20ed8e..8663f36a7cc 100644 --- a/x/protorev/types/keys.go +++ b/x/protorev/types/keys.go @@ -15,6 +15,8 @@ const ( // StoreKey defines the primary module store key StoreKey = ModuleName + TransientStoreKey = "transient_" + ModuleName + // RouterKey defines the module's message routing key RouterKey = ModuleName ) diff --git a/x/twap/store.go b/x/twap/store.go index c07bb9e9786..5f57d5b3099 100644 --- a/x/twap/store.go +++ b/x/twap/store.go @@ -31,7 +31,7 @@ func (e timeTooOldError) Error() string { // just has to not be empty, for store to work / not register as a delete. var sentinelExistsValue = []byte{1} -// trackChangedPool places an entry into a transient store, +// trackChangedPool places an entry into a transient store,x // to track that this pool changed this block. // This tracking is for use in EndBlock, to create new TWAP records. func (k Keeper) trackChangedPool(ctx sdk.Context, poolId uint64) { From fdf53a9a8cd828ad52ecec4d6bd509bcf9c20327 Mon Sep 17 00:00:00 2001 From: mattverse Date: Wed, 6 Mar 2024 10:34:34 +0900 Subject: [PATCH 2/5] Bring back cache ctx --- .vscode/settings.json | 4 ++-- x/protorev/keeper/posthandler.go | 8 +++++++- x/protorev/keeper/posthandler_test.go | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 7a98d33e4fa..3e0b7dcda9b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -11,14 +11,14 @@ "[go.mod]": { "editor.formatOnSave": true, "editor.codeActionsOnSave": { - "source.organizeImports": true + "source.organizeImports": "explicit" } }, "[go]": { "editor.snippetSuggestions": "none", "editor.formatOnSave": true, "editor.codeActionsOnSave": { - "source.organizeImports": true + "source.organizeImports": "explicit" } }, "gopls": { diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 9bd16b16870..1e6b4c55c9d 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -36,12 +36,16 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu // Create a cache context to execute the posthandler such that // 1. If there is an error, then the cache context is discarded // 2. If there is no error, then the cache context is written to the main context with no gas consumed + cacheCtx, write := ctx.CacheContext() + // CacheCtx's by default _share_ their gas meter with the parent. // In our case, the cache ctx is given a new gas meter instance entirely, // so gas usage is not counted towards tx gas usage. // // 50M is chosen as a large enough number to ensure that the posthandler will not run out of gas, // but will eventually terminate in event of an accidental infinite loop with some gas usage. + upperGasLimitMeter := sdk.NewGasMeter(sdk.Gas(50_000_000)) + cacheCtx = cacheCtx.WithGasMeter(upperGasLimitMeter) // Check if the protorev posthandler can be executed if err := protoRevDec.ProtoRevKeeper.AnteHandleCheck(ctx); err != nil { @@ -55,7 +59,9 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu } // Attempt to execute arbitrage trades - if err := protoRevDec.ProtoRevKeeper.ProtoRevTrade(ctx, swappedPools); err != nil { + if err := protoRevDec.ProtoRevKeeper.ProtoRevTrade(cacheCtx, swappedPools); err == nil { + write() + } else { ctx.Logger().Error("ProtoRevTrade failed with error: " + err.Error()) } diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 5b235c1e095..01ff15432f4 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -429,18 +429,18 @@ func (s *KeeperTestSuite) TestPostHandle() { // Set pools to backrun s.App.AppKeepers.ProtoRevKeeper.AddSwapsToSwapsToBackrun(s.Ctx, tc.params.trades) - gasBefore := s.Ctx.GasMeter().GasConsumed() + // gasBefore := s.Ctx.GasMeter().GasConsumed() gasLimitBefore := s.Ctx.GasMeter().Limit() _, err = posthandlerProtoRev(s.Ctx, tx, false, true) - gasAfter := s.Ctx.GasMeter().GasConsumed() + // gasAfter := s.Ctx.GasMeter().GasConsumed() gasLimitAfter := s.Ctx.GasMeter().Limit() if tc.expectPass { s.Require().NoError(err) // Check that the gas consumed is the same before and after the posthandler - s.Require().Equal(gasBefore, gasAfter) + // s.Require().Equal(gasBefore, gasAfter) // Check that the gas limit is the same before and after the posthandler s.Require().Equal(gasLimitBefore, gasLimitAfter) From 1e4e42bddadab29c09864aeeac409ae1f5938d89 Mon Sep 17 00:00:00 2001 From: mattverse Date: Wed, 6 Mar 2024 11:05:56 +0900 Subject: [PATCH 3/5] Fix lint --- x/protorev/keeper/posthandler.go | 11 +++++------ x/protorev/keeper/posthandler_test.go | 6 +++--- x/twap/store.go | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 1e6b4c55c9d..cc694571040 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -37,7 +37,6 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu // 1. If there is an error, then the cache context is discarded // 2. If there is no error, then the cache context is written to the main context with no gas consumed cacheCtx, write := ctx.CacheContext() - // CacheCtx's by default _share_ their gas meter with the parent. // In our case, the cache ctx is given a new gas meter instance entirely, // so gas usage is not counted towards tx gas usage. @@ -48,12 +47,12 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu cacheCtx = cacheCtx.WithGasMeter(upperGasLimitMeter) // Check if the protorev posthandler can be executed - if err := protoRevDec.ProtoRevKeeper.AnteHandleCheck(ctx); err != nil { + if err := protoRevDec.ProtoRevKeeper.AnteHandleCheck(cacheCtx); err != nil { return next(ctx, tx, success, simulate) } // Extract all of the pools that were swapped in the tx - swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(ctx) + swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(cacheCtx) if len(swappedPools) == 0 { return next(ctx, tx, success, simulate) } @@ -65,9 +64,9 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu ctx.Logger().Error("ProtoRevTrade failed with error: " + err.Error()) } - // // Delete swaps to backrun for next transaction without consuming gas - // // from the current transaction's gas meter, but instead from a new gas meter with 50mil gas. - // // 50 mil gas was chosen as an arbitrary large number to ensure deletion does not run out of gas. + // Delete swaps to backrun for next transaction without consuming gas + // from the current transaction's gas meter, but instead from a new gas meter with 50mil gas. + // 50 mil gas was chosen as an arbitrary large number to ensure deletion does not run out of gas. // protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(50_000_000)))) return next(ctx, tx, success, simulate) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 01ff15432f4..5b235c1e095 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -429,18 +429,18 @@ func (s *KeeperTestSuite) TestPostHandle() { // Set pools to backrun s.App.AppKeepers.ProtoRevKeeper.AddSwapsToSwapsToBackrun(s.Ctx, tc.params.trades) - // gasBefore := s.Ctx.GasMeter().GasConsumed() + gasBefore := s.Ctx.GasMeter().GasConsumed() gasLimitBefore := s.Ctx.GasMeter().Limit() _, err = posthandlerProtoRev(s.Ctx, tx, false, true) - // gasAfter := s.Ctx.GasMeter().GasConsumed() + gasAfter := s.Ctx.GasMeter().GasConsumed() gasLimitAfter := s.Ctx.GasMeter().Limit() if tc.expectPass { s.Require().NoError(err) // Check that the gas consumed is the same before and after the posthandler - // s.Require().Equal(gasBefore, gasAfter) + s.Require().Equal(gasBefore, gasAfter) // Check that the gas limit is the same before and after the posthandler s.Require().Equal(gasLimitBefore, gasLimitAfter) diff --git a/x/twap/store.go b/x/twap/store.go index 5f57d5b3099..c07bb9e9786 100644 --- a/x/twap/store.go +++ b/x/twap/store.go @@ -31,7 +31,7 @@ func (e timeTooOldError) Error() string { // just has to not be empty, for store to work / not register as a delete. var sentinelExistsValue = []byte{1} -// trackChangedPool places an entry into a transient store,x +// trackChangedPool places an entry into a transient store, // to track that this pool changed this block. // This tracking is for use in EndBlock, to create new TWAP records. func (k Keeper) trackChangedPool(ctx sdk.Context, poolId uint64) { From 46909089cf1f5ef16ad47823f272542b1eda7866 Mon Sep 17 00:00:00 2001 From: mattverse Date: Thu, 7 Mar 2024 15:29:32 +0900 Subject: [PATCH 4/5] Add back delete swaps to backrun after each tx --- x/protorev/keeper/posthandler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index cc694571040..42acef2a727 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -67,7 +67,7 @@ func (protoRevDec ProtoRevDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simu // Delete swaps to backrun for next transaction without consuming gas // from the current transaction's gas meter, but instead from a new gas meter with 50mil gas. // 50 mil gas was chosen as an arbitrary large number to ensure deletion does not run out of gas. - // protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(50_000_000)))) + protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(50_000_000)))) return next(ctx, tx, success, simulate) } From 4aec2f8fc01004dedf73cec1d316369946b73b64 Mon Sep 17 00:00:00 2001 From: mattverse Date: Thu, 7 Mar 2024 15:40:43 +0900 Subject: [PATCH 5/5] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 849bd1809c8..55d3f20a5c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#7619](https://github.com/osmosis-labs/osmosis/pull/7619) Slight speedup/gas improvement to CL GetTotalPoolLiquidity queries * [#7623](https://github.com/osmosis-labs/osmosis/pull/7623) Add query for querying all before send hooks * [#7622](https://github.com/osmosis-labs/osmosis/pull/7622) Remove duplicate CL accumulator update logic. +* [#7665](https://github.com/osmosis-labs/osmosis/pull/7665) feat(x/protorev): Use Transient store to store swap backruns. ### State Compatible