[Supplier] Allow Supplier to downstake (#946)

## Summary

Allow `Supplier`s to lower their stake down to `MinStake`.

## Issue

- TODO_BETA

## Type of change

Select one or more from the following:

- [x] New feature, functionality or library
- [ ] Consensus breaking; add the `consensus-breaking` label if so. See
#791 for details
- [ ] Bug fix
- [ ] Code health or cleanup
- [x] Documentation
- [ ] Other (specify)

## Testing


- [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: Daniel Olshansky <[email protected]>

docusaurus/docs/operate/configs/supplier_staking_config.md

@@ -208,24 +208,6 @@ stake_amount: <number>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`_

docusaurus/docs/operate/quickstart/docker_compose_walkthrough.md

@@ -381,14 +381,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

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
+}

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)