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.

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

Protorev: Update timing for pool updating and dev fee payout #4827

Merged
merged 40 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
df18e54
Update highest liquidity pools in daily epoch instead of weekly
NotJeremyLiu Apr 3, 2023
5bacd9e
Minimal code change to send dev profit after every trade
NotJeremyLiu Apr 3, 2023
1f2f3dc
Handles dev fee payment without storing in kvstore
NotJeremyLiu Apr 5, 2023
955b2aa
Merge branch 'main' into jl/protorev-update-timing
NotJeremyLiu Apr 24, 2023
9189461
Increase consensus version
NotJeremyLiu Apr 24, 2023
efbd26e
Revert "Handles dev fee payment without storing in kvstore"
NotJeremyLiu Apr 24, 2023
744d22e
add upgrade logic, new functions, and deprecated comments
NotJeremyLiu Apr 24, 2023
49c7eaf
add another todo
NotJeremyLiu Apr 24, 2023
104720a
Revert "add another todo"
NotJeremyLiu Apr 24, 2023
c8f8ff3
add another todo
NotJeremyLiu Apr 24, 2023
865161c
Panic on migration error
NotJeremyLiu Apr 24, 2023
3b490ee
Hardcode protorev from version to 1
NotJeremyLiu Apr 25, 2023
e7b0047
Add tests to ensure protorev upgrade is successful
NotJeremyLiu Apr 25, 2023
2653b70
Remove deprecated comment to pass linter
NotJeremyLiu Apr 25, 2023
f457bed
lint
NotJeremyLiu Apr 25, 2023
2474ad6
add changelog entry
NotJeremyLiu Apr 25, 2023
17a85bf
Merge branch 'main' into jl/protorev-update-timing
NotJeremyLiu Apr 25, 2023
5ebb8b6
Change documentation to reflect new timing cadence
NotJeremyLiu Apr 25, 2023
212ec42
Change developer_fees test for new SendDeveloperFee method
NotJeremyLiu Apr 26, 2023
666e53b
Update app/upgrades/v16/upgrades.go
NotJeremyLiu May 2, 2023
de3c073
move test to top of file, helper at bottom
NotJeremyLiu May 2, 2023
ad69aa4
Merge branch 'main' into jl/protorev-update-timing
NotJeremyLiu May 18, 2023
179fbcb
fix merge conflict typo
NotJeremyLiu May 18, 2023
97a6ea8
added migration function to upgrades.go
stackman27 May 23, 2023
127dc5b
bump consensus version to 2
NotJeremyLiu May 24, 2023
da4efca
comment out everything upgrade related to isolate problem
NotJeremyLiu May 24, 2023
314d1c4
Revert "comment out everything upgrade related to isolate problem"
NotJeremyLiu May 24, 2023
43b0abb
Revert "bump consensus version to 2"
NotJeremyLiu May 24, 2023
e68be34
log instead of pass back up errors
NotJeremyLiu May 25, 2023
f0a88c9
Remove commented out code
NotJeremyLiu May 25, 2023
b069fa1
clean up comments
NotJeremyLiu May 25, 2023
6723370
add clarifying comment
NotJeremyLiu May 25, 2023
af008b7
Merge branch 'main' into jl/protorev-update-timing
NotJeremyLiu May 25, 2023
61b7c83
fix typpo
NotJeremyLiu May 25, 2023
3be7cbc
Merge branch 'main' into jl/protorev-update-timing
NotJeremyLiu May 25, 2023
7f8f1e9
update v15 prev version tag in e2e container
NotJeremyLiu May 25, 2023
0d63186
remove e2e test that checks protorev dev account not initialized
NotJeremyLiu May 25, 2023
4957aff
return err if protorev dev account payment errors during upgrade
NotJeremyLiu May 25, 2023
783e448
Merge branch 'main' into jl/protorev-update-timing
NotJeremyLiu May 26, 2023
fced67d
Add dev fee payment check when testing trade execution
NotJeremyLiu May 26, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#4830](https://github.com/osmosis-labs/osmosis/pull/4830) Add gas cost when we AddToGaugeRewards, linearly increase with coins to add
* [#4886](https://github.com/osmosis-labs/osmosis/pull/4886) Implement MsgSplitRouteSwapExactAmountIn and MsgSplitRouteSwapExactAmountOut that supports route splitting.
* [#5000](https://github.com/osmosis-labs/osmosis/pull/5000) osmomath.Power panics for base < 1 to temporarily restrict broken logic for such base.
* [#4827] (https://github.com/osmosis-labs/osmosis/pull/4827) Protorev: Change highest liquidity pool updating from weekly to daily and change dev fee payout from weekly to after every trade.

### Misc Improvements

Expand Down
5 changes: 5 additions & 0 deletions app/upgrades/v16/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ func CreateUpgradeHandler(

updateTokenFactoryParams(ctx, keepers.TokenFactoryKeeper)

// Transfers out all the dev fees in kvstore to dev account during upgrade
if err := keepers.ProtoRevKeeper.SendDeveloperFeesToDeveloperAccount(ctx); err != nil {
return nil, err
}

return migrations, nil
}
}
Expand Down
38 changes: 38 additions & 0 deletions app/upgrades/v16/upgrades_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
v16 "github.com/osmosis-labs/osmosis/v15/app/upgrades/v16"
cltypes "github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/types"
poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types"
protorevtypes "github.com/osmosis-labs/osmosis/v15/x/protorev/types"
)

type UpgradeTestSuite struct {
Expand Down Expand Up @@ -50,6 +51,10 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
store := suite.Ctx.KVStore(upgradeStoreKey)
versionStore := prefix.NewStore(store, []byte{upgradetypes.VersionMapByte})
versionStore.Delete([]byte(cltypes.ModuleName))

// Ensure proper setup for ProtoRev upgrade testing
err := upgradeProtorevSetup(suite)
suite.Require().NoError(err)
}

testCases := []struct {
Expand Down Expand Up @@ -115,6 +120,9 @@ func (suite *UpgradeTestSuite) TestUpgrade() {

// Permissionless pool creation is disabled.
suite.Require().False(params.IsPermissionlessPoolCreationEnabled)

// Ensure that the protorev upgrade was successful
verifyProtorevUpdateSuccess(suite)
},
func() {
// Validate that tokenfactory params have been updated
Expand Down Expand Up @@ -149,3 +157,33 @@ func (suite *UpgradeTestSuite) TestUpgrade() {
})
}
}

