Skip to content

Commit

Permalink
[Application] Single Service Applications (#886)
Browse files Browse the repository at this point in the history
## Summary

This PR enforces staking `Application`s to have a single service,
updates the tests and add documentation about the reasoning behind this
enforcement.

Note that the `Application.ServiceConfigs` remains a slice, reducing the
codebase diff and allowing future multi-service `Applications`

## Issue

Having `MaxClaimableAmount = Application.Stake * numSessionSuppliers`
could allow `Application`s to over-service by making `Supplier`s of
different session claim from the same stake.


![image](https://github.com/user-attachments/assets/f1396ea5-5788-4185-8a3c-4c8afc3429ef)

-
#750 (comment)

## Type of change

Select one or more from the following:

- [x] New feature, functionality or library
- [x] Documentation

## Testing

- [x] **Documentation**: `make docusaurus_start`; only needed if you
make doc changes
- [x] **Unit Tests**: `make go_develop_and_test`
- [x] **LocalNet E2E Tests**: `make test_e2e`
- [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR.

## Sanity Checklist

- [x] I have tested my changes using the available tooling
- [x] I have commented my code
- [x] I have performed a self-review of my own code; both comments &
source code
- [ ] I create and reference any new tickets, if applicable
- [ ] I have left TODOs throughout the codebase, if applicable

---------

Co-authored-by: Dmitry K <[email protected]>
  • Loading branch information
red-0ne and okdas authored Oct 28, 2024
1 parent 96a9d29 commit ff99cfc
Show file tree
Hide file tree
Showing 18 changed files with 136 additions and 89 deletions.
13 changes: 8 additions & 5 deletions api/poktroll/application/types.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 12 additions & 0 deletions docusaurus/docs/operate/configs/app_staking_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ service_ids:
- <string>
```

:::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.
Expand Down
18 changes: 18 additions & 0 deletions docusaurus/docs/protocol/tokenomics/token_logic_modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 11 additions & 10 deletions e2e/tests/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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")
Expand Down
44 changes: 32 additions & 12 deletions e2e/tests/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package e2e
import (
"bytes"
"fmt"
"net/url"
"os"
"os/exec"
"strings"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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...)
}
}

Expand Down Expand Up @@ -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" {
Expand All @@ -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(),
Expand Down
7 changes: 4 additions & 3 deletions e2e/tests/relay.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
26 changes: 14 additions & 12 deletions e2e/tests/stake_app.feature
Original file line number Diff line number Diff line change
@@ -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
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
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/tx/client_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/tx/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 6 additions & 4 deletions proto/poktroll/application/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit ff99cfc

Please sign in to comment.