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

do not unregister if out of rent #743

Merged
merged 2 commits into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions proto/dex/params.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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\""
];
}
42 changes: 33 additions & 9 deletions x/dex/contract/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package contract

import (
"context"
"errors"
"fmt"
"sync"
"time"
Expand Down Expand Up @@ -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]
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
}
}
Expand All @@ -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
})
Expand All @@ -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)
}
}
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion x/dex/keeper/contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions x/dex/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 17 additions & 7 deletions x/dex/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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{}
Expand Down
11 changes: 8 additions & 3 deletions x/dex/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions x/dex/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
4 changes: 4 additions & 0 deletions x/dex/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var (
KeyDefaultGasPerCancel = []byte("KeyDefaultGasPerCancel")
KeyMinRentDeposit = []byte("KeyMinRentDeposit")
KeyGasAllowancePerSettlement = []byte("KeyGasAllowancePerSettlement")
KeyMinProcessableRent = []byte("KeyMinProcessableRent")
)

const (
Expand All @@ -27,6 +28,7 @@ const (
DefaultDefaultGasPerCancel = 55000
DefaultMinRentDeposit = 10000000 // 10 sei
DefaultGasAllowancePerSettlement = 10000
DefaultMinProcessableRent = 100000
)

var DefaultSudoCallGasPrice = sdk.NewDecWithPrec(1, 1) // 0.1
Expand All @@ -49,6 +51,7 @@ func DefaultParams() Params {
DefaultGasPerCancel: DefaultDefaultGasPerCancel,
MinRentDeposit: DefaultMinRentDeposit,
GasAllowancePerSettlement: DefaultGasAllowancePerSettlement,
MinProcessableRent: DefaultMinProcessableRent,
}
}

Expand All @@ -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),
}
}

Expand Down
Loading