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

[Supplier] Implement supplier revenue share #729

Merged
merged 64 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 62 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
a18ba42
feat: Implement non-custodial staking
red-0ne Jul 30, 2024
676c2c4
chore: Address review change requests
red-0ne Aug 1, 2024
14ea326
Merge remote-tracking branch 'origin/main' into refactor/non-custodia…
red-0ne Aug 1, 2024
bae1f00
refactor: unit/integration tests
red-0ne Aug 2, 2024
b44e07b
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 2, 2024
5f5e98b
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 2, 2024
473e042
fix: Remove tabs and non-printable chars
red-0ne Aug 2, 2024
3d01f53
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 2, 2024
2d064fd
Merge branch 'feat/non-custodial-staking' into refactor/non-custodial…
red-0ne Aug 2, 2024
0699a35
fix: Remove shadowed variable
red-0ne Aug 2, 2024
85e6b81
Merge remote-tracking branch 'origin/main' into refactor/non-custodia…
red-0ne Aug 2, 2024
bb84ab1
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 2, 2024
498fc26
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 2, 2024
853724a
fix: All tests working
red-0ne Aug 3, 2024
6d7151e
fix: Revert merge changes
red-0ne Aug 3, 2024
20b26e7
chore: Remove imoprt newline
red-0ne Aug 3, 2024
b33a013
fix: Add missing sender address
red-0ne Aug 3, 2024
b02af2f
fix: Use higher context err variable
red-0ne Aug 3, 2024
7b3555a
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 3, 2024
604224d
fix: Enfore sender to be owner or operator
red-0ne Aug 3, 2024
540f5db
Merge branch 'feat/non-custodial-staking' into refactor/non-custodial…
red-0ne Aug 3, 2024
ac07b43
fix: Remove err shadow declaration
red-0ne Aug 3, 2024
79e4c39
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 5, 2024
473e047
fix: Err var shadowing
red-0ne Aug 6, 2024
088e675
chore: Log default operator address fallback
red-0ne Aug 6, 2024
d8e2c70
wip: rev-share features
red-0ne Aug 7, 2024
428104b
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 7, 2024
ef62442
Merge branch 'feat/non-custodial-staking' into refactor/non-custodial…
red-0ne Aug 7, 2024
365ee96
fix: Add missing ctx argument
red-0ne Aug 7, 2024
d096d02
fix: Add polylog as side effect
red-0ne Aug 7, 2024
20e8ed5
fix: Address review changes, mimic Morse behavior
red-0ne Aug 8, 2024
b543120
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 8, 2024
fdfd9b9
fix: Duplicate supplierKeeper declaration
red-0ne Aug 8, 2024
4843c21
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 8, 2024
095571a
fix: Reward the owner instead of the operator
red-0ne Aug 8, 2024
611f583
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 8, 2024
3a7c0ba
fix: Update tests
red-0ne Aug 9, 2024
e34634b
chore: Add test external signer test case
red-0ne Aug 9, 2024
910c85b
fix: Allow owner to change its aaddress
red-0ne Aug 9, 2024
e092a4e
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 9, 2024
b3ac63c
fix: Wrong word added
red-0ne Aug 9, 2024
1f26789
fix: Stake auth contidional
red-0ne Aug 9, 2024
0a43775
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 9, 2024
85d84ec
fix: Missing owner address in test
red-0ne Aug 9, 2024
7f0dcda
Merge remote-tracking branch 'origin/main' into feat/rev-share
red-0ne Aug 9, 2024
3bb9d22
test: fix all unit tests
red-0ne Aug 9, 2024
29ba54a
Merge remote-tracking branch 'origin/main' into feat/rev-share
red-0ne Aug 9, 2024
391b6db
chore: Address review change requests
red-0ne Aug 9, 2024
98b0446
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 9, 2024
7b78f49
Merge branch 'feat/non-custodial-staking' into refactor/non-custodial…
red-0ne Aug 9, 2024
e3cc89e
Merge remote-tracking branch 'origin/feat/non-custodial-staking' into…
red-0ne Aug 10, 2024
8034c26
Merge remote-tracking branch 'origin/refactor/non-custodial-staking-t…
red-0ne Aug 10, 2024
9530a64
fix: Failing unit tests
red-0ne Aug 12, 2024
a94fa5e
fix: comment typo
red-0ne Aug 12, 2024
d204355
fix: comment formatting
red-0ne Aug 12, 2024
d869db8
Merge remote-tracking branch 'origin/main' into feat/rev-share
red-0ne Aug 14, 2024
2fe7526
chore: Address review change requests
red-0ne Aug 14, 2024
3a7816f
fix: Renamed proto type
red-0ne Aug 14, 2024
8e1df74
fix: Rev share comparaison test
red-0ne Aug 14, 2024
7dd64eb
fix: Linter complaining about nil value
red-0ne Aug 14, 2024
27366fd
docs: Add rev share owner_address usage
red-0ne Aug 14, 2024
7a25e34
Merge branch 'main' into feat/rev-share
Olshansk Aug 14, 2024
f72e9bc
fix: Remove redundant check
red-0ne Aug 14, 2024
905f650
chore: Address review change requests
red-0ne Aug 14, 2024
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
733 changes: 706 additions & 27 deletions api/poktroll/shared/service.pulsar.go

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,18 @@ genesis:
- configs: []
rpc_type: JSON_RPC
url: http://relayminer1:8545
rev_share:
- address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj
rev_share_percentage: "100"
- service:
id: ollama
endpoints:
- configs: []
rpc_type: REST
url: http://relayminer1:8545
rev_share:
- address: pokt19a3t4yunp0dlpfjrp7qwnzwlrzd5fzs2gjaaaj
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.
Expand Down
80 changes: 80 additions & 0 deletions docusaurus/docs/operate/configs/supplier_staking_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ a stake transaction required to provide RPC services on Pocket Network._
- [Reference Example](#reference-example)
- [Usage](#usage)
- [Configuration](#configuration)
- [`owner_address`](#owner_address)
- [`stake_amount`](#stake_amount)
- [`default_rev_share_percent`](#default_rev_share_percent)
- [`services`](#services)
- [`service_id`](#service_id)
- [`endpoints`](#endpoints)
- [`publicly_exposed_url`](#publicly_exposed_url)
- [`rpc_type`](#rpc_type)
- [`rev_share_percent`](#rev_share_percent)

## Reference Example

Expand All @@ -43,6 +46,18 @@ poktrolld tx supplier stake-supplier \

## Configuration

### `owner_address`

_`Required`_, _`Non-empty`_

```yaml
owner_address: <address>
```

The `owner_address` is used as the unique shareholder address for the `Supplier`
if none of `default_rev_share_percent` or `rev_share_percent` is defined in the
configuration file.

### `stake_amount`

_`Required`_, _`Non-empty`_
Expand Down Expand Up @@ -72,6 +87,43 @@ sybil or flooding attacks on the network.

:::

### `default_rev_share_percent`

_`Optional`_, _`Non-empty`_

```yaml
default_rev_share_percent:
<shareholder_address>: <float>
```

`default_rev_share_percent` is an optional map that defines the default the revenue
share percentage for all the `service`s that do not have their specific `rev_share_percent`
entry defined.

This field is useful if the `Supplier` owner wants to set a default revenue share
for all the `service`s entries that do not provide one. This way, the operator
does not have repeat the same values for each `service` in the `services` section.

This map cannot be empty but can be omitted, in which case the default revenue
share falls back to `100%` of the rewards allocated to the `Supplier`'s `owner_address`.

:::note

The `shareholder_address`s MUST be valid Pocket addresses.

The revenue share values MUST be strictly positive floats with a maximum value of
100 and a total sum of 100 across all the `shareholder_address`es.

:::

:::warning

If `default_rev_share_percent` is defined, then the `owner_address` of the `Supplier`
MUST be **explicitly** defined in the map if they are to receive a share on the
`service`s that fall back to the default.

:::
red-0ne marked this conversation as resolved.
Show resolved Hide resolved

### `services`

_`Required`_, _`Non-empty`_
Expand All @@ -82,6 +134,8 @@ services:
endpoints:
- publicly_exposed_url: <protocol>://<hostname>:<port>
rpc_type: <string>
rev_share_percent:
<shareholder_address>: <float>
```

`services` define the list of services that the `Supplier` wants to provide.
Expand Down Expand Up @@ -154,3 +208,29 @@ endpoints:
:::

The `rpc_type` MUST be one of the [supported types found here](https://github.com/pokt-network/poktroll/tree/main/pkg/relayer/config/types.go#L8).

#### `rev_share_percent`

`rev_share_percent` is an optional map that defines the `service`'s specific revenue
share percentage.

It overrides the `default_rev_share_percent` if defined for the `service`.

This map cannot be empty but can be omitted, in which case it falls back to the
`default_rev_share_percent` top level configuration entry.

:::note

The `shareholder_address`s MUST be valid Pocket addresses.

The revenue share values MUST be strictly positive decimals with a maximum value
of 100 and a total sum of 100 across all the `shareholder_address`es.

:::

:::warning

If `rev_share_percent` is defined for a `service`, then the `owner_address` of the
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
`Supplier` MUST be **explicitly** defined in the map if they are to receive a share.

:::
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 14 additions & 0 deletions localnet/poktrolld/config/supplier1_stake_config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
owner_address: pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4
# NB: The stake amount is exactly 1upokt greater than the value in genesis.json
# so that the stake command causes a state change.
stake_amount: 1000069upokt
# If default_rev_share_percent is omitted, the owner receives 100% of the rewards.
# default_rev_share_percent cannot be empty - it must either be omitted completely
# or include at least one item.
default_rev_share_percent:
# The sum of all shares must equal 100%. Staking will fail otherwise.
- pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4: 80.5
- pokt1eeeksh2tvkh7wzmfrljnhw4wrhs55lcuvmekkw: 19.5
services:
# 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.
Expand All @@ -10,6 +18,12 @@ services:
- publicly_exposed_url: http://relayminer1:8545
rpc_type: JSON_RPC
- service_id: ollama
# Service specific rev share, if rev_share_percent is omitted for a specific
# service, default_rev_share_percent is used.
# The sum of all shares must equal 100%. Staking will fail otherwise.
rev_share_percent:
- pokt1mrqt5f7qh8uxs27cjm9t7v9e74a9vvdnq5jva4: 50
- pokt1eeeksh2tvkh7wzmfrljnhw4wrhs55lcuvmekkw: 50
endpoints:
- publicly_exposed_url: http://relayminer1:8545
rpc_type: REST
7 changes: 7 additions & 0 deletions proto/poktroll/shared/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ message SupplierServiceConfig {
// TODO_MAINNET: Avoid embedding the full Service because we just need the ID.
Service service = 1; // The Service for which the supplier is configured
repeated SupplierEndpoint endpoints = 2; // List of endpoints for the service
repeated ServiceRevenueShare rev_share = 3; // List of revenue share configurations for the service
// TODO_MAINNET: There is an opportunity for supplier to advertise the min
// they're willing to earn for a certain configuration/price, but this is outside of scope.
}
Expand All @@ -54,6 +55,12 @@ message SupplierEndpoint {
repeated ConfigOption configs = 3; // Additional configuration options for the endpoint
}

// ServiceRevenueShare message to hold revenue share configuration details
message ServiceRevenueShare {
string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the revenue share recipient
float rev_share_percentage = 2; // The percentage of revenue share the recipient will receive
}

// Enum to define RPC types
enum RPCType {
UNKNOWN_RPC = 0; // Undefined RPC type
Expand Down
6 changes: 6 additions & 0 deletions testutil/integration/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,12 @@ func NewCompleteIntegrationApp(t *testing.T) *App {
Stake: &supplierStake,
Services: []*sharedtypes.SupplierServiceConfig{
{
RevShare: []*sharedtypes.ServiceRevenueShare{
{
Address: sample.AccAddress(),
RevSharePercentage: 100,
},
},
Service: &defaultService,
},
},
Expand Down
16 changes: 14 additions & 2 deletions testutil/keeper/tokenomics.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,22 @@ func TokenomicsKeeperWithActorAddrs(t testing.TB) (
}

// Prepare the test supplier.
supplierOwnerAddr := sample.AccAddress()
supplier := sharedtypes.Supplier{
OwnerAddress: sample.AccAddress(),
OperatorAddress: sample.AccAddress(),
OwnerAddress: supplierOwnerAddr,
OperatorAddress: supplierOwnerAddr,
Stake: &sdk.Coin{Denom: "upokt", Amount: math.NewInt(100000)},
Services: []*sharedtypes.SupplierServiceConfig{
{
Service: service,
RevShare: []*sharedtypes.ServiceRevenueShare{
{
Address: supplierOwnerAddr,
RevSharePercentage: 100,
},
},
},
},
}

ctrl := gomock.NewController(t)
Expand Down
60 changes: 60 additions & 0 deletions x/shared/helpers/service_configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@ package helpers
import (
"fmt"

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

sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
)

const (
requiredRevSharePercentageSum = 100
)

// ValidateAppServiceConfigs returns an error if any of the application service configs are invalid
func ValidateAppServiceConfigs(services []*sharedtypes.ApplicationServiceConfig) error {
if len(services) == 0 {
Expand Down Expand Up @@ -78,6 +84,60 @@ func ValidateSupplierServiceConfigs(services []*sharedtypes.SupplierServiceConfi
// return fmt.Errorf("endpoint.Configs must have at least one entry: %v", serviceConfig)
// }
}

if err := ValidateServiceRevShare(serviceConfig.RevShare); err != nil {
return err
}
Comment on lines +88 to +90
Copy link

Choose a reason for hiding this comment

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

Improve error message clarity in ValidateSupplierServiceConfigs.

Consider providing more context in the error message returned by ValidateServiceRevShare. This can help in debugging issues related to specific service configurations.

-	return err
+	return fmt.Errorf("invalid revenue share configuration for service %v: %w", serviceConfig.Service, err)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err := ValidateServiceRevShare(serviceConfig.RevShare); err != nil {
return err
}
if err := ValidateServiceRevShare(serviceConfig.RevShare); err != nil {
return fmt.Errorf("invalid revenue share configuration for service %v: %w", serviceConfig.Service, err)
}

}

return nil
}

// ValidateServiceRevShare validates the supplier's service revenue share,
// ensuring that the sum of the revenue share percentages is 100.
// This function is unit tested via the supplier staking config tests.
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
func ValidateServiceRevShare(revShareList []*sharedtypes.ServiceRevenueShare) error {
revSharePercentageSum := float32(0)

if len(revShareList) == 0 {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("no rev share configurations")
}

for _, revShare := range revShareList {
if revShare == nil {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("rev share cannot be nil")
}

if len(revShare.Address) == 0 {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("rev share address cannot be empty")
}

// Validate the revshare address
if revShare.Address == "" {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf("rev share address cannot be empty: %v", revShare)
}

if _, err := sdk.AccAddressFromBech32(revShare.Address); err != nil {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf("invalid rev share address %s; (%v)", revShare.Address, err)
}

if revShare.RevSharePercentage <= 0 || revShare.RevSharePercentage > 100 {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf(
"invalid rev share value %v; must be between 0 and 100",
revShare.RevSharePercentage,
)
}

revSharePercentageSum += revShare.RevSharePercentage
}

if revSharePercentageSum != requiredRevSharePercentageSum {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf(
"invalid rev share percentage sum %v; must be equal to %v",
revSharePercentageSum,
requiredRevSharePercentageSum,
)
}

Copy link

Choose a reason for hiding this comment

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

Remove redundant address check in ValidateServiceRevShare.

The check for revShare.Address == "" is redundant since len(revShare.Address) == 0 covers the same condition.

-    if revShare.Address == "" {
-        return sharedtypes.ErrSharedInvalidRevShare.Wrapf("rev share address cannot be empty: %v", revShare)
-    }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func ValidateServiceRevShare(revShareList []*sharedtypes.ServiceRevenueShare) error {
revSharePercentageSum := float32(0)
if len(revShareList) == 0 {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("no rev share configurations")
}
for _, revShare := range revShareList {
if revShare == nil {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("rev share cannot be nil")
}
if len(revShare.Address) == 0 {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("rev share address cannot be empty")
}
// Validate the revshare address
if revShare.Address == "" {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf("rev share address cannot be empty: %v", revShare)
}
if _, err := sdk.AccAddressFromBech32(revShare.Address); err != nil {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf("invalid rev share address %s; (%v)", revShare.Address, err)
}
if revShare.RevSharePercentage <= 0 || revShare.RevSharePercentage > 100 {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf(
"invalid rev share value %v; must be between 0 and 100",
revShare.RevSharePercentage,
)
}
revSharePercentageSum += revShare.RevSharePercentage
}
if revSharePercentageSum != requiredRevSharePercentageSum {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf(
"invalid rev share percentage sum %v; must be equal to %v",
revSharePercentageSum,
requiredRevSharePercentageSum,
)
}
func ValidateServiceRevShare(revShareList []*sharedtypes.ServiceRevenueShare) error {
revSharePercentageSum := float32(0)
if len(revShareList) == 0 {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("no rev share configurations")
}
for _, revShare := range revShareList {
if revShare == nil {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("rev share cannot be nil")
}
if len(revShare.Address) == 0 {
return sharedtypes.ErrSharedInvalidRevShare.Wrap("rev share address cannot be empty")
}
if _, err := sdk.AccAddressFromBech32(revShare.Address); err != nil {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf("invalid rev share address %s; (%v)", revShare.Address, err)
}
if revShare.RevSharePercentage <= 0 || revShare.RevSharePercentage > 100 {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf(
"invalid rev share value %v; must be between 0 and 100",
revShare.RevSharePercentage,
)
}
revSharePercentageSum += revShare.RevSharePercentage
}
if revSharePercentageSum != requiredRevSharePercentageSum {
return sharedtypes.ErrSharedInvalidRevShare.Wrapf(
"invalid rev share percentage sum %v; must be equal to %v",
revSharePercentageSum,
requiredRevSharePercentageSum,
)
}

return nil
}
1 change: 1 addition & 0 deletions x/shared/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ var (
ErrSharedParamInvalid = sdkerrors.Register(ModuleName, 1103, "the provided param is invalid")
ErrSharedEmitEvent = sdkerrors.Register(ModuleName, 1104, "failed to emit event")
ErrSharedUnauthorizedSupplierUpdate = sdkerrors.Register(ModuleName, 1105, "unauthorized supplier update")
ErrSharedInvalidRevShare = sdkerrors.Register(ModuleName, 1106, "invalid revenue share configuration")
)
Loading
Loading