do not unregister if out of rent (#743)

* do not unregister if out of rent

* add test & min processable rent

proto/dex/params.proto

@@ -43,4 +43,8 @@ message Params {
     (gogoproto.jsontag)   = "gas_allowance_per_settlement",
     (gogoproto.moretags) = "yaml:\"gas_allowance_per_settlement\""
   ];
+  uint64 min_processable_rent = 9 [
+    (gogoproto.jsontag)   = "min_processable_rent",
+    (gogoproto.moretags) = "yaml:\"min_processable_rent\""
+  ];
 }

x/dex/contract/abci.go

@@ -2,6 +2,7 @@ package contract
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sync"
 	"time"
@@ -34,6 +35,7 @@ const LogExecSigSendAfter = 2 * time.Second
 type environment struct {
 	validContractsInfo          []types.ContractInfoV2
 	failedContractAddresses     datastructures.SyncSet[string]
+	outOfRentContractAddresses  datastructures.SyncSet[string]
 	settlementsByContract       *datastructures.TypedSyncMap[string, []*types.SettlementEntry]
 	executionTerminationSignals *datastructures.TypedSyncMap[string, chan struct{}]
 	registeredPairs             *datastructures.TypedSyncMap[string, []types.Pair]
@@ -43,7 +45,7 @@ type environment struct {
 	eventManagerMutex *sync.Mutex
 }
 
-func EndBlockerAtomic(ctx sdk.Context, keeper *keeper.Keeper, validContractsInfo []types.ContractInfoV2, tracingInfo *tracing.Info) ([]types.ContractInfoV2, sdk.Context, bool) {
+func EndBlockerAtomic(ctx sdk.Context, keeper *keeper.Keeper, validContractsInfo []types.ContractInfoV2, tracingInfo *tracing.Info) ([]types.ContractInfoV2, []types.ContractInfoV2, sdk.Context, bool) {
 	tracer := tracingInfo.Tracer
 	spanCtx, span := tracingInfo.Start("DexEndBlockerAtomic")
 	defer span.End()
@@ -78,7 +80,7 @@ func EndBlockerAtomic(ctx sdk.Context, keeper *keeper.Keeper, validContractsInfo
 		postRunRents := keeper.GetRentsForContracts(cachedCtx, seiutils.Map(validContractsInfo, func(c types.ContractInfoV2) string { return c.ContractAddr }))
 		TransferRentFromDexToCollector(ctx, keeper.BankKeeper, preRunRents, postRunRents)
 		msCached.Write()
-		return env.validContractsInfo, ctx, true
+		return env.validContractsInfo, []types.ContractInfoV2{}, ctx, true
 	}
 
 	// persistent contract rent charges for failed contracts and discard everything else
@@ -103,7 +105,7 @@ func EndBlockerAtomic(ctx sdk.Context, keeper *keeper.Keeper, validContractsInfo
 
 	// restore keeper in-memory state
 	newGoContext := context.WithValue(ctx.Context(), dexutils.DexMemStateContextKey, memStateCopy)
-	return filterNewValidContracts(ctx, env), ctx.WithContext(newGoContext), false
+	return filterNewValidContracts(ctx, env), getOutOfRentContracts(env), ctx.WithContext(newGoContext), false
 }
 
 func newEnv(ctx sdk.Context, validContractsInfo []types.ContractInfoV2, keeper *keeper.Keeper) *environment {
@@ -123,6 +125,7 @@ func newEnv(ctx sdk.Context, validContractsInfo []types.ContractInfoV2, keeper *
 	return &environment{
 		validContractsInfo:          validContractsInfo,
 		failedContractAddresses:     datastructures.NewSyncSet([]string{}),
+		outOfRentContractAddresses:  datastructures.NewSyncSet([]string{}),
 		settlementsByContract:       settlementsByContract,
 		executionTerminationSignals: executionTerminationSignals,
 		registeredPairs:             registeredPairs,
@@ -132,6 +135,14 @@ func newEnv(ctx sdk.Context, validContractsInfo []types.ContractInfoV2, keeper *
 	}
 }
 
+func (e *environment) addError(contractAddr string, err error) {
+	if err == types.ErrInsufficientRent {
+		e.outOfRentContractAddresses.Add(contractAddr)
+		return
+	}
+	e.failedContractAddresses.Add(contractAddr)
+}
+
 func cacheContext(ctx sdk.Context, env *environment) (sdk.Context, sdk.CacheMultiStore) {
 	cachedCtx, msCached := store.GetCachedContext(ctx)
 	goCtx := context.WithValue(cachedCtx.Context(), dexcache.CtxKeyExecTermSignal, env.executionTerminationSignals)
@@ -159,7 +170,7 @@ func handleDeposits(spanCtx context.Context, ctx sdk.Context, env *environment,
 			continue
 		}
 		if err := keeperWrapper.HandleEBDeposit(spanCtx, ctx, tracer, contract.ContractAddr); err != nil {
-			env.failedContractAddresses.Add(contract.ContractAddr)
+			env.addError(contract.ContractAddr, err)
 		}
 	}
 }
@@ -180,7 +191,7 @@ func handleSettlements(ctx context.Context, sdkCtx sdk.Context, env *environment
 		}
 		if err := HandleSettlements(sdkCtx, contractAddr, keeper, settlements); err != nil {
 			sdkCtx.Logger().Error(fmt.Sprintf("Error handling settlements for %s", contractAddr))
-			env.failedContractAddresses.Add(contractAddr)
+			env.addError(contractAddr, err)
 		}
 		return true
 	})
@@ -197,7 +208,7 @@ func handleUnfulfilledMarketOrders(ctx context.Context, sdkCtx sdk.Context, env
 			}
 			if err := CancelUnfulfilledMarketOrders(ctx, sdkCtx, contract.ContractAddr, keeper, registeredPairs, tracer); err != nil {
 				sdkCtx.Logger().Error(fmt.Sprintf("Error cancelling unfulfilled market orders for %s", contract.ContractAddr))
-				env.failedContractAddresses.Add(contract.ContractAddr)
+				env.addError(contract.ContractAddr, err)
 			}
 		}
 	}