func upgradeProtorevSetup(suite *UpgradeTestSuite) error {
account := apptesting.CreateRandomAccounts(1)[0]
suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)

devFee := sdk.NewCoin("uosmo", sdk.NewInt(1000000))
if err := suite.App.ProtoRevKeeper.SetDeveloperFees(suite.Ctx, devFee); err != nil {
return err
}

fundCoin := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(1000000)))

if err := suite.App.AppKeepers.BankKeeper.MintCoins(suite.Ctx, protorevtypes.ModuleName, fundCoin); err != nil {
return err
}

return nil
}

func verifyProtorevUpdateSuccess(suite *UpgradeTestSuite) {
// Ensure balance was transferred to the developer account
devAcc, err := suite.App.ProtoRevKeeper.GetDeveloperAccount(suite.Ctx)
suite.Require().NoError(err)
suite.Require().Equal(suite.App.BankKeeper.GetBalance(suite.Ctx, devAcc, "uosmo"), sdk.NewCoin("uosmo", sdk.NewInt(1000000)))

// Ensure developer fees are empty
coins, err := suite.App.ProtoRevKeeper.GetAllDeveloperFees(suite.Ctx)
suite.Require().NoError(err)
suite.Require().Equal(coins, []sdk.Coin{})
}
36 changes: 36 additions & 0 deletions x/protorev/keeper/developer_fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/osmosis-labs/osmosis/v15/x/protorev/types"
)

