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

feat(x/protorev): Use Transient store to store swap backruns #7665

Merged
merged 6 commits into from
Mar 7, 2024
Merged

feat(x/protorev): Use Transient store to store swap backruns #7665

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c29573f
Replace to use transient store
mattverse Mar 5, 2024
fdf53a9
Bring back cache ctx
mattverse Mar 6, 2024
1e4e42b
Fix lint
mattverse Mar 6, 2024
4690908
Add back delete swaps to backrun after each tx
mattverse Mar 7, 2024
4aec2f8
Add changelog
mattverse Mar 7, 2024
90c3156
Merge branch 'main' into mattverse/transient
mattverse Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
"[go.mod]": {
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": true
"source.organizeImports": "explicit"
Copy link
Member Author

Choose a reason for hiding this comment

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

drive by change, true has been deprecated and has been replaced to explicit

}
},
"[go]": {
"editor.snippetSuggestions": "none",
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": true
"source.organizeImports": "explicit"
}
},
"gopls": {
Expand Down
1 change: 1 addition & 0 deletions app/keepers/keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion app/keepers/keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions x/protorev/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -49,6 +51,7 @@ func NewKeeper(
return Keeper{
cdc: cdc,
storeKey: storeKey,
transientKey: transientKey,
paramstore: ps,
accountKeeper: accountKeeper,
bankKeeper: bankKeeper,
Expand Down
13 changes: 7 additions & 6 deletions x/protorev/keeper/posthandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ 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.
Expand All @@ -47,12 +48,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(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)
mattverse marked this conversation as resolved.
Show resolved Hide resolved
if len(swappedPools) == 0 {
return next(ctx, tx, success, simulate)
}
Expand All @@ -64,10 +65,10 @@ 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.
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))))
Copy link
Member

Choose a reason for hiding this comment

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

Don't we still need this? Just on the transient store?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we? I thought the purpose of Transient stores was that it drops all states anyways so thought we wouldn't need to delete them 🤔

Copy link
Member

Choose a reason for hiding this comment

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

It does drop all state, but at the end of the block not the end of each tx

Copy link
Member

Choose a reason for hiding this comment

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

(notably these need to get deleted end of each tx, in order to not cause excess backruns next block)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh gotcha, confused myself with this logic being in endblocker. Fixed in 4690908 this commit!


return next(ctx, tx, success, simulate)
}
Expand Down
6 changes: 3 additions & 3 deletions x/protorev/keeper/posthandler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
7 changes: 3 additions & 4 deletions x/protorev/keeper/protorev.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{}
Expand All @@ -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)
}

Expand Down
2 changes: 2 additions & 0 deletions x/protorev/types/keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
2 changes: 1 addition & 1 deletion x/twap/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down