[Utility] Update SessionSettlement when app goes into a negative amou…

…nt (#509)

When an app exceeds its stake, make the settlement amount its full balance.

Prevent an error to avoid consensus breaking issues.

## Issue

It seems that a segfault occurs when we restart due to the -ve balance an app tries to hit.

Started with this:

<img width="762" alt="Screenshot 2024-04-27 at 6 28 01 PM" src="https://github.com/pokt-network/poktroll/assets/1892194/6231033b-f90e-43bc-aeb7-24285ba8a10f">

And then we found this:

<img width="1063" alt="Screenshot 2024-04-27 at 5 37 33 PM" src="https://github.com/pokt-network/poktroll/assets/1892194/df55a2f0-78cd-43f2-98d5-07bd0d1fca0f">

<img width="1234" alt="Screenshot 2024-04-27 at 5 37 26 PM" src="https://github.com/pokt-network/poktroll/assets/1892194/b255a28c-f357-4e95-8bb1-0dad10f3951a">

Which led us to discovering this:

<img width="1840" alt="Screenshot 2024-04-27 at 6 13 50 PM" src="https://github.com/pokt-network/poktroll/assets/1892194/55bbfb19-ca2a-49f2-bca8-0c008b6d9b85">

x/tokenomics/keeper/settle_pending_claims.go

@@ -57,11 +57,11 @@ func (k Keeper) SettlePendingClaims(ctx sdk.Context) (numClaimsSettled, numClaim
 
  sessionId := claim.SessionHeader.SessionId
 
+ _, isProofFound := k.proofKeeper.GetProof(ctx, sessionId, claim.SupplierAddress)
  // Using the probabilistic proofs approach, determine if this expiring
  // claim required an on-chain proof
  isProofRequiredForClaim := k.isProofRequiredForClaim(ctx, &claim)
  if isProofRequiredForClaim {
- _, isProofFound := k.proofKeeper.GetProof(ctx, sessionId, claim.SupplierAddress)
  // If a proof is not found, the claim will expire and never be settled.
  if !isProofFound {
  // Emit an event that a claim has expired and being removed without being settled.
@@ -101,9 +101,13 @@ func (k Keeper) SettlePendingClaims(ctx sdk.Context) (numClaimsSettled, numClaim
  // The claim & proof are no longer necessary, so there's no need for them
  // to take up on-chain space.
  k.proofKeeper.RemoveClaim(ctx, sessionId, claim.SupplierAddress)
- // NB: We are calling `RemoveProof` of whether or not the proof was required
- // to delete it from the state. It is okay for it to fail here if it doesn't exist.
- k.proofKeeper.RemoveProof(ctx, sessionId, claim.SupplierAddress)
+ // Whether or not the proof is required, the supplier may have submitted one
+ // so we need to delete it either way. If we don't have the if structure,
+ // a safe error will be printed, but it can be confusing to the operator
+ // or developer.
+ if isProofFound {
+ k.proofKeeper.RemoveProof(ctx, sessionId, claim.SupplierAddress)
+ }
 
  numClaimsSettled++
  logger.Info(fmt.Sprintf("Successfully settled claim for session ID %q at block height %d", claim.SessionHeader.SessionId, blockHeight))

x/tokenomics/keeper/settle_session_accounting.go

@@ -147,8 +147,8 @@ func (k Keeper) SettleSessionAccounting(
 
  // Verify that the application has enough uPOKT to pay for the services it consumed
  if application.Stake.IsLT(settlementAmt) {
- logger.Error(fmt.Sprintf(
- "THIS SHOULD NOT HAPPEN. Application with address %s needs to be charged more than it has staked: %v > %v",
+ logger.Warn(fmt.Sprintf(
+ "THIS SHOULD NEVER HAPPEN. Application with address %s needs to be charged more than it has staked: %v > %v",
  applicationAddress,
  settlementAmtuPOKT,
  application.Stake,
@@ -157,7 +157,9 @@ func (k Keeper) SettleSessionAccounting(
  // goes "into debt". Need to design a way to handle this when we implement
  // probabilistic proofs and add all the parameter logic. Do we touch the application balance?
  // Do we just let it go into debt? Do we penalize the application? Do we unstake it? Etc...
- settlementAmtuPOKT = sdk.NewCoins(*application.Stake)
+ settlementAmt = sdk.NewCoin("upokt", math.Int(application.Stake.Amount))
+ settlementAmtuPOKT = sdk.NewCoins(settlementAmt)
+ // TODO_BLOCKER: The application should be immediately unstaked at this point in time
  }
 
  // Burn uPOKT from the application module account which was held in escrow
@@ -172,7 +174,7 @@ func (k Keeper) SettleSessionAccounting(
  // Update the application's on-chain stake
  newAppStake, err := (*application.Stake).SafeSub(settlementAmt)
  if err != nil {
- return types.ErrTokenomicsApplicationNewStakeInvalid.Wrapf("application %q stake cannot be reduce to a negative amount %v", applicationAddress, newAppStake)
+ return types.ErrTokenomicsApplicationNewStakeInvalid.Wrapf("application %q stake cannot be reduced to a negative amount %v", applicationAddress, newAppStake)
  }
  application.Stake = &newAppStake
  k.applicationKeeper.SetApplication(ctx, application)

x/tokenomics/keeper/settle_session_accounting_test.go

@@ -5,21 +5,63 @@ import (
  "fmt"
  "testing"
 
+ "cosmossdk.io/math"
+ "github.com/cosmos/cosmos-sdk/types"
  "github.com/pokt-network/smt"
  "github.com/stretchr/testify/require"
 
  testkeeper "github.com/pokt-network/poktroll/testutil/keeper"
  "github.com/pokt-network/poktroll/testutil/sample"
+ apptypes "github.com/pokt-network/poktroll/x/application/types"
  prooftypes "github.com/pokt-network/poktroll/x/proof/types"
  sessionkeeper "github.com/pokt-network/poktroll/x/session/keeper"
  sessiontypes "github.com/pokt-network/poktroll/x/session/types"
  sharedtypes "github.com/pokt-network/poktroll/x/shared/types"
- "github.com/pokt-network/poktroll/x/tokenomics/types"
+ tokenomicstypes "github.com/pokt-network/poktroll/x/tokenomics/types"
 )
 
 // TODO_TEST(@bryanchriswhite, @Olshansk): Improve tokenomics tests (i.e. checking balances)
 // once in-memory network integration tests are supported.
 
+func TestSettleSessionAccounting_HandleAppGoingIntoDebt(t *testing.T) {
+ keepers, ctx := testkeeper.NewTokenomicsModuleKeepers(t)
+
+ // Add a new application
+ appStake := types.NewCoin("upokt", math.NewInt(1000000))
+ app := apptypes.Application{
+ Address: sample.AccAddress(),
+ Stake: &appStake,
+ }
+ keepers.SetApplication(ctx, app)
+
+ // Add a new supplier
+ supplierStake := types.NewCoin("upokt", math.NewInt(1000000))
+ supplier := sharedtypes.Supplier{
+ Address: sample.AccAddress(),
+ Stake: &supplierStake,
+ }
+
+ // The base claim whose root will be customized for testing purposes
+ claim := prooftypes.Claim{
+ SupplierAddress: supplier.Address,
+ SessionHeader: &sessiontypes.SessionHeader{
+ ApplicationAddress: app.Address,
+ Service: &sharedtypes.Service{
+ Id: "svc1",
+ Name: "svcName1",
+ },
+ SessionId: "session_id",
+ SessionStartBlockHeight: 1,
+ SessionEndBlockHeight: sessionkeeper.GetSessionEndBlockHeight(1),
+ },
+ RootHash: smstRootWithSum(appStake.Amount.Uint64() + 1), // More than the app stake
+ }
+
+ err := keepers.SettleSessionAccounting(ctx, &claim)
+ require.NoError(t, err)
+ // TODO_BLOCKER: Need to make sure the application is unstaked at this point in time.
+}
+
 func TestSettleSessionAccounting_ValidAccounting(t *testing.T) {
  t.Skip("TODO_BLOCKER(@Olshansk): Add E2E and integration tests so we validate the actual state changes of the bank & account keepers.")
  // Assert that `suppliertypes.ModuleName` account module balance is *unchanged*
@@ -58,7 +100,7 @@ func TestSettleSessionAccounting_AppNotFound(t *testing.T) {
 
  err := keeper.SettleSessionAccounting(ctx, &claim)
  require.Error(t, err)
- require.ErrorIs(t, err, types.ErrTokenomicsApplicationNotFound)
+ require.ErrorIs(t, err, tokenomicstypes.ErrTokenomicsApplicationNotFound)
 }
 
 func TestSettleSessionAccounting_InvalidRoot(t *testing.T) {
@@ -169,7 +211,7 @@ func TestSettleSessionAccounting_InvalidClaim(t *testing.T) {
  desc: "Nil Claim",
  claim: nil,
  errExpected: true,
- expectErr: types.ErrTokenomicsClaimNil,
+ expectErr: tokenomicstypes.ErrTokenomicsClaimNil,
  },
  {
  desc: "Claim with nil session header",
@@ -179,7 +221,7 @@ func TestSettleSessionAccounting_InvalidClaim(t *testing.T) {
  return &claim
  }(),
  errExpected: true,
- expectErr: types.ErrTokenomicsSessionHeaderNil,
+ expectErr: tokenomicstypes.ErrTokenomicsSessionHeaderNil,
  },
  {
  desc: "Claim with invalid session id",
@@ -189,7 +231,7 @@ func TestSettleSessionAccounting_InvalidClaim(t *testing.T) {
  return &claim
  }(),
  errExpected: true,
- expectErr: types.ErrTokenomicsSessionHeaderInvalid,
+ expectErr: tokenomicstypes.ErrTokenomicsSessionHeaderInvalid,
  },
  {
  desc: "Claim with invalid application address",
@@ -199,7 +241,7 @@ func TestSettleSessionAccounting_InvalidClaim(t *testing.T) {
  return &claim
  }(),
  errExpected: true,
- expectErr: types.ErrTokenomicsSessionHeaderInvalid,
+ expectErr: tokenomicstypes.ErrTokenomicsSessionHeaderInvalid,
  },
  {
  desc: "Claim with invalid supplier address",
@@ -209,7 +251,7 @@ func TestSettleSessionAccounting_InvalidClaim(t *testing.T) {
  return &claim
  }(),
  errExpected: true,
- expectErr: types.ErrTokenomicsSupplierAddressInvalid,
+ expectErr: tokenomicstypes.ErrTokenomicsSupplierAddressInvalid,
  },
  }
 

x/tokenomics/types/errors.go

@@ -19,5 +19,5 @@ var (
  ErrTokenomicsApplicationAddressInvalid = sdkerrors.Register(ModuleName, 1112, "the application address in the claim is not a valid bech32 address")
  ErrTokenomicsParamsInvalid = sdkerrors.Register(ModuleName, 1113, "provided params are invalid")
  ErrTokenomicsRootHashInvalid = sdkerrors.Register(ModuleName, 1114, "the root hash in the claim is invalid")
- ErrTokenomicsApplicationNewStakeInvalid = sdkerrors.Register(ModuleName, 1115, "application stake cannot be reduce to a -ve amount")
+ ErrTokenomicsApplicationNewStakeInvalid = sdkerrors.Register(ModuleName, 1115, "application stake cannot be reduced to a -ve amount")
 )