diff --git a/docusaurus/docs/operate/configs/supplier_staking_config.md b/docusaurus/docs/operate/configs/supplier_staking_config.md index 7e2283f3e..5f5f8b305 100644 --- a/docusaurus/docs/operate/configs/supplier_staking_config.md +++ b/docusaurus/docs/operate/configs/supplier_staking_config.md @@ -208,24 +208,6 @@ stake_amount: upokt Defines the amount of `upokt` to stake for the `Supplier` account. This amount covers all the `service`s defined in the `services` section. -:::note - -If the `Supplier` account already has a stake and wishes to change or add -to the `service`s that it provides, then it MUST increase the current -`stake_amount` by at least `1upokt`. - -For example, if the current stake is `1000upokt` and the `Supplier` wants to add -a new `service`, then `stake_amount: 1001upokt` should be specified in the -configuration file. This will increase the stake by `1upokt` and deduct `1upokt` -from the `Supplier`'s account balance. - -The upstaking requirement is to ensure that a `Supplier` incurs a cost for -changing the services they provide too frequently, which could lead to a poor user -experience for `Gateways` and `Applications`. It is also necessary to dissuade -sybil or flooding attacks on the network. - -::: - ### `default_rev_share_percent` _`Optional`_, _`Non-empty`_ diff --git a/docusaurus/docs/operate/quickstart/docker_compose_walkthrough.md b/docusaurus/docs/operate/quickstart/docker_compose_walkthrough.md index 5c88000b4..daca04d00 100644 --- a/docusaurus/docs/operate/quickstart/docker_compose_walkthrough.md +++ b/docusaurus/docs/operate/quickstart/docker_compose_walkthrough.md @@ -380,14 +380,6 @@ Use the configuration to stake your supplier: poktrolld tx supplier stake-supplier --config=/poktroll/stake_configs/supplier_stake_config_example.yaml --from=supplier-1 --chain-id=poktroll --yes ``` -:::warning Upstaking to restake - -If you need to change any of the configurations in your staking config, you MUST -increase the stake by at least one uPOKT. This is the `stake_amount` field -in the `supplier_stake_config_example.yaml` file above. - -::: - Verify your supplier is staked: ```bash diff --git a/x/application/types/message_update_param.go b/x/application/types/message_update_param.go index 251ab6a4b..33971c17e 100644 --- a/x/application/types/message_update_param.go +++ b/x/application/types/message_update_param.go @@ -57,7 +57,7 @@ func (msg *MsgUpdateParam) ValidateBasic() error { } } -// genericTypeIsCoin checks if the parameter type is T, returning an error if not. +// genericParamTypeIs checks if the parameter type is T, returning an error if not. func genericParamTypeIs[T any](msg *MsgUpdateParam) error { if _, ok := msg.AsType.(T); !ok { return ErrAppParamInvalid.Wrapf( diff --git a/x/service/types/relay.go b/x/service/types/relay.go index 2824538de..57264aafc 100644 --- a/x/service/types/relay.go +++ b/x/service/types/relay.go @@ -118,7 +118,7 @@ func (res *RelayResponse) VerifySupplierOperatorSignature(supplierOperatorPubKey return nil } -// nullifyForObservability generates an empty RelayRequest that has the same +// NullifyForObservability generates an empty RelayRequest that has the same // service and payload as the source RelayRequest if they are not nil. // It is meant to be used when replying with an error but no valid RelayRequest is available. func (sourceRelayRequest *RelayRequest) NullifyForObservability() *RelayRequest { diff --git a/x/supplier/keeper/msg_server_stake_supplier.go b/x/supplier/keeper/msg_server_stake_supplier.go index 3a591d460..6d03abdd0 100644 --- a/x/supplier/keeper/msg_server_stake_supplier.go +++ b/x/supplier/keeper/msg_server_stake_supplier.go @@ -8,8 +8,10 @@ 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" suppliertypes "github.com/pokt-network/poktroll/x/supplier/types" ) @@ -45,19 +47,20 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *suppliertypes.MsgStak // Check if the supplier already exists or not var ( err error - coinsToEscrow sdk.Coin wasSupplierUnbonding bool + supplierCurrentStake sdk.Coin ) supplier, isSupplierFound := k.GetSupplier(ctx, msg.OperatorAddress) if !isSupplierFound { + supplierCurrentStake = sdk.NewInt64Coin(volatile.DenomuPOKT, 0) logger.Info(fmt.Sprintf("Supplier not found. Creating new supplier for address %q", msg.OperatorAddress)) supplier = k.createSupplier(ctx, msg) - - coinsToEscrow = *msg.Stake } else { logger.Info(fmt.Sprintf("Supplier found. About to try updating supplier with address %q", msg.OperatorAddress)) + supplierCurrentStake = *supplier.Stake + // Ensure the signer is either the owner or the operator of the supplier. if !msg.IsSigner(supplier.OwnerAddress) && !msg.IsSigner(supplier.OperatorAddress) { return nil, status.Error( @@ -92,17 +95,10 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *suppliertypes.MsgStak return nil, status.Error(codes.InvalidArgument, err.Error()) } - currSupplierStake := *supplier.Stake if err = k.updateSupplier(ctx, &supplier, msg); err != nil { logger.Info(fmt.Sprintf("ERROR: could not update supplier for address %q due to error %v", msg.OperatorAddress, err)) return nil, status.Error(codes.InvalidArgument, err.Error()) } - coinsToEscrow, err = (*msg.Stake).SafeSub(currSupplierStake) - if err != nil { - logger.Info(fmt.Sprintf("ERROR: %s", err)) - return nil, status.Error(codes.Internal, err.Error()) - } - logger.Info(fmt.Sprintf("Supplier is going to escrow an additional %+v coins", coinsToEscrow)) // If the supplier has initiated an unstake action, cancel it since they are staking again. if supplier.UnstakeSessionEndHeight != sharedtypes.SupplierNotUnstaking { @@ -111,17 +107,6 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *suppliertypes.MsgStak } } - // TODO_BETA(@red-0ne): 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 = suppliertypes.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()) - } - // MUST ALWAYS have at least minimum stake. minStake := k.GetParams(ctx).MinStake if msg.Stake.Amount.LT(minStake.Amount) { @@ -140,15 +125,20 @@ func (k msgServer) StakeSupplier(ctx context.Context, msg *suppliertypes.MsgStak return nil, status.Error(codes.InvalidArgument, err.Error()) } - // Send the coins from the message signer account to the staked supplier pool supplierStakingFee := k.GetParams(ctx).StakingFee - stakeWithFee := sdk.NewCoins(coinsToEscrow.Add(*supplierStakingFee)) - err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, msgSignerAddress, suppliertypes.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, suppliertypes.ModuleName, err)) - return nil, status.Error(codes.InvalidArgument, err.Error()) + + if err = k.reconcileSupplierStakeDiff(ctx, msgSignerAddress, supplierCurrentStake, *msg.Stake); err != nil { + logger.Error(fmt.Sprintf("Could not transfer supplier stake difference due to %s", err)) + return nil, status.Error(codes.Internal, err.Error()) + } + + // Deduct the staking fee from the supplier's account balance. + // This is called after the stake difference is transferred to give the supplier + // the opportunity to have enough balance to pay the fee. + if err = k.bankKeeper.SendCoinsFromAccountToModule(ctx, msgSignerAddress, suppliertypes.ModuleName, sdk.NewCoins(*supplierStakingFee)); err != nil { + logger.Info(fmt.Sprintf("ERROR: signer %q could not pay for the staking fee %s: %s", msgSignerAddress, supplierStakingFee, err)) + return nil, status.Error(codes.FailedPrecondition, err.Error()) } - logger.Info(fmt.Sprintf("Successfully escrowed %v coins from %q to %q module account", coinsToEscrow, msgSignerAddress, suppliertypes.ModuleName)) // Update the Supplier in the store k.SetSupplier(ctx, supplier) @@ -220,13 +210,6 @@ func (k msgServer) updateSupplier( return suppliertypes.ErrSupplierInvalidStake.Wrapf("stake amount cannot be nil") } - // TODO_BETA: No longer require upstaking. Remove this check. - if msg.Stake.IsLT(*supplier.Stake) { - return suppliertypes.ErrSupplierInvalidStake.Wrapf( - "stake amount %v must be greater than or equal than previous stake amount %v", - msg.Stake, supplier.Stake, - ) - } supplier.Stake = msg.Stake supplier.OwnerAddress = msg.OwnerAddress @@ -267,3 +250,47 @@ func (k msgServer) updateSupplier( return nil } + +// reconcileSupplierStakeDiff transfers the difference between the current and new stake +// amounts by either escrowing, when the stake is increased, or unescrowing otherwise. +func (k msgServer) reconcileSupplierStakeDiff( + ctx context.Context, + signerAddr sdk.AccAddress, + currentStake sdk.Coin, + newStake sdk.Coin, +) error { + logger := k.Logger().With("method", "reconcileSupplierStakeDiff") + + // The Supplier is increasing its stake, so escrow the difference + if currentStake.Amount.LT(newStake.Amount) { + coinsToEscrow := sdk.NewCoins(newStake.Sub(currentStake)) + + // Send the coins from the message signer account to the staked supplier pool + return k.bankKeeper.SendCoinsFromAccountToModule(ctx, signerAddr, suppliertypes.ModuleName, coinsToEscrow) + } + + // Ensure that the new stake is at least the minimum stake which is required for: + // 1. The supplier to be considered active. + // 2. Cover for any potential slashing that may occur during claims settlement. + minStake := k.GetParams(ctx).MinStake + if newStake.Amount.LT(minStake.Amount) { + err := suppliertypes.ErrSupplierInvalidStake.Wrapf( + "supplier with owner %q must stake at least %s", + signerAddr, minStake, + ) + return err + } + + // The supplier is decreasing its stake, unescrow the difference. + if currentStake.Amount.GT(newStake.Amount) { + coinsToUnescrow := sdk.NewCoins(currentStake.Sub(newStake)) + + // Send the coins from the staked supplier pool to the message signer account + return k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, signerAddr, coinsToUnescrow) + } + + // The supplier is not changing its stake. This can happen if the supplier + // is updating its service configurations or owner address but not the stake. + logger.Info(fmt.Sprintf("Updating supplier with address %q but stake is unchanged", signerAddr.String())) + return nil +} diff --git a/x/supplier/keeper/msg_server_stake_supplier_test.go b/x/supplier/keeper/msg_server_stake_supplier_test.go index 9a8216697..da628fd82 100644 --- a/x/supplier/keeper/msg_server_stake_supplier_test.go +++ b/x/supplier/keeper/msg_server_stake_supplier_test.go @@ -135,16 +135,18 @@ func TestMsgServer_StakeSupplier_FailRestakingDueToInvalidServices(t *testing.T) require.Equal(t, "http://localhost:8080", supplierFound.Services[0].Endpoints[0].Url) } -func TestMsgServer_StakeSupplier_FailLoweringStake(t *testing.T) { +func TestMsgServer_StakeSupplier_FailLoweringStakeBelowMinStake(t *testing.T) { supplierModuleKeepers, ctx := keepertest.SupplierKeeper(t) srv := keeper.NewMsgServerImpl(*supplierModuleKeepers.Keeper) + minStake := supplierModuleKeepers.Keeper.GetParams(ctx).MinStake.Amount.Int64() + // Generate an owner and operator address for the supplier ownerAddr := sample.AccAddress() operatorAddr := sample.AccAddress() // Prepare the supplier stake message - stakeMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, 1000000, "svcId") + stakeMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, minStake, "svcId") // Stake the supplier & verify that the supplier exists _, err := srv.StakeSupplier(ctx, stakeMsg) @@ -154,7 +156,7 @@ func TestMsgServer_StakeSupplier_FailLoweringStake(t *testing.T) { require.True(t, isSupplierFound) // Prepare an update supplier msg with a lower stake - updateMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, 50, "svcId") + updateMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, minStake-1, "svcId") updateMsg.Signer = operatorAddr // Verify that it fails @@ -168,6 +170,78 @@ func TestMsgServer_StakeSupplier_FailLoweringStake(t *testing.T) { require.Len(t, supplierFound.Services, 1) } +func TestMsgServer_StakeSupplier_SuccessLoweringStakeAboveMinStake(t *testing.T) { + supplierModuleKeepers, ctx := keepertest.SupplierKeeper(t) + srv := keeper.NewMsgServerImpl(*supplierModuleKeepers.Keeper) + + minStake := supplierModuleKeepers.Keeper.GetParams(ctx).MinStake.Amount.Int64() + + // Generate an owner and operator address for the supplier + ownerAddr := sample.AccAddress() + operatorAddr := sample.AccAddress() + + // Prepare the supplier stake message + stakeMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, minStake, "svcId") + + // Stake the supplier & verify that the supplier exists + _, err := srv.StakeSupplier(ctx, stakeMsg) + require.NoError(t, err) + + _, isSupplierFound := supplierModuleKeepers.GetSupplier(ctx, operatorAddr) + require.True(t, isSupplierFound) + + // Prepare an updated supplier msg with a lower stake which is below the minimum staking fee. + newStake := minStake - 1 + updateMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, newStake, "svcId") + updateMsg.Signer = operatorAddr + + // Verify that it fails + _, err = srv.StakeSupplier(ctx, updateMsg) + require.Error(t, err) + + // Verify that the supplier stake is unchanged + supplierFound, isSupplierFound := supplierModuleKeepers.GetSupplier(ctx, operatorAddr) + require.True(t, isSupplierFound) + require.Equal(t, minStake, supplierFound.Stake.Amount.Int64()) + require.Len(t, supplierFound.Services, 1) +} + +func TestMsgServer_StakeSupplier_SuccessIncreasingStake(t *testing.T) { + supplierModuleKeepers, ctx := keepertest.SupplierKeeper(t) + srv := keeper.NewMsgServerImpl(*supplierModuleKeepers.Keeper) + + minStake := supplierModuleKeepers.Keeper.GetParams(ctx).MinStake.Amount.Int64() + + // Generate an owner and operator address for the supplier + ownerAddr := sample.AccAddress() + operatorAddr := sample.AccAddress() + + // Prepare the supplier stake message + stakeMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, minStake, "svcId") + + // Stake the supplier & verify that the supplier exists + _, err := srv.StakeSupplier(ctx, stakeMsg) + require.NoError(t, err) + + _, isSupplierFound := supplierModuleKeepers.GetSupplier(ctx, operatorAddr) + require.True(t, isSupplierFound) + + // Prepare an update supplier msg with a higher stake. + newStake := minStake + 1 + updateMsg, _ := newSupplierStakeMsg(ownerAddr, operatorAddr, newStake, "svcId") + updateMsg.Signer = operatorAddr + + // Verify that succeeds + _, err = srv.StakeSupplier(ctx, updateMsg) + require.NoError(t, err) + + // Verify that the supplier stake is unchanged + supplierFound, isSupplierFound := supplierModuleKeepers.GetSupplier(ctx, operatorAddr) + require.True(t, isSupplierFound) + require.Equal(t, newStake, supplierFound.Stake.Amount.Int64()) + require.Len(t, supplierFound.Services, 1) +} + func TestMsgServer_StakeSupplier_FailWithNonExistingService(t *testing.T) { supplierModuleKeepers, ctx := keepertest.SupplierKeeper(t) srv := keeper.NewMsgServerImpl(*supplierModuleKeepers.Keeper) diff --git a/x/tokenomics/keeper/keeper_settle_pending_claims_test.go b/x/tokenomics/keeper/keeper_settle_pending_claims_test.go index f40d66890..076663cce 100644 --- a/x/tokenomics/keeper/keeper_settle_pending_claims_test.go +++ b/x/tokenomics/keeper/keeper_settle_pending_claims_test.go @@ -204,7 +204,7 @@ func (s *TestSuite) SetupTest() { s.proof = *testtree.NewProof(t, supplierOwnerAddr, sessionHeader, sessionTree, expectedMerkleProofPath) } -// TestSettleExpiringClaimsSuite tests the claim settlement process. +// TestSettlePendingClaims tests the claim settlement process. // NB: Each test scenario (method) is run in isolation and #TestSetup() is called // for each prior to running. func TestSettlePendingClaims(t *testing.T) {