// Used in v16 upgrade, can be removed in v17
// SendDeveloperFeesToDeveloperAccount sends the developer fees from the module account to the developer account
func (k Keeper) SendDeveloperFeesToDeveloperAccount(ctx sdk.Context) error {
// Developer account must be set in order to be able to withdraw developer fees
Expand All @@ -32,6 +33,7 @@ func (k Keeper) SendDeveloperFeesToDeveloperAccount(ctx sdk.Context) error {
return nil
}

// Deprecated: Can be removed in v16
// UpdateDeveloperFees updates the fees that developers can withdraw from the module account
func (k Keeper) UpdateDeveloperFees(ctx sdk.Context, denom string, profit sdk.Int) error {
Comment on lines +36 to 38
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove now?

Copy link
Member

Choose a reason for hiding this comment

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

This is unused now. Please submit separate PR if agreed. Not going to block merge on this

daysSinceGenesis, err := k.GetDaysSinceModuleGenesis(ctx)
Expand Down Expand Up @@ -62,3 +64,37 @@ func (k Keeper) UpdateDeveloperFees(ctx sdk.Context, denom string, profit sdk.In

return nil
}

// SendDeveloperFee sends the developer fee from the module account to the developer account
func (k Keeper) SendDeveloperFee(ctx sdk.Context, arbProfit sdk.Coin) error {
// Developer account must be set in order to be able to withdraw developer fees
developerAccount, err := k.GetDeveloperAccount(ctx)
if err != nil {
return err
}

// Get the days since genesis
daysSinceGenesis, err := k.GetDaysSinceModuleGenesis(ctx)
if err != nil {
return err
}

// Initialize the developer profit to 0
devProfit := sdk.NewCoin(arbProfit.Denom, sdk.ZeroInt())

// Calculate the developer fee
if daysSinceGenesis < types.Phase1Length {
devProfit.Amount = arbProfit.Amount.MulRaw(types.ProfitSplitPhase1).QuoRaw(100)
} else if daysSinceGenesis < types.Phase2Length {
devProfit.Amount = arbProfit.Amount.MulRaw(types.ProfitSplitPhase2).QuoRaw(100)
} else {
devProfit.Amount = arbProfit.Amount.MulRaw(types.ProfitSplitPhase3).QuoRaw(100)
}

// Send the developer profit to the developer account
if err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, developerAccount, sdk.NewCoins(devProfit)); err != nil {
return err
}

return nil
}
162 changes: 42 additions & 120 deletions x/protorev/keeper/developer_fees_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,168 +7,90 @@ import (
"github.com/osmosis-labs/osmosis/v15/x/protorev/types"
)

// TestSendDeveloperFeesToDeveloperAccount tests the SendDeveloperFeesToDeveloperAccount function
func (s *KeeperTestSuite) TestSendDeveloperFeesToDeveloperAccount() {
// TestSendDeveloperFee tests the SendDeveloperFee function
func (suite *KeeperTestSuite) TestSendDeveloperFee() {
cases := []struct {
description string
alterState func()
expectedErr bool
expectedCoins sdk.Coins
description string
alterState func()
expectedErr bool
expectedDevProfit sdk.Coin
}{
{
description: "Send with unset developer account",
alterState: func() {},
expectedErr: true,
expectedCoins: sdk.NewCoins(),
description: "Send with unset developer account",
alterState: func() {},
expectedErr: true,
expectedDevProfit: sdk.Coin{},
},
{
description: "Send with set developer account",
description: "Send with set developer account in first phase",
alterState: func() {
account := apptesting.CreateRandomAccounts(1)[0]
s.App.ProtoRevKeeper.SetDeveloperAccount(s.Ctx, account)
suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)

err := s.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(2000), 0)
s.Require().NoError(err)
err := suite.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(1000), 100)
suite.Require().NoError(err)
},
expectedErr: false,
expectedCoins: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(400))),
expectedErr: false,
expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(20)),
},
{
description: "Send with set developer account (after multiple trades)",
description: "Send with set developer account in second phase",
alterState: func() {
account := apptesting.CreateRandomAccounts(1)[0]
s.App.ProtoRevKeeper.SetDeveloperAccount(s.Ctx, account)
suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)

// Trade 1
err := s.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(2000), 0)
s.Require().NoError(err)

