From 0e91bf7fad4700096a33ee2b61eb8176d303be58 Mon Sep 17 00:00:00 2001 From: zemyblue Date: Mon, 6 Feb 2023 13:37:26 +0900 Subject: [PATCH] chore: apply review feedback - move all function of `wasmplus/keeper/keeper_extension.go` to `wasmplus/keeper/keeper.go` - remove no need files (keeper_extension.go, metrics.go) --- x/wasmplus/keeper/keeper.go | 65 +++++++++++- x/wasmplus/keeper/keeper_extension.go | 68 ------------ x/wasmplus/keeper/metrics.go | 145 -------------------------- 3 files changed, 62 insertions(+), 216 deletions(-) delete mode 100644 x/wasmplus/keeper/keeper_extension.go delete mode 100644 x/wasmplus/keeper/metrics.go diff --git a/x/wasmplus/keeper/keeper.go b/x/wasmplus/keeper/keeper.go index 92e29a3a78..d023441118 100644 --- a/x/wasmplus/keeper/keeper.go +++ b/x/wasmplus/keeper/keeper.go @@ -5,6 +5,7 @@ import ( "github.com/line/lbm-sdk/codec" sdk "github.com/line/lbm-sdk/types" + sdkerrors "github.com/line/lbm-sdk/types/errors" bankpluskeeper "github.com/line/lbm-sdk/x/bankplus/keeper" paramtypes "github.com/line/lbm-sdk/x/params/types" "github.com/line/ostracon/libs/log" @@ -18,7 +19,7 @@ type Keeper struct { wasmkeeper.Keeper cdc codec.Codec storeKey sdk.StoreKey - metrics *Metrics + metrics *wasmkeeper.Metrics bank bankpluskeeper.Keeper } @@ -48,9 +49,8 @@ func NewKeeper( result := Keeper{ cdc: cdc, storeKey: storeKey, - metrics: NopMetrics(), + metrics: wasmkeeper.NopMetrics(), bank: bankPlusKeeper, - //paramSpace: paramSpace, } result.Keeper = wasmkeeper.NewKeeper( cdc, @@ -89,3 +89,62 @@ func (Keeper) Logger(ctx sdk.Context) log.Logger { func ModuleLogger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName)) } + +func (k Keeper) IsInactiveContract(ctx sdk.Context, contractAddress sdk.AccAddress) bool { + store := ctx.KVStore(k.storeKey) + return store.Has(types.GetInactiveContractKey(contractAddress)) +} + +func (k Keeper) IterateInactiveContracts(ctx sdk.Context, fn func(contractAddress sdk.AccAddress) (stop bool)) { + store := ctx.KVStore(k.storeKey) + prefix := types.InactiveContractPrefix + iterator := sdk.KVStorePrefixIterator(store, prefix) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + contractAddress := sdk.AccAddress(iterator.Value()) + if stop := fn(contractAddress); stop { + break + } + } +} + +func (k Keeper) addInactiveContract(ctx sdk.Context, contractAddress sdk.AccAddress) { + store := ctx.KVStore(k.storeKey) + key := types.GetInactiveContractKey(contractAddress) + + store.Set(key, contractAddress) +} + +func (k Keeper) deleteInactiveContract(ctx sdk.Context, contractAddress sdk.AccAddress) { + store := ctx.KVStore(k.storeKey) + key := types.GetInactiveContractKey(contractAddress) + store.Delete(key) +} + +// activateContract delete the contract address from inactivateContract list if the contract is deactivated. +func (k Keeper) activateContract(ctx sdk.Context, contractAddress sdk.AccAddress) error { + if !k.IsInactiveContract(ctx, contractAddress) { + return sdkerrors.Wrapf(wasmtypes.ErrNotFound, "no inactivate contract %s", contractAddress.String()) + } + + k.deleteInactiveContract(ctx, contractAddress) + k.bank.DeleteFromInactiveAddr(ctx, contractAddress) + + return nil +} + +// deactivateContract add the contract address to inactivateContract list. +func (k Keeper) deactivateContract(ctx sdk.Context, contractAddress sdk.AccAddress) error { + if k.IsInactiveContract(ctx, contractAddress) { + return sdkerrors.Wrapf(wasmtypes.ErrAccountExists, "already inactivate contract %s", contractAddress.String()) + } + if !k.HasContractInfo(ctx, contractAddress) { + return sdkerrors.Wrapf(wasmtypes.ErrInvalid, "no contract %s", contractAddress.String()) + } + + k.addInactiveContract(ctx, contractAddress) + k.bank.AddToInactiveAddr(ctx, contractAddress) + + return nil +} diff --git a/x/wasmplus/keeper/keeper_extension.go b/x/wasmplus/keeper/keeper_extension.go deleted file mode 100644 index 6ddcff4882..0000000000 --- a/x/wasmplus/keeper/keeper_extension.go +++ /dev/null @@ -1,68 +0,0 @@ -package keeper - -import ( - sdk "github.com/line/lbm-sdk/types" - sdkerrors "github.com/line/lbm-sdk/types/errors" - - wasmtypes "github.com/line/wasmd/x/wasm/types" - "github.com/line/wasmd/x/wasmplus/types" -) - -func (k Keeper) IsInactiveContract(ctx sdk.Context, contractAddress sdk.AccAddress) bool { - store := ctx.KVStore(k.storeKey) - return store.Has(types.GetInactiveContractKey(contractAddress)) -} - -func (k Keeper) IterateInactiveContracts(ctx sdk.Context, fn func(contractAddress sdk.AccAddress) (stop bool)) { - store := ctx.KVStore(k.storeKey) - prefix := types.InactiveContractPrefix - iterator := sdk.KVStorePrefixIterator(store, prefix) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - contractAddress := sdk.AccAddress(iterator.Value()) - if stop := fn(contractAddress); stop { - break - } - } -} - -func (k Keeper) addInactiveContract(ctx sdk.Context, contractAddress sdk.AccAddress) { - store := ctx.KVStore(k.storeKey) - key := types.GetInactiveContractKey(contractAddress) - - store.Set(key, contractAddress) -} - -func (k Keeper) deleteInactiveContract(ctx sdk.Context, contractAddress sdk.AccAddress) { - store := ctx.KVStore(k.storeKey) - key := types.GetInactiveContractKey(contractAddress) - store.Delete(key) -} - -// activateContract delete the contract address from inactivateContract list if the contract is deactivated. -func (k Keeper) activateContract(ctx sdk.Context, contractAddress sdk.AccAddress) error { - if !k.IsInactiveContract(ctx, contractAddress) { - return sdkerrors.Wrapf(wasmtypes.ErrNotFound, "no inactivate contract %s", contractAddress.String()) - } - - k.deleteInactiveContract(ctx, contractAddress) - k.bank.DeleteFromInactiveAddr(ctx, contractAddress) - - return nil -} - -// deactivateContract add the contract address to inactivateContract list. -func (k Keeper) deactivateContract(ctx sdk.Context, contractAddress sdk.AccAddress) error { - if k.IsInactiveContract(ctx, contractAddress) { - return sdkerrors.Wrapf(wasmtypes.ErrAccountExists, "already inactivate contract %s", contractAddress.String()) - } - if !k.HasContractInfo(ctx, contractAddress) { - return sdkerrors.Wrapf(wasmtypes.ErrInvalid, "no contract %s", contractAddress.String()) - } - - k.addInactiveContract(ctx, contractAddress) - k.bank.AddToInactiveAddr(ctx, contractAddress) - - return nil -} diff --git a/x/wasmplus/keeper/metrics.go b/x/wasmplus/keeper/metrics.go deleted file mode 100644 index cfe8016785..0000000000 --- a/x/wasmplus/keeper/metrics.go +++ /dev/null @@ -1,145 +0,0 @@ -package keeper - -import ( - "github.com/go-kit/kit/metrics" - "github.com/go-kit/kit/metrics/discard" - go_prometheus "github.com/go-kit/kit/metrics/prometheus" - "github.com/prometheus/client_golang/prometheus" - - wasmvmtypes "github.com/line/wasmvm/types" -) - -const ( - labelPinned = "pinned" - labelMemory = "memory" - labelFs = "fs" - MetricsSubsystem = "wasm" -) - -type Metrics struct { - InstantiateElapsedTimes metrics.Histogram - ExecuteElapsedTimes metrics.Histogram - MigrateElapsedTimes metrics.Histogram - SudoElapsedTimes metrics.Histogram - QuerySmartElapsedTimes metrics.Histogram - QueryRawElapsedTimes metrics.Histogram -} - -func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics { - return &Metrics{ - InstantiateElapsedTimes: go_prometheus.NewSummaryFrom(prometheus.SummaryOpts{ - Namespace: namespace, - Subsystem: MetricsSubsystem, - Name: "instantiate", - Help: "elapsed time of Instantiate the wasm contract", - }, nil), - ExecuteElapsedTimes: go_prometheus.NewSummaryFrom(prometheus.SummaryOpts{ - Namespace: namespace, - Subsystem: MetricsSubsystem, - Name: "execute", - Help: "elapsed time of Execute the wasm contract", - }, nil), - MigrateElapsedTimes: go_prometheus.NewSummaryFrom(prometheus.SummaryOpts{ - Namespace: namespace, - Subsystem: MetricsSubsystem, - Name: "migrate", - Help: "elapsed time of Migrate the wasm contract", - }, nil), - SudoElapsedTimes: go_prometheus.NewSummaryFrom(prometheus.SummaryOpts{ - Namespace: namespace, - Subsystem: MetricsSubsystem, - Name: "sudo", - Help: "elapsed time of Sudo the wasm contract", - }, nil), - QuerySmartElapsedTimes: go_prometheus.NewSummaryFrom(prometheus.SummaryOpts{ - Namespace: namespace, - Subsystem: MetricsSubsystem, - Name: "query_smart", - Help: "elapsed time of QuerySmart the wasm contract", - }, nil), - QueryRawElapsedTimes: go_prometheus.NewSummaryFrom(prometheus.SummaryOpts{ - Namespace: namespace, - Subsystem: MetricsSubsystem, - Name: "query_raw", - Help: "elapsed time of QueryRaw the wasm contract", - }, nil), - } -} - -// NopMetrics returns no-op Metrics. -func NopMetrics() *Metrics { - return &Metrics{ - InstantiateElapsedTimes: discard.NewHistogram(), - ExecuteElapsedTimes: discard.NewHistogram(), - MigrateElapsedTimes: discard.NewHistogram(), - SudoElapsedTimes: discard.NewHistogram(), - QuerySmartElapsedTimes: discard.NewHistogram(), - QueryRawElapsedTimes: discard.NewHistogram(), - } -} - -type MetricsProvider func() *Metrics - -// PrometheusMetricsProvider returns PrometheusMetrics for each store -func PrometheusMetricsProvider(namespace string, labelsAndValues ...string) func() *Metrics { - return func() *Metrics { - return PrometheusMetrics(namespace, labelsAndValues...) - } -} - -// NopMetricsProvider returns NopMetrics for each store -func NopMetricsProvider() func() *Metrics { - //nolint:gocritic - return func() *Metrics { - return NopMetrics() - } -} - -// metricSource source of wasmvm metrics -type metricSource interface { - GetMetrics() (*wasmvmtypes.Metrics, error) -} - -var _ prometheus.Collector = (*WasmVMCacheMetricsCollector)(nil) - -// WasmVMCacheMetricsCollector custom metrics collector to be used with Prometheus -type WasmVMCacheMetricsCollector struct { - source metricSource - CacheHitsDescr *prometheus.Desc - CacheMissesDescr *prometheus.Desc - CacheElementsDescr *prometheus.Desc - CacheSizeDescr *prometheus.Desc -} - -// Register registers all metrics -func (p *WasmVMCacheMetricsCollector) Register(r prometheus.Registerer) { - r.MustRegister(p) -} - -// Describe sends the super-set of all possible descriptors of metrics -func (p *WasmVMCacheMetricsCollector) Describe(descs chan<- *prometheus.Desc) { - descs <- p.CacheHitsDescr - descs <- p.CacheMissesDescr - descs <- p.CacheElementsDescr - descs <- p.CacheSizeDescr -} - -// Collect is called by the Prometheus registry when collecting metrics. -func (p *WasmVMCacheMetricsCollector) Collect(c chan<- prometheus.Metric) { - m, err := p.source.GetMetrics() - if err != nil { - return - } - c <- prometheus.MustNewConstMetric(p.CacheHitsDescr, prometheus.CounterValue, float64(m.HitsPinnedMemoryCache), labelPinned) - c <- prometheus.MustNewConstMetric(p.CacheHitsDescr, prometheus.CounterValue, float64(m.HitsMemoryCache), labelMemory) - c <- prometheus.MustNewConstMetric(p.CacheHitsDescr, prometheus.CounterValue, float64(m.HitsFsCache), labelFs) - c <- prometheus.MustNewConstMetric(p.CacheMissesDescr, prometheus.CounterValue, float64(m.Misses)) - c <- prometheus.MustNewConstMetric(p.CacheElementsDescr, prometheus.GaugeValue, float64(m.ElementsPinnedMemoryCache), labelPinned) - c <- prometheus.MustNewConstMetric(p.CacheElementsDescr, prometheus.GaugeValue, float64(m.ElementsMemoryCache), labelMemory) - c <- prometheus.MustNewConstMetric(p.CacheSizeDescr, prometheus.GaugeValue, float64(m.SizeMemoryCache), labelMemory) - c <- prometheus.MustNewConstMetric(p.CacheSizeDescr, prometheus.GaugeValue, float64(m.SizePinnedMemoryCache), labelPinned) - // Node about fs metrics: - // The number of elements and the size of elements in the file system cache cannot easily be obtained. - // We had to either scan the whole directory of potentially thousands of files or track the values when files are added or removed. - // Such a tracking would need to be on disk such that the values are not cleared when the node is restarted. -}