diff --git a/api/poktroll/application/types.pulsar.go b/api/poktroll/application/types.pulsar.go index a2e9a5f2f..4a7751438 100644 --- a/api/poktroll/application/types.pulsar.go +++ b/api/poktroll/application/types.pulsar.go @@ -2190,11 +2190,14 @@ type Application struct { Address string `protobuf:"bytes,1,opt,name=address,proto3" json:"address,omitempty"` // The Bech32 address of the application. Stake *v1beta1.Coin `protobuf:"bytes,2,opt,name=stake,proto3" json:"stake,omitempty"` // The total amount of uPOKT the application has staked - // TODO_BETA(@red-0ne, @olshansk): Limit this to one service_config. - // - // Remove `repeated`, drop the `s` from service_configs and document why - // this is the case in the app config (and here) per this discussion: - // https://github.com/pokt-network/poktroll/pull/750#discussion_r1735025033 + // As per this discussion: + // https://github.com/pokt-network/poktroll/pull/750#discussion_r1735025033 + // The number of service_configs is limited to 1 per service. + // This is to ensure that an application could not over service by making multiple + // claims settelments compete to burn the same stake. + // A slice of service_configs is still maintained to allow for future multi-service + // capabilities to be added, such as off-chain application stake tracking by suppliers: + // https://www.notion.so/buildwithgrove/Off-chain-Application-Stake-Tracking-6a8bebb107db4f7f9dc62cbe7ba555f7 ServiceConfigs []*shared.ApplicationServiceConfig `protobuf:"bytes,3,rep,name=service_configs,json=serviceConfigs,proto3" json:"service_configs,omitempty"` // The list of services this appliccation is configured to request service for // TODO_BETA: Rename `delegatee_gateway_addresses` to `gateway_addresses_delegated_to`. // Ensure to rename all relevant configs, comments, variables, function names, etc as well. diff --git a/config.yml b/config.yml index 5ce114e81..942614aca 100644 --- a/config.yml +++ b/config.yml @@ -171,25 +171,25 @@ genesis: denom: upokt applicationList: - address: pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4 - delegatee_gateway_addresses: [] + delegatee_gateway_addresses: [ + pokt15vzxjqklzjtlz7lahe8z2dfe9nm5vxwwmscne4 + ] service_configs: - service_id: anvil - - service_id: rest - - service_id: ollama stake: # NB: This value should be exactly 1upokt smaller than the value in - # `supplier1_stake_config.yaml` so that the stake command causes a state change. + # `application1_stake_config.yaml` so that the stake command causes a state change. amount: "100000068" # ~100 POKT denom: upokt - - address: pokt1ad28jdap2zfanjd7hpkh984yveney6k9a42man - delegatee_gateway_addresses: [] + - address: pokt184zvylazwu4queyzpl0gyz9yf5yxm2kdhh9hpm + delegatee_gateway_addresses: [ + pokt15vzxjqklzjtlz7lahe8z2dfe9nm5vxwwmscne4 + ] service_configs: - - service_id: anvil - service_id: rest - - service_id: ollama stake: # NB: This value should be exactly 1upokt smaller than the value in - # `supplier1_stake_config.yaml` so that the stake command causes a state change. + # `application1_stake_config.yaml` so that the stake command causes a state change. amount: "100000068" # ~100 POKT denom: upokt supplier: @@ -228,7 +228,7 @@ genesis: rev_share_percentage: "100" stake: # NB: This value should be exactly 1upokt smaller than the value in - # `application1_stake_config.yaml` so that the stake command causes a state change. + # `supplier1_stake_config.yaml` so that the stake command causes a state change. amount: "1000068" denom: upokt gateway: diff --git a/docusaurus/docs/operate/configs/app_staking_config.md b/docusaurus/docs/operate/configs/app_staking_config.md index bd640e85f..88282c572 100644 --- a/docusaurus/docs/operate/configs/app_staking_config.md +++ b/docusaurus/docs/operate/configs/app_staking_config.md @@ -70,6 +70,18 @@ service_ids: - ``` +:::warning + +The `service_ids` list must contain a unique entry. + +The current protocol requires the `service_ids` list to contain **EXACTLY ONE** entry +to prevent `Application`s from over-servicing. + +A detailed explanation of why this is the case can be found in +[Tokenomis/TLM](protocol/tokenomics/token_logic_modules.md#tlm-pre-processing). + +::: + Defines the list of services the `Application` is willing to consume on the Pocket network. Each entry in the list is a `service_id` that identifies a service that is available on Pocket network. diff --git a/docusaurus/docs/protocol/tokenomics/token_logic_modules.md b/docusaurus/docs/protocol/tokenomics/token_logic_modules.md index d8afc1c37..917d808c9 100644 --- a/docusaurus/docs/protocol/tokenomics/token_logic_modules.md +++ b/docusaurus/docs/protocol/tokenomics/token_logic_modules.md @@ -110,6 +110,24 @@ flowchart TB class CC question; ``` +:::warning + +In order for the `MaxClaimableAmount` to prevent Applications from over-servicing, +the `Application.Stake` must be claimable only by `Supplier`s from the same session +(i.e. the same service). + +For a given application `MaxClaimableAmount` is `Application.Stake / NumSuppliersPerSession` +and defined per session/service. + +If an `Application` is able send traffic to `n` services then it could be over-servicing +up to `n` times its stake for a given session number by performing +`MaxClaimableAmount * NumSuppliersPerSession * n > Application.Stake` worth of work. + +To avoid thy type of over-servicing, The Pocket protocol requires `Application`s +to only be able to stake for EXACTLY ONE service. + +::: + :::note TODO_POST_MAINNET: After the Shannon upgrade, the team at Grove has a lot of ideas diff --git a/e2e/tests/init_test.go b/e2e/tests/init_test.go index c4df34b45..e9aaa41e7 100644 --- a/e2e/tests/init_test.go +++ b/e2e/tests/init_test.go @@ -61,7 +61,10 @@ var ( flagFeaturesPath string keyRingFlag = "--keyring-backend=test" chainIdFlag = "--chain-id=poktroll" - appGateServerUrl = "http://localhost:42069" // Keeping localhost by default because that is how we run the tests on our machines locally + // Keeping localhost by default because that is how we run the tests on our machines locally + // gatewayUrl is pointing to a non-sovereign app gate server so multiple + // apps could relay through it. + gatewayUrl = "http://localhost:42079" ) func init() { @@ -71,9 +74,9 @@ func init() { flag.StringVar(&flagFeaturesPath, "features-path", "*.feature", "Specifies glob paths for the runner to look up .feature files") - // If "APPGATE_SERVER_URL" envar is present, use it for appGateServerUrl - if url := os.Getenv("APPGATE_SERVER_URL"); url != "" { - appGateServerUrl = url + // If "GATEWAY_URL" envar is present, use it for appGateServerUrl + if url := os.Getenv("GATEWAY_URL"); url != "" { + gatewayUrl = url } } @@ -453,22 +456,20 @@ func (s *suite) TheSessionForApplicationAndServiceContainsTheSupplier(appName st } func (s *suite) TheApplicationSendsTheSupplierASuccessfulRequestForServiceWithPathAndData(appName, supplierOperatorName, serviceId, path, requestData string) { - // TODO_HACK: We need to support a non self_signing LocalNet AppGateServer - // that allows any application to send a relay in LocalNet and our E2E Tests. - require.Equal(s, "app1", appName, "TODO_HACK: The LocalNet AppGateServer is self_signing and only supports app1.") - method := "POST" // If requestData is empty, assume a GET request if requestData == "" { method = "GET" } - res, err := s.pocketd.RunCurlWithRetry(appGateServerUrl, serviceId, method, path, requestData, 5) + appAddr := accNameToAddrMap[appName] + + res, err := s.pocketd.RunCurlWithRetry(gatewayUrl, serviceId, method, path, appAddr, requestData, 5) require.NoError(s, err, "error sending relay request from app %q to supplier %q for service %q", appName, supplierOperatorName, serviceId) var jsonContent json.RawMessage err = json.Unmarshal([]byte(res.Stdout), &jsonContent) - require.NoError(s, err, `Expected valid JSON, got: %s`, res.Stdout) + require.NoError(s, err, `Expected valid JSON, got: %s`) jsonMap, err := jsonToMap(jsonContent) require.NoError(s, err, "error converting JSON to map") diff --git a/e2e/tests/node.go b/e2e/tests/node.go index d6a446e29..ada189be0 100644 --- a/e2e/tests/node.go +++ b/e2e/tests/node.go @@ -5,6 +5,7 @@ package e2e import ( "bytes" "fmt" + "net/url" "os" "os/exec" "strings" @@ -53,7 +54,7 @@ type commandResult struct { type PocketClient interface { RunCommand(args ...string) (*commandResult, error) RunCommandOnHost(rpcUrl string, args ...string) (*commandResult, error) - RunCurl(rpcUrl, service, method, path, data string, args ...string) (*commandResult, error) + RunCurl(rpcUrl, service, method, path, appAddr, data string, args ...string) (*commandResult, error) } // Ensure that pocketdBin struct fulfills PocketClient @@ -98,24 +99,24 @@ func (p *pocketdBin) RunCommandOnHostWithRetry(rpcUrl string, numRetries uint8, } // RunCurl runs a curl command on the local machine -func (p *pocketdBin) RunCurl(rpcUrl, service, method, path, data string, args ...string) (*commandResult, error) { +func (p *pocketdBin) RunCurl(rpcUrl, service, method, path, appAddr, data string, args ...string) (*commandResult, error) { if rpcUrl == "" { rpcUrl = defaultAppGateServerURL } - return p.runCurlCmd(rpcUrl, service, method, path, data, args...) + return p.runCurlCmd(rpcUrl, service, method, path, appAddr, data, args...) } // RunCurlWithRetry runs a curl command on the local machine with multiple retries. // It also accounts for an ephemeral error that may occur due to DNS resolution such as "no such host". -func (p *pocketdBin) RunCurlWithRetry(rpcUrl, service, method, path, data string, numRetries uint8, args ...string) (*commandResult, error) { +func (p *pocketdBin) RunCurlWithRetry(rpcUrl, service, method, path, appAddr, data string, numRetries uint8, args ...string) (*commandResult, error) { // No more retries left if numRetries <= 0 { - return p.RunCurl(rpcUrl, service, method, path, data, args...) + return p.RunCurl(rpcUrl, service, method, path, appAddr, data, args...) } // Run the curl command - res, err := p.RunCurl(rpcUrl, service, method, path, data, args...) + res, err := p.RunCurl(rpcUrl, service, method, path, appAddr, data, args...) if err != nil { - return p.RunCurlWithRetry(rpcUrl, service, method, path, data, numRetries-1, args...) + return p.RunCurlWithRetry(rpcUrl, service, method, path, appAddr, data, numRetries-1, args...) } // TODO_HACK: This is a list of common flaky / ephemeral errors that can occur @@ -130,7 +131,7 @@ func (p *pocketdBin) RunCurlWithRetry(rpcUrl, service, method, path, data string fmt.Println("Retrying due to ephemeral error:", res.Stdout) } time.Sleep(10 * time.Millisecond) - return p.RunCurlWithRetry(rpcUrl, service, method, path, data, numRetries-1, args...) + return p.RunCurlWithRetry(rpcUrl, service, method, path, appAddr, data, numRetries-1, args...) } } @@ -171,20 +172,39 @@ func (p *pocketdBin) runPocketCmd(args ...string) (*commandResult, error) { } // runCurlPostCmd is a helper to run a command using the local pocketd binary with the flags provided -func (p *pocketdBin) runCurlCmd(rpcUrl, service, method, path, data string, args ...string) (*commandResult, error) { +func (p *pocketdBin) runCurlCmd(rpcBaseURL, service, method, path, appAddr, data string, args ...string) (*commandResult, error) { + rpcUrl, err := url.Parse(rpcBaseURL) + if err != nil { + return nil, err + } + + if len(service) > 0 { + rpcUrl.Path = rpcUrl.Path + service + } + // Ensure that if a path is provided, it starts with a "/". // This is required for RESTful APIs that use a path to identify resources. // For JSON-RPC APIs, the resource path should be empty, so empty paths are allowed. if len(path) > 0 && path[0] != '/' { path = "/" + path } - urlStr := fmt.Sprintf("%s/%s%s", rpcUrl, service, path) + + rpcUrl.Path = rpcUrl.Path + path + + // When sending a relay request, through a gateway (i.e. non-sovereign application) + // then, the application address must be provided. + if len(appAddr) > 0 { + queryValues := rpcUrl.Query() + queryValues.Set("applicationAddr", appAddr) + rpcUrl.RawQuery = queryValues.Encode() + } + base := []string{ "-v", // verbose output "-sS", // silent with error "-X", method, // HTTP method "-H", "Content-Type: application/json", // HTTP headers - urlStr, + rpcUrl.String(), } if method == "POST" { @@ -200,7 +220,7 @@ func (p *pocketdBin) runCurlCmd(rpcUrl, service, method, path, data string, args cmd.Stdout = &stdoutBuf cmd.Stderr = &stderrBuf - err := cmd.Run() + err = cmd.Run() r := &commandResult{ Command: commandStr, // Set the command string Stdout: stdoutBuf.String(), diff --git a/e2e/tests/relay.feature b/e2e/tests/relay.feature index 64011ce28..023eedf06 100644 --- a/e2e/tests/relay.feature +++ b/e2e/tests/relay.feature @@ -10,10 +10,11 @@ Feature: Relay Namespace Scenario: App can send a REST relay to Supplier Given the user has the pocketd binary installed - And the application "app1" is staked for service "rest" + # Account "app2" is staked for service "rest" + And the application "app2" is staked for service "rest" And the supplier "supplier1" is staked for service "rest" - And the session for application "app1" and service "rest" contains the supplier "supplier1" - When the application "app1" sends the supplier "supplier1" a successful request for service "rest" with path "/quote" + And the session for application "app2" and service "rest" contains the supplier "supplier1" + When the application "app2" sends the supplier "supplier1" a successful request for service "rest" with path "/quote" Then a "tokenomics" module "ClaimSettled" end block event is broadcast # TODO_TEST(@Olshansk): diff --git a/e2e/tests/stake_app.feature b/e2e/tests/stake_app.feature index eef0ee69f..f9d1461de 100644 --- a/e2e/tests/stake_app.feature +++ b/e2e/tests/stake_app.feature @@ -1,26 +1,28 @@ Feature: Stake App Namespaces + # This test assume the account for app3 IS NOT STAKED at genesis Scenario: User can stake an Application waiting for it to unbond Given the user has the pocketd binary installed - And the user verifies the "application" for account "app2" is not staked - And the account "app2" has a balance greater than "1000070" uPOKT - When the user stakes a "application" with "1000070" uPOKT for "anvil" service from the account "app2" + And the user verifies the "application" for account "app3" is not staked + And the account "app3" has a balance greater than "1000070" uPOKT + When the user stakes a "application" with "1000070" uPOKT for "anvil" service from the account "app3" Then the user should be able to see standard output containing "txhash:" And the user should be able to see standard output containing "code: 0" And the pocketd binary should exit without error And the user should wait for the "application" module "StakeApplication" message to be submitted - And the "application" for account "app2" is staked with "1000070" uPOKT - And the account balance of "app2" should be "1000070" uPOKT "less" than before + And the "application" for account "app3" is staked with "1000070" uPOKT + And the account balance of "app3" should be "1000070" uPOKT "less" than before + # Use the app3 account which is not staked at genesis time Scenario: User can unstake an Application Given the user has the pocketd binary installed - And the "application" for account "app2" is staked with "1000070" uPOKT - And an account exists for "app2" - When the user unstakes a "application" from the account "app2" + And the "application" for account "app3" is staked with "1000070" uPOKT + And an account exists for "app3" + When the user unstakes a "application" from the account "app3" Then the user should be able to see standard output containing "txhash:" And the user should be able to see standard output containing "code: 0" And the pocketd binary should exit without error - And the application for account "app2" is in the "unbonding" period - When the user waits for the application for account "app2" "unbonding" period to finish - And the user verifies the "application" for account "app2" is not staked - And the account balance of "app2" should be "1000070" uPOKT "more" than before \ No newline at end of file + And the application for account "app3" is in the "unbonding" period + When the user waits for the application for account "app3" "unbonding" period to finish + And the user verifies the "application" for account "app3" is not staked + And the account balance of "app3" should be "1000070" uPOKT "more" than before \ No newline at end of file diff --git a/e2e/tests/stake_supplier.feature b/e2e/tests/stake_supplier.feature index de0440165..b24a579d9 100644 --- a/e2e/tests/stake_supplier.feature +++ b/e2e/tests/stake_supplier.feature @@ -2,8 +2,9 @@ Feature: Stake Supplier Namespace Scenario: User can stake a Supplier Given the user has the pocketd binary installed + # TODO_UPNEXT: Set the supplier stake fee once it is a param. And the user verifies the "supplier" for account "supplier2" is not staked - And the account "supplier2" has a balance greater than "1000070" uPOKT + And the account "supplier2" has a balance greater than "1000071" uPOKT When the user stakes a "supplier" with "1000070" uPOKT for "anvil" service from the account "supplier2" Then the user should be able to see standard output containing "txhash:" And the user should be able to see standard output containing "code: 0" @@ -11,7 +12,7 @@ Feature: Stake Supplier Namespace And the user should wait for the "supplier" module "StakeSupplier" message to be submitted And the user should wait for the "supplier" module "SupplierStaked" tx event to be broadcast And the "supplier" for account "supplier2" is staked with "1000070" uPOKT - And the account balance of "supplier2" should be "1000070" uPOKT "less" than before + And the account balance of "supplier2" should be "1000071" uPOKT "less" than before Scenario: User can unstake a Supplier Given the user has the pocketd binary installed diff --git a/go.mod b/go.mod index 8e7700b69..91de15f1d 100644 --- a/go.mod +++ b/go.mod @@ -82,7 +82,6 @@ require ( require ( cosmossdk.io/x/tx v0.13.4 github.com/jhump/protoreflect v1.16.0 - go.uber.org/mock v0.4.0 ) require ( diff --git a/go.sum b/go.sum index b35917516..62dc61bb0 100644 --- a/go.sum +++ b/go.sum @@ -1213,8 +1213,6 @@ go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0 go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= -go.uber.org/mock v0.4.0 h1:VcM4ZOtdbR4f6VXfiOpwpVJDL6lCReaZ6mw31wqh7KU= -go.uber.org/mock v0.4.0/go.mod h1:a6FSlNadKUHUa9IP5Vyt1zh4fC7uAwxMutEAscFbkZc= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= diff --git a/pkg/client/tx/client_integration_test.go b/pkg/client/tx/client_integration_test.go index 034b9de4c..ac2ee52fb 100644 --- a/pkg/client/tx/client_integration_test.go +++ b/pkg/client/tx/client_integration_test.go @@ -58,7 +58,7 @@ func TestTxClient_SignAndBroadcast_Integration(t *testing.T) { appStakeMsg := &apptypes.MsgStakeApplication{ Address: signingKeyAddr.String(), Stake: &appStake, - Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 2), + Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 1), } // Sign and broadcast the message. diff --git a/pkg/client/tx/client_test.go b/pkg/client/tx/client_test.go index b745dfbb1..e39985641 100644 --- a/pkg/client/tx/client_test.go +++ b/pkg/client/tx/client_test.go @@ -102,7 +102,7 @@ func TestTxClient_SignAndBroadcast_Succeeds(t *testing.T) { appStakeMsg := &apptypes.MsgStakeApplication{ Address: signingKeyAddr.String(), Stake: &appStake, - Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 2), + Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 1), } // Sign and broadcast the message. @@ -339,7 +339,7 @@ $ go test -v -count=1 -run TestTxClient_SignAndBroadcast_CheckTxError ./pkg/clie appStakeMsg := &apptypes.MsgStakeApplication{ Address: signingKeyAddr.String(), Stake: &appStake, - Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 2), + Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 1), } // Sign and broadcast the message. @@ -411,7 +411,7 @@ func TestTxClient_SignAndBroadcast_Timeout(t *testing.T) { appStakeMsg := &apptypes.MsgStakeApplication{ Address: signingKeyAddr.String(), Stake: &appStake, - Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 2), + Services: client.NewTestApplicationServiceConfig(testServiceIdPrefix, 1), } // Sign and broadcast the message in a transaction. diff --git a/proto/poktroll/application/types.proto b/proto/poktroll/application/types.proto index 5752c5d19..c4c0625cb 100644 --- a/proto/poktroll/application/types.proto +++ b/proto/poktroll/application/types.proto @@ -18,10 +18,12 @@ import "poktroll/shared/service.proto"; message Application { string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the application. cosmos.base.v1beta1.Coin stake = 2; // The total amount of uPOKT the application has staked - // TODO_BETA(@red-0ne, @olshansk): Limit this to one service_config. - // Remove `repeated`, drop the `s` from service_configs and document why - // this is the case in the app config (and here) per this discussion: - // https://github.com/pokt-network/poktroll/pull/750#discussion_r1735025033 + // CRITICAL_DEV_NOTE: The number of service_configs must be EXACTLY ONE. + // This prevents applications from over-servicing. + // The field is kept repeated (a list) for both legacy and future logic reaosns. + // References: + // - https://github.com/pokt-network/poktroll/pull/750#discussion_r1735025033 + // - https://www.notion.so/buildwithgrove/Off-chain-Application-Stake-Tracking-6a8bebb107db4f7f9dc62cbe7ba555f7 repeated poktroll.shared.ApplicationServiceConfig service_configs = 3; // The list of services this appliccation is configured to request service for // TODO_BETA: Rename `delegatee_gateway_addresses` to `gateway_addresses_delegated_to`. // Ensure to rename all relevant configs, comments, variables, function names, etc as well. diff --git a/testutil/keeper/supplier.go b/testutil/keeper/supplier.go index bef17530d..83527c27a 100644 --- a/testutil/keeper/supplier.go +++ b/testutil/keeper/supplier.go @@ -33,7 +33,7 @@ type SupplierModuleKeepers struct { *keeper.Keeper types.SharedKeeper // Tracks the amount of funds returned to the supplier owner when the supplier is unbonded. - SupplierUnstakedFundsMap map[string]int64 + SupplierBalanceMap map[string]int64 } func SupplierKeeper(t testing.TB) (SupplierModuleKeepers, context.Context) { @@ -53,17 +53,19 @@ func SupplierKeeper(t testing.TB) (SupplierModuleKeepers, context.Context) { cdc := codec.NewProtoCodec(registry) authority := authtypes.NewModuleAddress(govtypes.ModuleName) - // Set a simple map to track the where the supplier stake is returned when - // the supplier is unbonded. - supplierUnstakedFundsMap := make(map[string]int64) + // Set a simple map to track the suppliers balances. + supplierBalanceMap := make(map[string]int64) ctrl := gomock.NewController(t) mockBankKeeper := mocks.NewMockBankKeeper(ctrl) - mockBankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), types.ModuleName, gomock.Any()).AnyTimes() + mockBankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), types.ModuleName, gomock.Any()).AnyTimes(). + Do(func(ctx context.Context, addr sdk.AccAddress, module string, coins sdk.Coins) { + supplierBalanceMap[addr.String()] -= coins[0].Amount.Int64() + }) mockBankKeeper.EXPECT().SpendableCoins(gomock.Any(), gomock.Any()).AnyTimes() mockBankKeeper.EXPECT().SendCoinsFromModuleToAccount(gomock.Any(), types.ModuleName, gomock.Any(), gomock.Any()).AnyTimes(). Do(func(ctx context.Context, module string, addr sdk.AccAddress, coins sdk.Coins) { - supplierUnstakedFundsMap[addr.String()] += coins[0].Amount.Int64() + supplierBalanceMap[addr.String()] += coins[0].Amount.Int64() }) // Construct a real shared keeper. @@ -104,9 +106,9 @@ func SupplierKeeper(t testing.TB) (SupplierModuleKeepers, context.Context) { serviceKeeper.SetService(ctx, sharedtypes.Service{Id: "svcId2"}) supplierModuleKeepers := SupplierModuleKeepers{ - Keeper: &supplierKeeper, - SharedKeeper: sharedKeeper, - SupplierUnstakedFundsMap: supplierUnstakedFundsMap, + Keeper: &supplierKeeper, + SharedKeeper: sharedKeeper, + SupplierBalanceMap: supplierBalanceMap, } return supplierModuleKeepers, ctx diff --git a/x/application/keeper/msg_server_stake_application_test.go b/x/application/keeper/msg_server_stake_application_test.go index aae43e4e1..ba0aed48c 100644 --- a/x/application/keeper/msg_server_stake_application_test.go +++ b/x/application/keeper/msg_server_stake_application_test.go @@ -74,13 +74,12 @@ func TestMsgServer_StakeApplication_SuccessfulCreateAndUpdate(t *testing.T) { require.Len(t, foundApp.ServiceConfigs, 1) require.Equal(t, "svc1", foundApp.ServiceConfigs[0].ServiceId) - // Prepare an updated application with a higher stake and another service + // Prepare an updated application with a higher stake and a different service upStake := initialStake.AddAmount(math.NewInt(100)) updateStakeMsg := &apptypes.MsgStakeApplication{ Address: appAddr, Stake: &upStake, Services: []*sharedtypes.ApplicationServiceConfig{ - {ServiceId: "svc1"}, {ServiceId: "svc2"}, }, } @@ -91,10 +90,8 @@ func TestMsgServer_StakeApplication_SuccessfulCreateAndUpdate(t *testing.T) { foundApp, isAppFound = k.GetApplication(ctx, appAddr) require.True(t, isAppFound) require.Equal(t, &upStake, foundApp.Stake) - require.Len(t, foundApp.ServiceConfigs, 2) - require.Equal(t, "svc1", foundApp.ServiceConfigs[0].ServiceId) - require.Equal(t, "svc2", foundApp.ServiceConfigs[1].ServiceId) - + require.Len(t, foundApp.ServiceConfigs, 1) + require.Equal(t, "svc2", foundApp.ServiceConfigs[0].ServiceId) // Assert that the EventApplicationStaked event is emitted. expectedEvent, err = cosmostypes.TypedEventToEvent( &apptypes.EventApplicationStaked{ diff --git a/x/application/module/tx_stake_application_test.go b/x/application/module/tx_stake_application_test.go index c5f31bb6a..a59d767e9 100644 --- a/x/application/module/tx_stake_application_test.go +++ b/x/application/module/tx_stake_application_test.go @@ -44,8 +44,6 @@ func TestCLI_StakeApplication(t *testing.T) { stake_amount: 1000upokt service_ids: - svc1 - - svc2 - - svc3 ` tests := []struct { @@ -92,8 +90,6 @@ func TestCLI_StakeApplication(t *testing.T) { stake_amount: # explicitly missing service_ids: - svc1 - - svc2 - - svc3 `, expectedErr: types.ErrAppInvalidStake, @@ -106,8 +102,6 @@ func TestCLI_StakeApplication(t *testing.T) { stake_amount: 1000invalid service_ids: - svc1 - - svc2 - - svc3 `, expectedErr: types.ErrAppInvalidStake, @@ -120,8 +114,6 @@ func TestCLI_StakeApplication(t *testing.T) { stake_amount: 0upokt service_ids: - svc1 - - svc2 - - svc3 `, expectedErr: types.ErrAppInvalidStake, @@ -134,8 +126,6 @@ func TestCLI_StakeApplication(t *testing.T) { stake_amount: -1000upokt service_ids: - svc1 - - svc2 - - svc3 `, expectedErr: types.ErrAppInvalidStake, @@ -165,13 +155,13 @@ func TestCLI_StakeApplication(t *testing.T) { expectedErr: types.ErrAppInvalidServiceConfigs, }, { - desc: "invalid: one of two services is invalid because it contains spaces", + desc: "invalid: contains multiple services", appAddr: appAccount.Address.String(), appConfig: ` stake_amount: 1000upokt service_ids: - - svc1 svc1_part2 + - svc1 - svc2 `, @@ -184,7 +174,6 @@ func TestCLI_StakeApplication(t *testing.T) { appConfig: ` stake_amount: 1000upokt service_ids: - - svc1, - abcdefghi `, diff --git a/x/application/types/message_stake_application_test.go b/x/application/types/message_stake_application_test.go index 152ae71e7..4b94ab42b 100644 --- a/x/application/types/message_stake_application_test.go +++ b/x/application/types/message_stake_application_test.go @@ -94,7 +94,7 @@ func TestMsgStakeApplication_ValidateBasic(t *testing.T) { // service related tests { - desc: "valid service configs - multiple services", + desc: "invalid service configs - multiple services", msg: MsgStakeApplication{ Address: sample.AccAddress(), Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(100)}, @@ -103,6 +103,7 @@ func TestMsgStakeApplication_ValidateBasic(t *testing.T) { {ServiceId: "svc2"}, }, }, + expectedErr: ErrAppInvalidServiceConfigs, }, { desc: "invalid service configs - not present", diff --git a/x/application/types/types.pb.go b/x/application/types/types.pb.go index 47d91e6b5..9e5b8ff6c 100644 --- a/x/application/types/types.pb.go +++ b/x/application/types/types.pb.go @@ -31,10 +31,14 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package type Application struct { Address string `protobuf:"bytes,1,opt,name=address,proto3" json:"address,omitempty"` Stake *types.Coin `protobuf:"bytes,2,opt,name=stake,proto3" json:"stake,omitempty"` - // TODO_BETA(@red-0ne, @olshansk): Limit this to one service_config. - // Remove `repeated`, drop the `s` from service_configs and document why - // this is the case in the app config (and here) per this discussion: - // https://github.com/pokt-network/poktroll/pull/750#discussion_r1735025033 + // As per this discussion: + // https://github.com/pokt-network/poktroll/pull/750#discussion_r1735025033 + // The number of service_configs is limited to 1 per service. + // This is to ensure that an application could not over service by making multiple + // claims settelments compete to burn the same stake. + // A slice of service_configs is still maintained to allow for future multi-service + // capabilities to be added, such as off-chain application stake tracking by suppliers: + // https://www.notion.so/buildwithgrove/Off-chain-Application-Stake-Tracking-6a8bebb107db4f7f9dc62cbe7ba555f7 ServiceConfigs []*types1.ApplicationServiceConfig `protobuf:"bytes,3,rep,name=service_configs,json=serviceConfigs,proto3" json:"service_configs,omitempty"` // TODO_BETA: Rename `delegatee_gateway_addresses` to `gateway_addresses_delegated_to`. // Ensure to rename all relevant configs, comments, variables, function names, etc as well. diff --git a/x/shared/types/service_configs.go b/x/shared/types/service_configs.go index 076ebd6ee..2538d3152 100644 --- a/x/shared/types/service_configs.go +++ b/x/shared/types/service_configs.go @@ -12,8 +12,8 @@ const ( // ValidateAppServiceConfigs returns an error if any of the application service configs are invalid func ValidateAppServiceConfigs(services []*ApplicationServiceConfig) error { - if len(services) == 0 { - return fmt.Errorf("no services configs provided for application: %v", services) + if len(services) != 1 { + return fmt.Errorf("application must have exactly one service: %v", services) } for _, serviceConfig := range services { if serviceConfig == nil { diff --git a/x/supplier/keeper/msg_server_stake_supplier.go b/x/supplier/keeper/msg_server_stake_supplier.go index e128849f0..f8eb5f548 100644 --- a/x/supplier/keeper/msg_server_stake_supplier.go +++ b/x/supplier/keeper/msg_server_stake_supplier.go @@ -8,11 +8,19 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/pokt-network/poktroll/app/volatile" "github.com/pokt-network/poktroll/telemetry" sharedtypes "github.com/pokt-network/poktroll/x/shared/types" "github.com/pokt-network/poktroll/x/supplier/types" ) +var ( + // TODO_BETA: Make supplier staking fee a governance parameter + // TODO_BETA(@red-0ne): Update supplier staking documentation to remove the upstaking + // requirement and introduce the staking fee. + SupplierStakingFee = sdk.NewInt64Coin(volatile.DenomuPOKT, 1) +) + func (k msgServer) StakeSupplier(ctx context.Context, msg *types.MsgStakeSupplier) (*types.MsgStakeSupplierResponse, error) { isSuccessful := false defer telemetry.EventSuccessCounter( @@ -104,9 +112,13 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *types.MsgStakeSupplie supplier.UnstakeSessionEndHeight = sharedtypes.SupplierNotUnstaking } - // MUST ALWAYS stake or upstake (> 0 delta) - if coinsToEscrow.IsZero() { - err = types.ErrSupplierInvalidStake.Wrapf("Signer %q must escrow more than 0 additional coins", msg.Signer) + // TODO_BETA: Remove requirement of MUST ALWAYS stake or upstake (>= 0 delta) + // TODO_POST_MAINNET: Should we allow stake decrease down to min stake? + if coinsToEscrow.IsNegative() { + err = types.ErrSupplierInvalidStake.Wrapf( + "Supplier signer %q stake (%s) must be greater than or equal to the current stake (%s)", + msg.Signer, msg.GetStake(), supplier.Stake, + ) logger.Info(fmt.Sprintf("WARN: %s", err)) return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -130,7 +142,8 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *types.MsgStakeSupplie } // Send the coins from the message signer account to the staked supplier pool - err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, msgSignerAddress, types.ModuleName, []sdk.Coin{coinsToEscrow}) + stakeWithFee := sdk.NewCoins(coinsToEscrow.Add(SupplierStakingFee)) + err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, msgSignerAddress, types.ModuleName, stakeWithFee) if err != nil { logger.Info(fmt.Sprintf("ERROR: could not send %v coins from %q to %q module account due to %v", coinsToEscrow, msgSignerAddress, types.ModuleName, err)) return nil, status.Error(codes.InvalidArgument, err.Error()) @@ -191,8 +204,12 @@ func (k msgServer) updateSupplier( return types.ErrSupplierInvalidStake.Wrapf("stake amount cannot be nil") } - if msg.Stake.IsLTE(*supplier.Stake) { - return types.ErrSupplierInvalidStake.Wrapf("stake amount %v must be higher than previous stake amount %v", msg.Stake, supplier.Stake) + // TODO_BETA: No longer require upstaking. Remove this check. + if msg.Stake.IsLT(*supplier.Stake) { + return types.ErrSupplierInvalidStake.Wrapf( + "stake amount %v must be greater than or equal than previous stake amount %v", + msg.Stake, supplier.Stake, + ) } supplier.Stake = msg.Stake diff --git a/x/supplier/keeper/msg_server_stake_supplier_test.go b/x/supplier/keeper/msg_server_stake_supplier_test.go index 5682425fe..25bf559a2 100644 --- a/x/supplier/keeper/msg_server_stake_supplier_test.go +++ b/x/supplier/keeper/msg_server_stake_supplier_test.go @@ -45,9 +45,16 @@ func TestMsgServer_StakeSupplier_SuccessfulCreateAndUpdate(t *testing.T) { require.Equal(t, "svcId", foundSupplier.Services[0].ServiceId) require.Len(t, foundSupplier.Services[0].Endpoints, 1) require.Equal(t, "http://localhost:8080", foundSupplier.Services[0].Endpoints[0].Url) - - // Prepare an updated supplier with a higher stake and a different URL for the service - updateMsg := stakeSupplierForServicesMsg(ownerAddr, operatorAddr, 2000000, "svcId2") + // Assert that the supplier's account balance was reduced by the staking fee + balanceDecrease := keeper.SupplierStakingFee.Amount.Int64() + foundSupplier.Stake.Amount.Int64() + // SupplierBalanceMap reflects the relative changes to the supplier's balance + // (i.e. it starts from 0 and can go below it). + // It is not using coins that enforce non-negativity of the balance nor account + // funding and lookups. + require.Equal(t, -balanceDecrease, supplierModuleKeepers.SupplierBalanceMap[ownerAddr]) + + // Prepare an updated supplier with the same stake and a different URL for the service + updateMsg := stakeSupplierForServicesMsg(ownerAddr, operatorAddr, 1000000, "svcId2") updateMsg.Services[0].Endpoints[0].Url = "http://localhost:8082" // Update the staked supplier @@ -56,7 +63,7 @@ func TestMsgServer_StakeSupplier_SuccessfulCreateAndUpdate(t *testing.T) { foundSupplier, isSupplierFound = supplierModuleKeepers.GetSupplier(ctx, operatorAddr) require.True(t, isSupplierFound) - require.Equal(t, int64(2000000), foundSupplier.Stake.Amount.Int64()) + require.Equal(t, int64(1000000), foundSupplier.Stake.Amount.Int64()) require.Len(t, foundSupplier.Services, 1) require.Equal(t, "svcId2", foundSupplier.Services[0].ServiceId) require.Len(t, foundSupplier.Services[0].Endpoints, 1) diff --git a/x/supplier/keeper/msg_server_unstake_supplier_test.go b/x/supplier/keeper/msg_server_unstake_supplier_test.go index 2ccee80d2..e27493381 100644 --- a/x/supplier/keeper/msg_server_unstake_supplier_test.go +++ b/x/supplier/keeper/msg_server_unstake_supplier_test.go @@ -226,15 +226,19 @@ func TestMsgServer_UnstakeSupplier_OperatorCanUnstake(t *testing.T) { unbondingHeight := sharedtypes.GetSupplierUnbondingHeight(&sharedParams, &foundSupplier) ctx = keepertest.SetBlockHeight(ctx, int64(unbondingHeight)) + // Balance decrease is the total amount deducted from the supplier's balance, including + // the initial stake and the staking fee. + balanceDecrease := keeper.SupplierStakingFee.Amount.Int64() + foundSupplier.Stake.Amount.Int64() // Ensure that the initial stake is not returned to the owner yet - require.Equal(t, int64(0), supplierModuleKeepers.SupplierUnstakedFundsMap[ownerAddr]) + require.Equal(t, -balanceDecrease, supplierModuleKeepers.SupplierBalanceMap[ownerAddr]) // Run the endblocker to unbond suppliers err = supplierModuleKeepers.EndBlockerUnbondSuppliers(ctx) require.NoError(t, err) - // Ensure that the initial stake is returned to the owner - require.Equal(t, initialStake, supplierModuleKeepers.SupplierUnstakedFundsMap[ownerAddr]) + // Ensure that the initial stake is returned to the owner while the staking fee + // remains deducted from the supplier's balance. + require.Equal(t, -keeper.SupplierStakingFee.Amount.Int64(), supplierModuleKeepers.SupplierBalanceMap[ownerAddr]) } func createStakeMsg(supplierOwnerAddr string, stakeAmount int64) *suppliertypes.MsgStakeSupplier {