// Trade 2
err = s.pseudoExecuteTrade("Atom", sdk.NewInt(2000), 0)
s.Require().NoError(err)
err := suite.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(1000), 500)
suite.Require().NoError(err)
},
expectedErr: false,
expectedCoins: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(400)), sdk.NewCoin("Atom", sdk.NewInt(400))),
expectedErr: false,
expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10)),
},
{
description: "Send with set developer account (after multiple trades across epochs)",
description: "Send with set developer account in third (final) phase",
alterState: func() {
account := apptesting.CreateRandomAccounts(1)[0]
s.App.ProtoRevKeeper.SetDeveloperAccount(s.Ctx, account)

// Trade 1
err := s.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(2000), 0)
s.Require().NoError(err)

// Trade 2
err = s.pseudoExecuteTrade("Atom", sdk.NewInt(2000), 0)
s.Require().NoError(err)

// Trade 3 after year 1
err = s.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(2000), 366)
s.Require().NoError(err)
suite.App.ProtoRevKeeper.SetDeveloperAccount(suite.Ctx, account)

// Trade 4 after year 2
err = s.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(2000), 366*2)
s.Require().NoError(err)
err := suite.pseudoExecuteTrade(types.OsmosisDenomination, sdk.NewInt(1000), 1000)
suite.Require().NoError(err)
},
expectedErr: false,
expectedCoins: sdk.NewCoins(sdk.NewCoin("Atom", sdk.NewInt(400)), sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(700))),
expectedErr: false,
expectedDevProfit: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(5)),
},
}

for _, tc := range cases {
s.Run(tc.description, func() {
s.SetupTest()
suite.Run(tc.description, func() {
suite.SetupTest()
tc.alterState()

err := s.App.ProtoRevKeeper.SendDeveloperFeesToDeveloperAccount(s.Ctx)
err := suite.App.ProtoRevKeeper.SendDeveloperFee(suite.Ctx, sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(100)))
if tc.expectedErr {
s.Require().Error(err)
suite.Require().Error(err)
} else {
s.Require().NoError(err)
suite.Require().NoError(err)
}

developerAccount, err := s.App.ProtoRevKeeper.GetDeveloperAccount(s.Ctx)
developerAccount, err := suite.App.ProtoRevKeeper.GetDeveloperAccount(suite.Ctx)
if !tc.expectedErr {
developerFees := s.App.AppKeepers.BankKeeper.GetAllBalances(s.Ctx, developerAccount)
s.Require().Equal(tc.expectedCoins, developerFees)
developerFee := suite.App.AppKeepers.BankKeeper.GetBalance(suite.Ctx, developerAccount, types.OsmosisDenomination)
suite.Require().Equal(tc.expectedDevProfit, developerFee)
} else {
s.Require().Error(err)
suite.Require().Error(err)
}
})
}
}

// TestUpdateDeveloperFees tests the UpdateDeveloperFees function
func (s *KeeperTestSuite) TestUpdateDeveloperFees() {
cases := []struct {
description string
denom string
profit sdk.Int
alterState func()
expected sdk.Coin
}{
{
description: "Update developer fees in year 1",
denom: types.OsmosisDenomination,
profit: sdk.NewInt(200),
alterState: func() {},
expected: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(40)),
},
{
description: "Update developer fees in year 2",
denom: types.OsmosisDenomination,
profit: sdk.NewInt(200),
alterState: func() {
s.App.ProtoRevKeeper.SetDaysSinceModuleGenesis(s.Ctx, 366)
},
expected: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(20)),
},
{
description: "Update developer fees after year 2",
denom: types.OsmosisDenomination,
profit: sdk.NewInt(200),
alterState: func() {
s.App.ProtoRevKeeper.SetDaysSinceModuleGenesis(s.Ctx, 731)
},
expected: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10)),
},
{
description: "Update developer fees after year 10",
denom: types.OsmosisDenomination,
profit: sdk.NewInt(200),
alterState: func() {
s.App.ProtoRevKeeper.SetDaysSinceModuleGenesis(s.Ctx, 365*10+1)
},
expected: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10)),
},
}

