Skip to content

Commit

Permalink
refactor!: change the meaning of the default empty securing pools (#31)
Browse files Browse the repository at this point in the history
## Description

This PR changes the meaning of "empty securing pools" that newly created
services have by default, from "secured by all pools" to "secured by no
pools".

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is
not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR
Targeting](https://github.com/milkyway-labs/milkyway/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building
modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration
[tests](https://github.com/milkyway-labs/milkyway/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go
code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable
and please add
your handle next to the items reviewed if you only reviewed selected
items.*

I have...

- [ ] confirmed the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit 623fa32)
  • Loading branch information
hallazzang authored and RiccardoM committed Jan 15, 2025
1 parent 1fce5da commit 1d2e6de
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 25 deletions.
2 changes: 2 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import (
milkywayabci "github.com/milkyway-labs/milkyway/v7/app/abci"
"github.com/milkyway-labs/milkyway/v7/app/forks"
v7 "github.com/milkyway-labs/milkyway/v7/app/forks/v7"
v8 "github.com/milkyway-labs/milkyway/v7/app/forks/v8"
"github.com/milkyway-labs/milkyway/v7/app/keepers"
"github.com/milkyway-labs/milkyway/v7/app/upgrades"
v6 "github.com/milkyway-labs/milkyway/v7/app/upgrades/v6"
Expand All @@ -81,6 +82,7 @@ var (

Forks = []forks.Fork{
v7.Fork,
v8.Fork,
}

Upgrades = []upgrades.Upgrade{
Expand Down
10 changes: 10 additions & 0 deletions app/forks/v8/constants.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package v8

import (
"github.com/milkyway-labs/milkyway/v7/app/forks"
)

var Fork = forks.Fork{
ForkHeight: 600000, // TODO: update height
BeginForkLogic: BeginFork,
}
58 changes: 58 additions & 0 deletions app/forks/v8/fork.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package v8

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/milkyway-labs/milkyway/v7/app/keepers"
"github.com/milkyway-labs/milkyway/v7/utils"
poolstypes "github.com/milkyway-labs/milkyway/v7/x/pools/types"
servicestypes "github.com/milkyway-labs/milkyway/v7/x/services/types"
)

func BeginFork(ctx sdk.Context, keepers *keepers.AppKeepers) {
ctx.Logger().Info(`
===================================================================================================
==== Forking chain state
===================================================================================================
`)

// Get all pools IDs first.
pools, err := keepers.PoolsKeeper.GetPools(ctx)
if err != nil {
panic(err)
}
poolIDs := utils.Map(pools, func(pool poolstypes.Pool) uint32 {
return pool.ID
})

// Update existing services that have no securing pools configured(which means
// they are secured by all pools by default) to have all the pools as their
// securing pools.
var serviceIDsToUpdate []uint32
err = keepers.ServicesKeeper.IterateServices(ctx, func(service servicestypes.Service) (stop bool, err error) {
// We only need to update services that have not been configured to secure pools
// since we changed the semantic of "empty securing pools"
configured, err := keepers.RestakingKeeper.IsServiceSecuringPoolsConfigured(ctx, service.ID)
if err != nil {
return true, err
}
if configured {
return false, nil
}

serviceIDsToUpdate = append(serviceIDsToUpdate, service.ID)
return false, nil
})
if err != nil {
panic(err)
}

for _, serviceID := range serviceIDsToUpdate {
for _, poolID := range poolIDs {
err = keepers.RestakingKeeper.AddPoolToServiceSecuringPools(ctx, serviceID, poolID)
if err != nil {
panic(err)
}
}
}
}
4 changes: 2 additions & 2 deletions x/restaking/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (k msgServer) BorrowPoolSecurity(ctx context.Context, msg *types.MsgBorrowP
}

// Ensure the pool is not in the service securing pools list
isSecuring, err := k.IsPoolInServiceSecuringPools(ctx, msg.ServiceID, msg.PoolID)
isSecuring, err := k.IsServiceSecuredByPool(ctx, msg.ServiceID, msg.PoolID)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -307,7 +307,7 @@ func (k msgServer) CeasePoolSecurityBorrow(ctx context.Context, msg *types.MsgCe
}

// Ensure the pool is in the service securing pools list
isSecuring, err := k.IsPoolInServiceSecuringPools(ctx, msg.ServiceID, msg.PoolID)
isSecuring, err := k.IsServiceSecuredByPool(ctx, msg.ServiceID, msg.PoolID)
if err != nil {
return nil, err
}
Expand Down
16 changes: 0 additions & 16 deletions x/restaking/keeper/service_restaking.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,25 +143,9 @@ func (k *Keeper) IsServiceSecuringPoolsConfigured(ctx context.Context, serviceID
return iterator.Valid(), nil
}

// IsPoolInServiceSecuringPools returns true if the pool is in the list
// of pools from which the service can borrow security
func (k *Keeper) IsPoolInServiceSecuringPools(ctx context.Context, serviceID uint32, poolID uint32) (bool, error) {
key := collections.Join(serviceID, poolID)
return k.serviceSecuringPools.Has(ctx, key)
}

// IsServiceSecuredByPool returns true if the service is being secured
// by the given pool
func (k *Keeper) IsServiceSecuredByPool(ctx context.Context, serviceID uint32, poolID uint32) (bool, error) {
configured, err := k.IsServiceSecuringPoolsConfigured(ctx, serviceID)
if err != nil {
return false, err
}
// Allow all when the list is empty
if !configured {
return true, nil
}

key := collections.Join(serviceID, poolID)
return k.serviceSecuringPools.Has(ctx, key)
}
Expand Down
4 changes: 2 additions & 2 deletions x/restaking/simulation/msg_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ func SimulateMsgBorrowPoolSecurity(
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "could not get service"), nil, nil
}

isServiceSecured, err := k.IsPoolInServiceSecuringPools(ctx, service.ID, pool.ID)
isServiceSecured, err := k.IsServiceSecuredByPool(ctx, service.ID, pool.ID)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -413,7 +413,7 @@ func SimulateMsgCeasePoolSecurityBorrow(
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "could not get service"), nil, nil
}

isServiceSecured, err := k.IsPoolInServiceSecuringPools(ctx, service.ID, pool.ID)
isServiceSecured, err := k.IsServiceSecuredByPool(ctx, service.ID, pool.ID)
if err != nil {
panic(err)
}
Expand Down
6 changes: 1 addition & 5 deletions x/rewards/keeper/allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func (k *Keeper) AllocateRewards(ctx context.Context) error {
return err
}

// The list is empty all pools are allowed
pools, err := k.poolsKeeper.GetPools(ctx)
if err != nil {
return err
Expand Down Expand Up @@ -299,10 +298,7 @@ func (k *Keeper) AllocateRewardsByPlan(
}

// getEligiblePools returns a list of pools that are eligible for rewards
// allocation based on the given service's service params. If the service's
// service params don't have any whitelisted pools, then all pools are eligible
// for rewards allocation. Also, if the service is not whitelisted in the pools
// params, then no pools are eligible for rewards allocation.
// allocation.
func (k *Keeper) getEligiblePools(
ctx context.Context,
service servicestypes.Service,
Expand Down
16 changes: 16 additions & 0 deletions x/rewards/keeper/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ func (suite *KeeperTestSuite) TestAllocateRewards_BasicScenario() {
operatorAdmin3 := testutil.TestAddress(10006)
operator3 := suite.CreateOperator(ctx, "Operator3", operatorAdmin3.String())

// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service1.ID, []uint32{1, 2, 3})
// Whitelist only $MILK and $MUSD pools.
suite.AddPoolsToServiceSecuringPools(ctx, service2.ID, []uint32{1, 3})
// Whitelist only Operator2 and Operator3.
Expand Down Expand Up @@ -332,6 +334,8 @@ func (suite *KeeperTestSuite) TestAllocateRewards_ZeroDelegations() {
// Create a service.
serviceAdmin := testutil.TestAddress(10000)
service := suite.CreateService(ctx, "Service", serviceAdmin.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service.ID, []uint32{1})

// Create an active rewards plan.
planStartTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
Expand Down Expand Up @@ -414,6 +418,8 @@ func (suite *KeeperTestSuite) TestAllocateRewards_WeightedDistributions() {
// Create a service.
serviceAdmin := testutil.TestAddress(10000)
service := suite.CreateService(ctx, "Service", serviceAdmin.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service.ID, []uint32{1, 2})

// Create operators.
operatorAdmin1 := testutil.TestAddress(10001)
Expand Down Expand Up @@ -530,6 +536,8 @@ func (suite *KeeperTestSuite) TestAllocateRewards_EgalitarianDistributions() {
// Create a service.
serviceAdmin := testutil.TestAddress(10000)
service := suite.CreateService(ctx, "Service", serviceAdmin.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service.ID, []uint32{1, 2})

// Create operators.
operatorAdmin1 := testutil.TestAddress(10001)
Expand Down Expand Up @@ -639,8 +647,12 @@ func (suite *KeeperTestSuite) TestAllocateRewards_TrustedServices() {
// Create services.
serviceAdmin1 := testutil.TestAddress(10000)
service1 := suite.CreateService(ctx, "Service1", serviceAdmin1.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service1.ID, []uint32{1})
serviceAdmin2 := testutil.TestAddress(10001)
service2 := suite.CreateService(ctx, "Service2", serviceAdmin2.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service2.ID, []uint32{1})

// Call AllocateRewards to set last rewards allocation time.
err := suite.keeper.AllocateRewards(ctx)
Expand Down Expand Up @@ -741,8 +753,12 @@ func (suite *KeeperTestSuite) TestAllocateRewards_UserTrustedServiceUpdated() {
// Create services.
serviceAdmin1 := testutil.TestAddress(10000)
service1 := suite.CreateService(ctx, "Service1", serviceAdmin1.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service1.ID, []uint32{1})
serviceAdmin2 := testutil.TestAddress(10001)
service2 := suite.CreateService(ctx, "Service2", serviceAdmin2.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service2.ID, []uint32{1})

// Call AllocateRewards to set last rewards allocation time.
err := suite.keeper.AllocateRewards(ctx)
Expand Down
2 changes: 2 additions & 0 deletions x/rewards/keeper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,8 @@ func (suite *KeeperTestSuite) setupSampleServiceAndOperator(ctx sdk.Context) (se
// Create a service.
serviceAdmin := testutil.TestAddress(10000)
service := suite.CreateService(ctx, "Service", serviceAdmin.String())
// Whitelist all pools.
suite.AddPoolsToServiceSecuringPools(ctx, service.ID, []uint32{1, 2})

// Create an operator.
operatorAdmin := testutil.TestAddress(10001)
Expand Down

0 comments on commit 1d2e6de

Please sign in to comment.