From 4ba2ec353f4714d3de25ccf93d8e73623080f9b4 Mon Sep 17 00:00:00 2001 From: Redouane Lakrache Date: Wed, 17 Jul 2024 02:32:43 +0200 Subject: [PATCH] [Supplier] Deny supplier staking with unknown services (#693) --- config.yml | 3 ++ .../config/supplier1_stake_config.yaml | 4 --- testutil/integration/app.go | 2 ++ testutil/keeper/proof.go | 14 +++++++++ testutil/keeper/supplier.go | 16 ++++++++++ testutil/keeper/tokenomics.go | 12 ++++++++ x/service/keeper/service.go | 4 +-- x/service/keeper/service_test.go | 2 +- x/service/module/genesis.go | 2 +- x/service/types/expected_keepers.go | 1 + x/supplier/keeper/keeper.go | 7 +++-- .../keeper/msg_server_stake_supplier.go | 8 +++++ .../keeper/msg_server_stake_supplier_test.go | 30 +++++++++++++++++++ x/supplier/module/module.go | 6 ++++ x/supplier/types/errors.go | 1 + x/supplier/types/expected_keepers.go | 8 +++++ 16 files changed, 110 insertions(+), 10 deletions(-) diff --git a/config.yml b/config.yml index 23ab20d44..fe52ad944 100644 --- a/config.yml +++ b/config.yml @@ -183,6 +183,9 @@ genesis: service: params: add_service_fee: "1000000000" + serviceList: + - id: anvil + name: "" proof: params: proof_request_probability: "0.25" diff --git a/localnet/poktrolld/config/supplier1_stake_config.yaml b/localnet/poktrolld/config/supplier1_stake_config.yaml index ca2e931f2..91978a6e7 100644 --- a/localnet/poktrolld/config/supplier1_stake_config.yaml +++ b/localnet/poktrolld/config/supplier1_stake_config.yaml @@ -2,10 +2,6 @@ # so that the stake command causes a state change. stake_amount: 1000069upokt services: - - service_id: svc1 - endpoints: - - publicly_exposed_url: http://localhost:8081 - rpc_type: JSON_RPC # The endpoint URL for the Anvil service is provided via the RelayMiner. # The RelayMiner acts as a proxy, forwarding requests to the actual Anvil data node behind it. # This setup allows for flexible and dynamic service provisioning within the network. diff --git a/testutil/integration/app.go b/testutil/integration/app.go index 05f036e69..76852d661 100644 --- a/testutil/integration/app.go +++ b/testutil/integration/app.go @@ -338,12 +338,14 @@ func NewCompleteIntegrationApp(t *testing.T) *App { logger, authority.String(), bankKeeper, + serviceKeeper, ) supplierModule := supplier.NewAppModule( cdc, supplierKeeper, accountKeeper, bankKeeper, + serviceKeeper, ) // Prepare the session keeper and module diff --git a/testutil/keeper/proof.go b/testutil/keeper/proof.go index 35ab490db..03ea7b4b2 100644 --- a/testutil/keeper/proof.go +++ b/testutil/keeper/proof.go @@ -28,6 +28,7 @@ import ( applicationmocks "github.com/pokt-network/poktroll/testutil/application/mocks" gatewaymocks "github.com/pokt-network/poktroll/testutil/gateway/mocks" "github.com/pokt-network/poktroll/testutil/proof/mocks" + servicemocks "github.com/pokt-network/poktroll/testutil/service/mocks" sessionmocks "github.com/pokt-network/poktroll/testutil/session/mocks" suppliermocks "github.com/pokt-network/poktroll/testutil/supplier/mocks" appkeeper "github.com/pokt-network/poktroll/x/application/keeper" @@ -37,6 +38,8 @@ import ( "github.com/pokt-network/poktroll/x/proof/keeper" "github.com/pokt-network/poktroll/x/proof/types" prooftypes "github.com/pokt-network/poktroll/x/proof/types" + servicekeeper "github.com/pokt-network/poktroll/x/service/keeper" + servicetypes "github.com/pokt-network/poktroll/x/service/types" sessionkeeper "github.com/pokt-network/poktroll/x/session/keeper" sessiontypes "github.com/pokt-network/poktroll/x/session/types" sharedkeeper "github.com/pokt-network/poktroll/x/shared/keeper" @@ -118,6 +121,7 @@ func NewProofModuleKeepers(t testing.TB, opts ...ProofKeepersOpt) (_ *ProofModul gatewaytypes.StoreKey, authtypes.StoreKey, sharedtypes.StoreKey, + servicetypes.StoreKey, ) // Construct a multistore & mount store keys for each keeper that will interact with the state store. @@ -179,6 +183,15 @@ func NewProofModuleKeepers(t testing.TB, opts ...ProofKeepersOpt) (_ *ProofModul ) require.NoError(t, appKeeper.SetParams(ctx, apptypes.DefaultParams())) + // Construct a service keeper need by the supplier keeper. + serviceKeeper := servicekeeper.NewKeeper( + cdc, + runtime.NewKVStoreService(keys[types.StoreKey]), + log.NewNopLogger(), + authority.String(), + servicemocks.NewMockBankKeeper(ctrl), + ) + // Construct a real supplier keeper to add suppliers to sessions. supplierKeeper := supplierkeeper.NewKeeper( cdc, @@ -186,6 +199,7 @@ func NewProofModuleKeepers(t testing.TB, opts ...ProofKeepersOpt) (_ *ProofModul log.NewNopLogger(), authority.String(), suppliermocks.NewMockBankKeeper(ctrl), + serviceKeeper, ) require.NoError(t, supplierKeeper.SetParams(ctx, suppliertypes.DefaultParams())) diff --git a/testutil/keeper/supplier.go b/testutil/keeper/supplier.go index 6f502a316..6f8efb086 100644 --- a/testutil/keeper/supplier.go +++ b/testutil/keeper/supplier.go @@ -20,6 +20,8 @@ import ( "github.com/stretchr/testify/require" "github.com/pokt-network/poktroll/testutil/supplier/mocks" + servicekeeper "github.com/pokt-network/poktroll/x/service/keeper" + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" "github.com/pokt-network/poktroll/x/supplier/keeper" "github.com/pokt-network/poktroll/x/supplier/types" ) @@ -42,6 +44,15 @@ func SupplierKeeper(t testing.TB) (keeper.Keeper, context.Context) { mockBankKeeper := mocks.NewMockBankKeeper(ctrl) mockBankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), types.ModuleName, gomock.Any()).AnyTimes() mockBankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), types.ModuleName, gomock.Any(), gomock.Any()).AnyTimes() + mockBankKeeper.EXPECT().SpendableCoins(gomock.Any(), gomock.Any()).AnyTimes() + + serviceKeeper := servicekeeper.NewKeeper( + cdc, + runtime.NewKVStoreService(storeKey), + log.NewNopLogger(), + authority.String(), + mockBankKeeper, + ) k := keeper.NewKeeper( cdc, @@ -49,6 +60,7 @@ func SupplierKeeper(t testing.TB) (keeper.Keeper, context.Context) { log.NewNopLogger(), authority.String(), mockBankKeeper, + serviceKeeper, ) ctx := sdk.NewContext(stateStore, cmtproto.Header{}, false, log.NewNopLogger()) @@ -56,6 +68,10 @@ func SupplierKeeper(t testing.TB) (keeper.Keeper, context.Context) { // Initialize params require.NoError(t, k.SetParams(ctx, types.DefaultParams())) + // Add existing services used in the test. + serviceKeeper.SetService(ctx, sharedtypes.Service{Id: "svcId"}) + serviceKeeper.SetService(ctx, sharedtypes.Service{Id: "svcId2"}) + return k, ctx } diff --git a/testutil/keeper/tokenomics.go b/testutil/keeper/tokenomics.go index 802634be4..a0b5ffc19 100644 --- a/testutil/keeper/tokenomics.go +++ b/testutil/keeper/tokenomics.go @@ -36,6 +36,8 @@ import ( gatewaytypes "github.com/pokt-network/poktroll/x/gateway/types" proofkeeper "github.com/pokt-network/poktroll/x/proof/keeper" prooftypes "github.com/pokt-network/poktroll/x/proof/types" + servicekeeper "github.com/pokt-network/poktroll/x/service/keeper" + servicetypes "github.com/pokt-network/poktroll/x/service/types" sessionkeeper "github.com/pokt-network/poktroll/x/session/keeper" sessiontypes "github.com/pokt-network/poktroll/x/session/types" sharedkeeper "github.com/pokt-network/poktroll/x/shared/keeper" @@ -290,6 +292,15 @@ func NewTokenomicsModuleKeepers( ) require.NoError(t, appKeeper.SetParams(ctx, apptypes.DefaultParams())) + // Construct a service keeper needed by the supplier keeper. + serviceKeeper := servicekeeper.NewKeeper( + cdc, + runtime.NewKVStoreService(keys[servicetypes.StoreKey]), + log.NewNopLogger(), + authority.String(), + bankKeeper, + ) + // Construct a real supplier keeper to add suppliers to sessions. supplierKeeper := supplierkeeper.NewKeeper( cdc, @@ -297,6 +308,7 @@ func NewTokenomicsModuleKeepers( log.NewNopLogger(), authority.String(), bankKeeper, + serviceKeeper, ) require.NoError(t, supplierKeeper.SetParams(ctx, suppliertypes.DefaultParams())) diff --git a/x/service/keeper/service.go b/x/service/keeper/service.go index fe060e9a7..6c4722195 100644 --- a/x/service/keeper/service.go +++ b/x/service/keeper/service.go @@ -46,8 +46,8 @@ func (k Keeper) RemoveService( store.Delete(types.ServiceKey(serviceId)) } -// GetAllService returns all service -func (k Keeper) GetAllService(ctx context.Context) (services []sharedtypes.Service) { +// GetAllServices returns all services +func (k Keeper) GetAllServices(ctx context.Context) (services []sharedtypes.Service) { storeAdapter := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) store := prefix.NewStore(storeAdapter, types.KeyPrefix(types.ServiceKeyPrefix)) iterator := storetypes.KVStorePrefixIterator(store, []byte{}) diff --git a/x/service/keeper/service_test.go b/x/service/keeper/service_test.go index cd236d332..12cbd52c5 100644 --- a/x/service/keeper/service_test.go +++ b/x/service/keeper/service_test.go @@ -69,6 +69,6 @@ func TestServiceGetAll(t *testing.T) { services := createNServices(keeper, ctx, 10) require.ElementsMatch(t, nullify.Fill(services), - nullify.Fill(keeper.GetAllService(ctx)), + nullify.Fill(keeper.GetAllServices(ctx)), ) } diff --git a/x/service/module/genesis.go b/x/service/module/genesis.go index fded93481..96a1f6275 100644 --- a/x/service/module/genesis.go +++ b/x/service/module/genesis.go @@ -24,7 +24,7 @@ func ExportGenesis(ctx context.Context, k keeper.Keeper) *types.GenesisState { genesis := types.DefaultGenesis() genesis.Params = k.GetParams(ctx) - genesis.ServiceList = k.GetAllService(ctx) + genesis.ServiceList = k.GetAllServices(ctx) // this line is used by starport scaffolding # genesis/module/export return genesis diff --git a/x/service/types/expected_keepers.go b/x/service/types/expected_keepers.go index 8157cc89b..01652cde6 100644 --- a/x/service/types/expected_keepers.go +++ b/x/service/types/expected_keepers.go @@ -22,4 +22,5 @@ type BankKeeper interface { // than "delegate" funds from one account to another which is more closely // linked to staking. SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error + SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error } diff --git a/x/supplier/keeper/keeper.go b/x/supplier/keeper/keeper.go index ef87b8f4d..af1753c79 100644 --- a/x/supplier/keeper/keeper.go +++ b/x/supplier/keeper/keeper.go @@ -21,7 +21,8 @@ type ( // should be the x/gov module account. authority string - bankKeeper types.BankKeeper + bankKeeper types.BankKeeper + serviceKeeper types.ServiceKeeper } ) @@ -32,6 +33,7 @@ func NewKeeper( authority string, bankKeeper types.BankKeeper, + serviceKeeper types.ServiceKeeper, ) Keeper { if _, err := sdk.AccAddressFromBech32(authority); err != nil { panic(fmt.Sprintf("invalid authority address: %s", authority)) @@ -43,7 +45,8 @@ func NewKeeper( authority: authority, logger: logger, - bankKeeper: bankKeeper, + bankKeeper: bankKeeper, + serviceKeeper: serviceKeeper, } } diff --git a/x/supplier/keeper/msg_server_stake_supplier.go b/x/supplier/keeper/msg_server_stake_supplier.go index f0a823d0f..56cd1ee5e 100644 --- a/x/supplier/keeper/msg_server_stake_supplier.go +++ b/x/supplier/keeper/msg_server_stake_supplier.go @@ -27,6 +27,14 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *types.MsgStakeSupplie return nil, err } + // Check if the services the supplier is staking for exist + for _, serviceConfig := range msg.Services { + if _, serviceFound := k.serviceKeeper.GetService(ctx, serviceConfig.Service.Id); !serviceFound { + logger.Error(fmt.Sprintf("service %q does not exist", serviceConfig.Service.Id)) + return nil, types.ErrSupplierServiceNotFound.Wrapf("service %q does not exist", serviceConfig.Service.Id) + } + } + // Check if the supplier already exists or not var err error var coinsToEscrow sdk.Coin diff --git a/x/supplier/keeper/msg_server_stake_supplier_test.go b/x/supplier/keeper/msg_server_stake_supplier_test.go index b0d3f1d50..8e4e0c247 100644 --- a/x/supplier/keeper/msg_server_stake_supplier_test.go +++ b/x/supplier/keeper/msg_server_stake_supplier_test.go @@ -234,3 +234,33 @@ func TestMsgServer_StakeSupplier_FailLoweringStake(t *testing.T) { require.Equal(t, int64(100), supplierFound.Stake.Amount.Int64()) require.Len(t, supplierFound.Services, 1) } + +func TestMsgServer_StakeSupplier_FailWithNonExistingService(t *testing.T) { + k, ctx := keepertest.SupplierKeeper(t) + srv := keeper.NewMsgServerImpl(k) + + // Prepare the supplier + supplierAddr := sample.AccAddress() + stakeMsg := &types.MsgStakeSupplier{ + Address: supplierAddr, + Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(100)}, + Services: []*sharedtypes.SupplierServiceConfig{ + { + Service: &sharedtypes.Service{ + Id: "newService", + }, + Endpoints: []*sharedtypes.SupplierEndpoint{ + { + Url: "http://localhost:8080", + RpcType: sharedtypes.RPCType_JSON_RPC, + Configs: make([]*sharedtypes.ConfigOption, 0), + }, + }, + }, + }, + } + + // Stake the supplier & verify that it fails because the service does not exist. + _, err := srv.StakeSupplier(ctx, stakeMsg) + require.ErrorIs(t, err, types.ErrSupplierServiceNotFound) +} diff --git a/x/supplier/module/module.go b/x/supplier/module/module.go index 8ac44e592..fd71be1b8 100644 --- a/x/supplier/module/module.go +++ b/x/supplier/module/module.go @@ -98,6 +98,7 @@ type AppModule struct { keeper keeper.Keeper accountKeeper types.AccountKeeper bankKeeper types.BankKeeper + serviceKeeper types.ServiceKeeper } func NewAppModule( @@ -105,12 +106,14 @@ func NewAppModule( keeper keeper.Keeper, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, + serviceKeeper types.ServiceKeeper, ) AppModule { return AppModule{ AppModuleBasic: NewAppModuleBasic(cdc), keeper: keeper, accountKeeper: accountKeeper, bankKeeper: bankKeeper, + serviceKeeper: serviceKeeper, } } @@ -179,6 +182,7 @@ type ModuleInputs struct { AccountKeeper types.AccountKeeper BankKeeper types.BankKeeper + ServiceKeeper types.ServiceKeeper } type ModuleOutputs struct { @@ -200,12 +204,14 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { in.Logger, authority.String(), in.BankKeeper, + in.ServiceKeeper, ) m := NewAppModule( in.Cdc, k, in.AccountKeeper, in.BankKeeper, + in.ServiceKeeper, ) return ModuleOutputs{SupplierKeeper: k, Module: m} diff --git a/x/supplier/types/errors.go b/x/supplier/types/errors.go index c89c13e39..6ceca8f4e 100644 --- a/x/supplier/types/errors.go +++ b/x/supplier/types/errors.go @@ -16,4 +16,5 @@ var ( ErrSupplierInvalidSessionId = sdkerrors.Register(ModuleName, 1107, "invalid session ID") ErrSupplierInvalidService = sdkerrors.Register(ModuleName, 1108, "invalid service in supplier") ErrSupplierInvalidSessionEndHeight = sdkerrors.Register(ModuleName, 1109, "invalid session ending height") + ErrSupplierServiceNotFound = sdkerrors.Register(ModuleName, 1110, "service not found") ) diff --git a/x/supplier/types/expected_keepers.go b/x/supplier/types/expected_keepers.go index ba491c020..90d24fee0 100644 --- a/x/supplier/types/expected_keepers.go +++ b/x/supplier/types/expected_keepers.go @@ -6,6 +6,8 @@ import ( "context" sdk "github.com/cosmos/cosmos-sdk/types" + + sharedtypes "github.com/pokt-network/poktroll/x/shared/types" ) // AccountKeeper defines the expected interface for the Account module. @@ -16,6 +18,12 @@ type AccountKeeper interface { // BankKeeper defines the expected interface for the Bank module. type BankKeeper interface { + SpendableCoins(context.Context, sdk.AccAddress) sdk.Coins SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error } + +// ServiceKeeper defines the expected interface for the Service module. +type ServiceKeeper interface { + GetService(ctx context.Context, serviceId string) (sharedtypes.Service, bool) +}