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 non-custodial staking #716

Merged
merged 21 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
21 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
b44e07b
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 2, 2024
3d01f53
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 2, 2024
bb84ab1
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 2, 2024
b33a013
fix: Add missing sender address
red-0ne Aug 3, 2024
604224d
fix: Enfore sender to be owner or operator
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
428104b
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
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
095571a
fix: Reward the owner instead of the operator
red-0ne Aug 8, 2024
910c85b
fix: Allow owner to change its aaddress
red-0ne Aug 9, 2024
1f26789
fix: Stake auth contidional
red-0ne Aug 9, 2024
98b0446
Merge remote-tracking branch 'origin/main' into feat/non-custodial-st…
red-0ne Aug 9, 2024
8060f46
chore: Address review change requests
red-0ne Aug 13, 2024
b887a0c
fix: Error sequence
red-0ne Aug 13, 2024
8121719
chore: Remove unused method
red-0ne Aug 13, 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
259 changes: 172 additions & 87 deletions api/poktroll/shared/supplier.pulsar.go

Large diffs are not rendered by default.

400 changes: 318 additions & 82 deletions api/poktroll/supplier/tx.pulsar.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions load-testing/tests/relays_stress_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,7 @@ func (s *relaysSuite) addActor(actorAddress string, actorStakeAmount sdk.Coin) *
// messages in a single supplier transaction.
func (s *relaysSuite) addPendingStakeSupplierMsg(supplier *accountInfo) {
supplier.addPendingMsg(suppliertypes.NewMsgStakeSupplier(
supplier.address,
supplier.address,
supplier.amountToStake,
[]*sharedtypes.SupplierServiceConfig{
Expand Down
21 changes: 15 additions & 6 deletions proto/poktroll/shared/supplier.proto
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,24 @@ import "poktroll/shared/service.proto";

// Supplier is the type defining the actor in Pocket Network that provides RPC services.
message Supplier {
string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the supplier using cosmos' ScalarDescriptor to ensure deterministic encoding
cosmos.base.v1beta1.Coin stake = 2; // The total amount of uPOKT the supplier has staked
repeated SupplierServiceConfig services = 3; // The service configs this supplier can support
// The address of the owner (i.e. staker, custodial) that owns the funds for staking.
// By default, this address is the one that receives all the rewards unless owtherwise specified.
// The owner can initially stake the supplier and unstake it.
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
// All other configurations are managed by the operator.
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
string owner_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // Bech32 cosmos address
// The operator address of the supplier that operates it.
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
// The operator address can update the supplier's configurations excluding the owner
// and operator addresses which do not change over the supplier's lifespan.
Copy link
Member

Choose a reason for hiding this comment

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

and operator addresses which do not change over the supplier's lifespan.

This is incorrect.

The operator can:

  • Update operator address

The owner can:

  • Update operator address
  • Update owner address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added this restriction to simplify the Supplier management.

Given that:

  1. Changing the operator address mid-session would have the same effect as changing other service configs, which raises the need to delay the update til the next session.

  2. The OperatorAddress is the Supplier identifier, if the owner is allowed to change it, it would need to specify the old OperatorAddress so we would know which supplier to update when calling GetSupplier (we actually have to delete then set the Supplier entry in the Supplier KVStore). Adding an old_operator_address to the staking config file is far from ideal ux wise.

Having the owner being able to update the OwnerAddress is fine though.

Copy link
Member

Choose a reason for hiding this comment

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

Having the owner being able to update the OwnerAddress is fine though.

Let's make this clearer.

Copy link
Member

Choose a reason for hiding this comment

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

Changing the operator address mid-session would have the same effect as changing other service configs, which raises the need to delay the update til the next session.

Add one line making it clearer why operator address is immutable.

Copy link
Member

Choose a reason for hiding this comment

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

Per this comment: #716 (comment)

Can you please evaluate if there is any difference with how it works in Morse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking directly into the code, these are the current differences.

Morse Shannon
Id Operator address Operator address
Initial stake Owner and Operator Owner only
Update Owner and operator Operator only
Update owner Owner None, must unstake->stake
Update operator Owner and Operator (see Note) None, must unstake->stake
Unstake Owner and Operator Owner only (can easily upgrade to both)

Note that from reading the code, ther seems to be no code path that can update the operator address, since the message would contain a new validator address which would not be found when doing k.GetValidator(ctx, validator.Address). I may be missing something though

// TODO(red-0ne): Rename this to `operator_address` include all downstream
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
// variables, comments, docs, tests, etc...
string address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // Bech32 cosmos address
cosmos.base.v1beta1.Coin stake = 3; // The total amount of uPOKT the supplier has staked
repeated SupplierServiceConfig services = 4; // The service configs this supplier can support
// The session end height at which an actively unbonding supplier unbonds its stake.
// If the supplier did not unstake, this value will be 0.
uint64 unstake_session_end_height = 4;
uint64 unstake_session_end_height = 5;
// services_activation_heights_map is a map of serviceIds to the height at
// which the staked supplier will become active for that service.
// Activation heights are session start heights.
map<string, uint64> services_activation_heights_map = 5;
map<string, uint64> services_activation_heights_map = 6;
}

25 changes: 19 additions & 6 deletions proto/poktroll/supplier/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,30 @@ message MsgUpdateParams {
message MsgUpdateParamsResponse {}

message MsgStakeSupplier {
option (cosmos.msg.v1.signer) = "address"; // https://docs.cosmos.network/main/build/building-modules/messages-and-queries
string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the supplier using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding
cosmos.base.v1beta1.Coin stake = 2; // The total amount of uPOKT the supplier has staked. Must be ≥ to the current amount that the supplier has staked (if any)
repeated poktroll.shared.SupplierServiceConfig services = 3; // The list of services this supplier is staked to provide service for
option (cosmos.msg.v1.signer) = "sender"; // https://docs.cosmos.network/main/build/building-modules/messages-and-queries

string sender = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the message sender (i.e. owner or operator) using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
string owner_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the owner (i.e. custodial, staker) using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding
string address = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the operator (i.e. provider, non-custodial) using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
cosmos.base.v1beta1.Coin stake = 4; // The total amount of uPOKT the supplier has staked. Must be ≥ to the current amount that the supplier has staked (if any)
repeated poktroll.shared.SupplierServiceConfig services = 5; // The list of services this supplier is staked to provide service for
}

message MsgStakeSupplierResponse {}

message MsgUnstakeSupplier {
option (cosmos.msg.v1.signer) = "address";
string address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address of the supplier using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding using cosmos' ScalarDescriptor to ensure deterministic deterministic encoding
option (cosmos.msg.v1.signer) = "owner_address";
// The address of the owner (i.e. staker, custodial) that owns the funds for staking.
// By default, this address is the one that receives all the rewards unless owtherwise specified.
red-0ne marked this conversation as resolved.
Show resolved Hide resolved
// The owner can initially stake the supplier and unstake it.
// All other configurations are managed by the operator.
string owner_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address
// The operator address of the supplier that operates it.
// The operator address can update the supplier's configurations excluding the owner
// and operator addresses which do not change over the supplier's lifespan.
// TODO(red-0ne): Rename this to `operator_address` include all downstream
// variables, comments, docs, tests, etc...
string address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // The Bech32 address
}

message MsgUnstakeSupplierResponse {}
Expand Down
2 changes: 2 additions & 0 deletions testutil/integration/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,14 @@ func NewCompleteIntegrationApp(t *testing.T) *App {
sharedKeeper,
sessionKeeper,
serviceKeeper,
supplierKeeper,
)
tokenomicsModule := tokenomics.NewAppModule(
cdc,
tokenomicsKeeper,
accountKeeper,
bankKeeper,
supplierKeeper,
)

// Prepare the message & query routers
Expand Down
6 changes: 6 additions & 0 deletions testutil/keeper/tokenomics.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ func TokenomicsKeeperWithActorAddrs(
mockSharedKeeper := mocks.NewMockSharedKeeper(ctrl)
mockSharedKeeper.EXPECT().GetProofWindowCloseHeight(gomock.Any(), gomock.Any()).AnyTimes()

// Mock the supplier keeper
mockSupplierKeeper := mocks.NewMockSupplierKeeper(ctrl)
mockSupplierKeeper.EXPECT().GetSupplier(gomock.Any(), gomock.Any()).AnyTimes()

// Mock the session keeper
mockSessionKeeper := mocks.NewMockSessionKeeper(ctrl)

Expand Down Expand Up @@ -198,6 +202,7 @@ func TokenomicsKeeperWithActorAddrs(
mockSharedKeeper,
mockSessionKeeper,
mockServiceKeeper,
mockSupplierKeeper,
)

sdkCtx := sdk.NewContext(stateStore, cmtproto.Header{}, false, log.NewNopLogger())
Expand Down Expand Up @@ -380,6 +385,7 @@ func NewTokenomicsModuleKeepers(
sharedKeeper,
sessionKeeper,
serviceKeeper,
supplierKeeper,
)

require.NoError(t, tokenomicsKeeper.SetParams(ctx, tokenomicstypes.DefaultParams()))
Expand Down
11 changes: 6 additions & 5 deletions x/shared/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (

// x/shared module sentinel errors
var (
ErrSharedInvalidSigner = sdkerrors.Register(ModuleName, 1100, "expected gov account as only signer for proposal message")
ErrSharedInvalidAddress = sdkerrors.Register(ModuleName, 1101, "invalid address")
ErrSharedParamNameInvalid = sdkerrors.Register(ModuleName, 1102, "the provided param name is invalid")
ErrSharedParamInvalid = sdkerrors.Register(ModuleName, 1103, "the provided param is invalid")
ErrSharedEmitEvent = sdkerrors.Register(ModuleName, 1104, "failed to emit event")
ErrSharedInvalidSigner = sdkerrors.Register(ModuleName, 1100, "expected gov account as only signer for proposal message")
ErrSharedInvalidAddress = sdkerrors.Register(ModuleName, 1101, "invalid address")
ErrSharedParamNameInvalid = sdkerrors.Register(ModuleName, 1102, "the provided param name is invalid")
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")
)
26 changes: 26 additions & 0 deletions x/shared/types/supplier.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,29 @@ func (s *Supplier) IsActive(queryHeight uint64, serviceId string) bool {

return true
}

// EnsureOwner returns an error if the given address does not match supplier's owner address.
func (s *Supplier) EnsureOwner(ownerAddress string) error {
if s.OwnerAddress != ownerAddress {
return ErrSharedUnauthorizedSupplierUpdate.Wrapf(
"msg.OwnerAddress %q != provided address %q",
s.OwnerAddress,
ownerAddress,
)
}

return nil
}

// EnsureOperator returns an error if the given address does not match supplier's operator address.
func (s *Supplier) EnsureOperator(operatorAddress string) error {
if s.Address != operatorAddress {
return ErrSharedUnauthorizedSupplierUpdate.Wrapf(
"msg.OperatorAddress %q != provided address %q",
s.OwnerAddress,
operatorAddress,
)
}

return nil
}
Loading
Loading