@@ -230,10 +241,10 @@ func orderMatchingRunnable(ctx context.Context, sdkContext sdk.Context, env *env
 
 	if !pairFound || !found {
 		sdkContext.Logger().Error(fmt.Sprintf("No pair or order book for %s", contractInfo.ContractAddr))
-		env.failedContractAddresses.Add(contractInfo.ContractAddr)
+		env.addError(contractInfo.ContractAddr, errors.New("no pair found (internal error)"))
 	} else if settlements, err := HandleExecutionForContract(ctx, sdkContext, contractInfo, keeper, pairs, orderBooks, tracer); err != nil {
 		sdkContext.Logger().Error(fmt.Sprintf("Error for EndBlock of %s", contractInfo.ContractAddr))
-		env.failedContractAddresses.Add(contractInfo.ContractAddr)
+		env.addError(contractInfo.ContractAddr, err)
 	} else {
 		env.settlementsByContract.Store(contractInfo.ContractAddr, settlements)
 	}
@@ -247,16 +258,29 @@ func orderMatchingRunnable(ctx context.Context, sdkContext sdk.Context, env *env
 func filterNewValidContracts(ctx sdk.Context, env *environment) []types.ContractInfoV2 {
 	newValidContracts := []types.ContractInfoV2{}
 	for _, contract := range env.validContractsInfo {
-		if !env.failedContractAddresses.Contains(contract.ContractAddr) {
+		if !env.failedContractAddresses.Contains(contract.ContractAddr) && !env.outOfRentContractAddresses.Contains(contract.ContractAddr) {
 			newValidContracts = append(newValidContracts, contract)
 		}
 	}
 	for _, failedContractAddress := range env.failedContractAddresses.ToOrderedSlice(datastructures.StringComparator) {
 		dexutils.GetMemState(ctx.Context()).DeepFilterAccount(ctx, failedContractAddress)
 	}
+	for _, outOfRentContractAddress := range env.outOfRentContractAddresses.ToOrderedSlice(datastructures.StringComparator) {
+		dexutils.GetMemState(ctx.Context()).DeepFilterAccount(ctx, outOfRentContractAddress)
+	}
 	return newValidContracts
 }
 
+func getOutOfRentContracts(env *environment) []types.ContractInfoV2 {
+	outOfRentContracts := []types.ContractInfoV2{}
+	for _, contract := range env.validContractsInfo {
+		if env.outOfRentContractAddresses.Contains(contract.ContractAddr) {
+			outOfRentContracts = append(outOfRentContracts, contract)
+		}
+	}
+	return outOfRentContracts
+}
+
 func TransferRentFromDexToCollector(ctx sdk.Context, bankKeeper bankkeeper.Keeper, preRents map[string]uint64, postRents map[string]uint64) {
 	total := uint64(0)
 	for addr, preRent := range preRents {

x/dex/keeper/contract.go

@@ -99,7 +99,7 @@ func (k Keeper) ChargeRentForGas(ctx sdk.Context, contractAddr string, gasUsed u
 		if err := k.SetContract(ctx, &contract); err != nil {
 			return err
 		}
-		return errors.New("insufficient rent")
+		return types.ErrInsufficientRent
 	}
 	contract.RentBalance -= uint64(gasPrice)
 	return k.SetContract(ctx, &contract)

x/dex/keeper/params.go

@@ -16,6 +16,10 @@ func (k Keeper) GetSettlementGasAllowance(ctx sdk.Context, numSettlements int) u
 	return k.GetParams(ctx).GasAllowancePerSettlement * uint64(numSettlements)
 }
 
+func (k Keeper) GetMinProcessableRent(ctx sdk.Context) uint64 {
+	return k.GetParams(ctx).MinProcessableRent
+}
+
 // SetParams set the params
 func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 	k.Paramstore.SetParamSet(ctx, &params)

x/dex/module.go

@@ -227,7 +227,7 @@ func (am AppModule) getAllContractInfo(ctx sdk.Context) []types.ContractInfoV2 {
 	// Do not process any contract that has zero rent balance
 	defer telemetry.MeasureSince(time.Now(), am.Name(), "get_all_contract_info")
 	allRegisteredContracts := am.keeper.GetAllContractInfo(ctx)
-	validContracts := utils.Filter(allRegisteredContracts, func(c types.ContractInfoV2) bool { return c.RentBalance > 0 })
+	validContracts := utils.Filter(allRegisteredContracts, func(c types.ContractInfoV2) bool { return c.RentBalance > am.keeper.GetMinProcessableRent(ctx) })
 	telemetry.SetGauge(float32(len(allRegisteredContracts)), am.Name(), "num_of_registered_contracts")
 	telemetry.SetGauge(float32(len(validContracts)), am.Name(), "num_of_valid_contracts")
 	telemetry.SetGauge(float32(len(allRegisteredContracts)-len(validContracts)), am.Name(), "num_of_zero_balance_contracts")
@@ -282,18 +282,20 @@ func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) (ret []abc
 
 	validContractsInfo := am.getAllContractInfo(ctx)
 	validContractsInfoAtBeginning := validContractsInfo
+	outOfRentContractsInfo := []types.ContractInfoV2{}
 	// Each iteration is atomic. If an iteration finishes without any error, it will return,
 	// otherwise it will rollback any state change, filter out contracts that cause the error,
 	// and proceed to the next iteration. The loop is guaranteed to finish since
 	// `validContractAddresses` will always decrease in size every iteration.
 	iterCounter := len(validContractsInfo)
 	endBlockerStartTime := time.Now()
 	for len(validContractsInfo) > 0 {
-		newValidContractsInfo, ctx, ok := contract.EndBlockerAtomic(ctx, &am.keeper, validContractsInfo, am.tracingInfo)
+		newValidContractsInfo, newOutOfRentContractsInfo, ctx, ok := contract.EndBlockerAtomic(ctx, &am.keeper, validContractsInfo, am.tracingInfo)
 		if ok {
 			break
 		}
 		validContractsInfo = newValidContractsInfo
+		outOfRentContractsInfo = newOutOfRentContractsInfo
 
 		// technically we don't really need this if `EndBlockerAtomic` guarantees that `validContractsInfo` size will
 		// always shrink if not `ok`, but just in case, we decided to have an explicit termination criteria here to
@@ -305,16 +307,24 @@ func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) (ret []abc
 		}
 	}
 	telemetry.MeasureSince(endBlockerStartTime, am.Name(), "total_end_blocker_atomic")
-	validContractAddrs := map[string]struct{}{}
+	validContractAddrs, outOfRentContractAddrs := map[string]struct{}{}, map[string]struct{}{}
 	for _, c := range validContractsInfo {
 		validContractAddrs[c.ContractAddr] = struct{}{}
 	}
+	for _, c := range outOfRentContractsInfo {
+		outOfRentContractAddrs[c.ContractAddr] = struct{}{}
+	}
 	for _, c := range validContractsInfoAtBeginning {
-		if _, ok := validContractAddrs[c.ContractAddr]; !ok {
-			ctx.Logger().Error(fmt.Sprintf("Unregistering invalid contract %s", c.ContractAddr))
-			am.keeper.DoUnregisterContract(ctx, c)
-			telemetry.IncrCounter(float32(1), am.Name(), "total_unregistered_contracts")
+		if _, ok := validContractAddrs[c.ContractAddr]; ok {
+			continue
+		}
+		if _, ok := outOfRentContractAddrs[c.ContractAddr]; ok {
+			telemetry.IncrCounter(float32(1), am.Name(), "total_out_of_rent_contracts")
+			continue
 		}
+		ctx.Logger().Error(fmt.Sprintf("Unregistering invalid contract %s", c.ContractAddr))
+		am.keeper.DoUnregisterContract(ctx, c)
+		telemetry.IncrCounter(float32(1), am.Name(), "total_unregistered_contracts")
 	}
 
 	return []abci.ValidatorUpdate{}

x/dex/module_test.go

@@ -616,6 +616,10 @@ func TestEndBlockRollbackWithRentCharge(t *testing.T) {
 			Amount:  sdk.MustNewDecFromStr("10000"),
 		},
 	)
+	// overwrite params for testing
+	params := dexkeeper.GetParams(ctx)
+	params.MinProcessableRent = 0
+	dexkeeper.SetParams(ctx, params)
 
 	ctx = ctx.WithBlockHeight(1)
 	creatorBalanceBefore := bankkeeper.GetBalance(ctx, testAccount, "usei")
@@ -625,8 +629,9 @@ func TestEndBlockRollbackWithRentCharge(t *testing.T) {
 	require.Equal(t, 0, len(matchResult.Orders))
 	// rent should still be charged even if the contract failed, so no rent should be sent to the creator after
 	// auto unregister
-	_, err = dexkeeper.GetContract(ctx, contractAddr.String())
-	require.NotNil(t, err) // auto-unregistered
+	c, err := dexkeeper.GetContract(ctx, contractAddr.String())
+	require.Nil(t, err)                        // out-of-rent contract should not be auto-unregistered
+	require.Equal(t, uint64(0), c.RentBalance) // rent balance should be drained
 	creatorBalanceAfter := bankkeeper.GetBalance(ctx, testAccount, "usei")
 	require.Equal(t, creatorBalanceBefore, creatorBalanceAfter)
 }
@@ -671,6 +676,6 @@ func TestEndBlockContractWithoutPair(t *testing.T) {
 	ti := tracing.Info{
 		Tracer: &tr,
 	}
-	_, _, success := contract.EndBlockerAtomic(ctx, &testApp.DexKeeper, []types.ContractInfoV2{contractInfo}, &ti)
+	_, _, _, success := contract.EndBlockerAtomic(ctx, &testApp.DexKeeper, []types.ContractInfoV2{contractInfo}, &ti)
 	require.True(t, success)
 }

x/dex/types/errors.go

@@ -21,5 +21,6 @@ var (
 	ErrPairNotRegistered          = sdkerrors.Register(ModuleName, 16, "pair is not registered")
 	ErrContractNotExists          = sdkerrors.Register(ModuleName, 17, "Error finding contract info")
 	ErrParsingContractInfo        = sdkerrors.Register(ModuleName, 18, "Error parsing contract info")
+	ErrInsufficientRent           = sdkerrors.Register(ModuleName, 19, "Error contract does not have sufficient fee")
 	ErrCircularContractDependency = sdkerrors.Register(ModuleName, 1103, "circular contract dependency detected")
 )

x/dex/types/params.go

@@ -17,6 +17,7 @@ var (
 	KeyDefaultGasPerCancel       = []byte("KeyDefaultGasPerCancel")
 	KeyMinRentDeposit            = []byte("KeyMinRentDeposit")
 	KeyGasAllowancePerSettlement = []byte("KeyGasAllowancePerSettlement")
+	KeyMinProcessableRent        = []byte("KeyMinProcessableRent")
 )
 
 const (
@@ -27,6 +28,7 @@ const (
 	DefaultDefaultGasPerCancel       = 55000
 	DefaultMinRentDeposit            = 10000000 // 10 sei
 	DefaultGasAllowancePerSettlement = 10000
+	DefaultMinProcessableRent        = 100000
 )
 
 var DefaultSudoCallGasPrice = sdk.NewDecWithPrec(1, 1) // 0.1
@@ -49,6 +51,7 @@ func DefaultParams() Params {
 		DefaultGasPerCancel:       DefaultDefaultGasPerCancel,
 		MinRentDeposit:            DefaultMinRentDeposit,
 		GasAllowancePerSettlement: DefaultGasAllowancePerSettlement,
+		MinProcessableRent:        DefaultMinProcessableRent,
 	}
 }
 
@@ -63,6 +66,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 		paramtypes.NewParamSetPair(KeyDefaultGasPerCancel, &p.DefaultGasPerCancel, validateUint64Param),
 		paramtypes.NewParamSetPair(KeyMinRentDeposit, &p.MinRentDeposit, validateUint64Param),
 		paramtypes.NewParamSetPair(KeyGasAllowancePerSettlement, &p.GasAllowancePerSettlement, validateUint64Param),
+		paramtypes.NewParamSetPair(KeyMinProcessableRent, &p.MinProcessableRent, validateUint64Param),
 	}
 }