for _, tc := range cases {
s.Run(tc.description, func() {
s.SetupTest()
tc.alterState()

err := s.App.ProtoRevKeeper.UpdateDeveloperFees(s.Ctx, tc.denom, tc.profit)
s.Require().NoError(err)

developerFees, err := s.App.ProtoRevKeeper.GetDeveloperFees(s.Ctx, tc.denom)
s.Require().NoError(err)
s.Require().Equal(tc.expected, developerFees)
})
}
}

// pseudoExecuteTrade is a helper function to execute a trade given denom of profit, profit, and days since genesis
func (s *KeeperTestSuite) pseudoExecuteTrade(denom string, profit sdk.Int, daysSinceGenesis uint64) error {
func (suite *KeeperTestSuite) pseudoExecuteTrade(denom string, profit sdk.Int, daysSinceGenesis uint64) error {
// Initialize the number of days since genesis
s.App.ProtoRevKeeper.SetDaysSinceModuleGenesis(s.Ctx, daysSinceGenesis)
suite.App.ProtoRevKeeper.SetDaysSinceModuleGenesis(suite.Ctx, daysSinceGenesis)
// Mint the profit to the module account (which will be sent to the developer account later)
err := s.App.AppKeepers.BankKeeper.MintCoins(s.Ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin(denom, profit)))
err := suite.App.AppKeepers.BankKeeper.MintCoins(suite.Ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin(denom, profit)))
if err != nil {
return err
}
// Update the developer fees
return s.App.ProtoRevKeeper.UpdateDeveloperFees(s.Ctx, denom, profit)

return nil
}
10 changes: 3 additions & 7 deletions x/protorev/keeper/epoch_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,16 @@ func (h EpochHooks) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, ep
func (h EpochHooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) error {
if h.k.GetProtoRevEnabled(ctx) {
switch epochIdentifier {
case "week":
// Distribute developer fees to the developer account. We do not error check because the developer account
// may not have been set by this point (gets set by the admin account after module genesis)
_ = h.k.SendDeveloperFeesToDeveloperAccount(ctx)

// Update the pools in the store
return h.k.UpdatePools(ctx)
case "day":
// Increment number of days since module genesis to properly calculate developer fees after cyclic arbitrage trades
if daysSinceGenesis, err := h.k.GetDaysSinceModuleGenesis(ctx); err != nil {
h.k.SetDaysSinceModuleGenesis(ctx, 1)
} else {
h.k.SetDaysSinceModuleGenesis(ctx, daysSinceGenesis+1)
}

// Update the pools in the store
return h.k.UpdatePools(ctx)
}
}

Expand Down
5 changes: 5 additions & 0 deletions x/protorev/keeper/posthandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"

"github.com/osmosis-labs/osmosis/v15/app/apptesting"
gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types"
poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types"
"github.com/osmosis-labs/osmosis/v15/x/protorev/keeper"
Expand Down Expand Up @@ -90,6 +91,10 @@ func (s *KeeperTestSuite) TestAnteHandle() {
acc1 := s.App.AccountKeeper.NewAccountWithAddress(s.Ctx, addr0)
s.App.AccountKeeper.SetAccount(s.Ctx, acc1)

// Set protorev developer account
devAccount := apptesting.CreateRandomAccounts(1)[0]
s.App.ProtoRevKeeper.SetDeveloperAccount(s.Ctx, devAccount)

// Keep testing order consistent to make adding tests easier
// Add all tests that are not expected to execute a trade first
// Then track number of trades and profits for the rest of the tests
Expand Down
Loading