From 18cbb229c577b6fe3437b5700d778c8e1548ec44 Mon Sep 17 00:00:00 2001 From: testinginprod <98415576+testinginprod@users.noreply.github.com> Date: Sat, 20 Jan 2024 02:24:27 +0100 Subject: [PATCH] fix(StargateQueries): use a sync pool when unmarshalling responses of protobuf objects (#7346) * use a sync pool when unmarshalling responses of protobuf objects in StargateQueries * fix uninitted pool * type assertion and lints * changelog * add comment for returnStargateResponseToPool * add setWhitelistedQuery comment * lint --------- Co-authored-by: unknown unknown Co-authored-by: Adam Tucker (cherry picked from commit 2caa5c6b16742d73830557c0c41bc163e1c934d5) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 56 +++++++++++++++++++++++++ cmd/osmosisd/cmd/stargate-query.go | 2 +- wasmbinding/export_test.go | 6 ++- wasmbinding/query_plugin.go | 6 ++- wasmbinding/stargate_whitelist.go | 66 ++++++++++++++++++++---------- 5 files changed, 112 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d273d35d530..06e1084a48e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,62 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +<<<<<<< HEAD +======= +## Unreleased + +### State Breaking + +* [#7181](https://github.com/osmosis-labs/osmosis/pull/7181) Improve errors for out of gas + +### Bug Fixes + +* [#7346](https://github.com/osmosis-labs/osmosis/pull/7346) Prevent heavy gRPC load from app hashing nodes + +## v22.0.0 + +### Fee Market Parameter Updates +* [#7285](https://github.com/osmosis-labs/osmosis/pull/7285) The following updates are applied: + * Dynamic recheck factor based on current base fee value. Under 0.01, the recheck factor is 3. + In face of continuous spam, will take ~19 blocks from base fee > spam cost, to mempool eviction. + Above 0.01, the recheck factor is 2.3. In face of continuous spam, will take ~15 blocks from base fee > spam cost, to mempool eviction. + * Reset interval set to 6000 which is approximately 8.5 hours. + * Default base fee is reduced by 2 to 0.005. + * Set target gas to .625 * block_gas_limt = 187.5 million + +### State Breaking + +### API +* [#6991](https://github.com/osmosis-labs/osmosis/pull/6991) Fix: total liquidity poolmanager grpc gateway query +* [#7237](https://github.com/osmosis-labs/osmosis/pull/7237) Removes tx_fee_tracker from the proto rev tracker, no longer tracks in state. +* [#7240](https://github.com/osmosis-labs/osmosis/pull/7240) Protorev tracker now tracks a coin array to improve gas efficiency. + +### Features +* [#6847](https://github.com/osmosis-labs/osmosis/pull/6847) feat: allow sending denoms with URL encoding +* [#7270](https://github.com/osmosis-labs/osmosis/pull/7270) feat: eip target gas from consensus params + +### Bug Fixes +* [#7120](https://github.com/osmosis-labs/osmosis/pull/7120) fix: remove duplicate `query gamm pool` subcommand +* [#7139](https://github.com/osmosis-labs/osmosis/pull/7139) fix: add amino signing support to tokenfactory messages +* [#7245](https://github.com/osmosis-labs/osmosis/pull/7245) fix: correcting json tag value for `SwapAmountOutSplitRouteWrapper.OutDenom` +* [#7267](https://github.com/osmosis-labs/osmosis/pull/7267) fix: support CL pools in tx fee module +* [#7220](https://github.com/osmosis-labs/osmosis/pull/7220) Register consensus params; Set MaxGas to 300m and MaxBytes to 5mb. +* [#7300](https://github.com/osmosis-labs/osmosis/pull/7300) fix: update wasm vm as per CWA-2023-004 + +### Misc Improvements +* [#6993](https://github.com/osmosis-labs/osmosis/pull/6993) chore: add mutative api for BigDec.BigInt() +* [#7074](https://github.com/osmosis-labs/osmosis/pull/7074) perf: don't load all poolmanager params every swap +* [#7243](https://github.com/osmosis-labs/osmosis/pull/7243) chore: update gov metadata length from 256 to 10200 +* [#7258](https://github.com/osmosis-labs/osmosis/pull/7258) Remove an iterator call in CL swaps and spot price calls. +* [#7259](https://github.com/osmosis-labs/osmosis/pull/7259) Lower gas and CPU overhead of chargeTakerFee (in every swap) +* [#7249](https://github.com/osmosis-labs/osmosis/pull/7249) Double auth tx size cost per byte from 10 to 20 +* [#7272](https://github.com/osmosis-labs/osmosis/pull/7272) Upgrade go 1.20 -> 1.21 +* [#7282](https://github.com/osmosis-labs/osmosis/pull/7282) perf:Update sdk fork to no longer utilize reverse denom mapping, reducing gas costs. +* [#7203](https://github.com/osmosis-labs/osmosis/pull/7203) Make a maximum number of pools of 100 billion. +* [#7282](https://github.com/osmosis-labs/osmosis/pull/7282) Update sdk fork to no longer utilize reverse denom mapping, reducing gas costs. +* [#7291](https://github.com/osmosis-labs/osmosis/pull/7291) Raise mempool config's default max gas per tx configs. + +>>>>>>> 2caa5c6b (fix(StargateQueries): use a sync pool when unmarshalling responses of protobuf objects (#7346)) ## v21.2.2 ### Features * [#7238](https://github.com/osmosis-labs/osmosis/pull/7238) re-add clawback vesting command diff --git a/cmd/osmosisd/cmd/stargate-query.go b/cmd/osmosisd/cmd/stargate-query.go index 378341bcd38..9d56ddb2733 100644 --- a/cmd/osmosisd/cmd/stargate-query.go +++ b/cmd/osmosisd/cmd/stargate-query.go @@ -78,7 +78,7 @@ Example: //nolint:staticcheck func GetStructAndFill(queryPath, module, structName string, structArguments ...string) (interface{}, error) { const ParamRequest = "QueryParamsRequest" - _, err := wasmbinding.GetWhitelistedQuery(queryPath) + err := wasmbinding.IsWhitelistedQuery(queryPath) if err != nil { return nil, err } diff --git a/wasmbinding/export_test.go b/wasmbinding/export_test.go index 537dc1496e3..7cf51e6bf7d 100644 --- a/wasmbinding/export_test.go +++ b/wasmbinding/export_test.go @@ -2,6 +2,10 @@ package wasmbinding import "github.com/cosmos/cosmos-sdk/codec" -func SetWhitelistedQuery(queryPath string, protoType codec.ProtoMarshaler) { +func SetWhitelistedQuery[T any, PT protoTypeG[T]](queryPath string, protoType PT) { setWhitelistedQuery(queryPath, protoType) } + +func GetWhitelistedQuery(queryPath string) (codec.ProtoMarshaler, error) { + return getWhitelistedQuery(queryPath) +} diff --git a/wasmbinding/query_plugin.go b/wasmbinding/query_plugin.go index 2cf392ff512..0a510490cea 100644 --- a/wasmbinding/query_plugin.go +++ b/wasmbinding/query_plugin.go @@ -17,11 +17,15 @@ import ( // StargateQuerier dispatches whitelisted stargate queries func StargateQuerier(queryRouter baseapp.GRPCQueryRouter, cdc codec.Codec) func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { return func(ctx sdk.Context, request *wasmvmtypes.StargateQuery) ([]byte, error) { - protoResponseType, err := GetWhitelistedQuery(request.Path) + protoResponseType, err := getWhitelistedQuery(request.Path) if err != nil { return nil, err } + // no matter what happens after this point, we must return + // the response type to prevent sync.Pool from leaking. + defer returnStargateResponseToPool(request.Path, protoResponseType) + route := queryRouter.Route(request.Path) if route == nil { return nil, wasmvmtypes.UnsupportedRequest{Kind: fmt.Sprintf("No route to query '%s'", request.Path)} diff --git a/wasmbinding/stargate_whitelist.go b/wasmbinding/stargate_whitelist.go index cc66b91f61b..affd0303df2 100644 --- a/wasmbinding/stargate_whitelist.go +++ b/wasmbinding/stargate_whitelist.go @@ -33,14 +33,15 @@ import ( epochtypes "github.com/osmosis-labs/osmosis/x/epochs/types" ) -// stargateWhitelist keeps whitelist and its deterministic +// stargateResponsePools keeps whitelist and its deterministic // response binding for stargate queries. // CONTRACT: since results of queries go into blocks, queries being added here should always be // deterministic or can cause non-determinism in the state machine. // -// The query can be multi-thread, so we have to use -// thread safe sync.Map. -var stargateWhitelist sync.Map +// The query is multi-threaded so we're using a sync.Pool +// to manage the allocation and de-allocation of newly created +// pb objects. +var stargateResponsePools = make(map[string]*sync.Pool) // Note: When adding a migration here, we should also add it to the Async ICQ params in the upgrade. // In the future we may want to find a better way to keep these in sync @@ -184,34 +185,57 @@ func init() { setWhitelistedQuery("/osmosis.concentratedliquidity.v1beta1.Query/CFMMPoolIdLinkFromConcentratedPoolId", &concentratedliquidityquery.CFMMPoolIdLinkFromConcentratedPoolIdResponse{}) } -// GetWhitelistedQuery returns the whitelisted query at the provided path. +// IsWhitelistedQuery returns if the query is not whitelisted. +func IsWhitelistedQuery(queryPath string) error { + _, isWhitelisted := stargateResponsePools[queryPath] + if !isWhitelisted { + return wasmvmtypes.UnsupportedRequest{Kind: fmt.Sprintf("'%s' path is not allowed from the contract", queryPath)} + } + return nil +} + +// getWhitelistedQuery returns the whitelisted query at the provided path. // If the query does not exist, or it was setup wrong by the chain, this returns an error. -func GetWhitelistedQuery(queryPath string) (codec.ProtoMarshaler, error) { - protoResponseAny, isWhitelisted := stargateWhitelist.Load(queryPath) +// CONTRACT: must call returnStargateResponseToPool in order to avoid pointless allocs. +func getWhitelistedQuery(queryPath string) (codec.ProtoMarshaler, error) { + protoResponseAny, isWhitelisted := stargateResponsePools[queryPath] if !isWhitelisted { return nil, wasmvmtypes.UnsupportedRequest{Kind: fmt.Sprintf("'%s' path is not allowed from the contract", queryPath)} } - protoResponseType, ok := protoResponseAny.(codec.ProtoMarshaler) + protoMarshaler, ok := protoResponseAny.Get().(codec.ProtoMarshaler) if !ok { - return nil, wasmvmtypes.Unknown{} + return nil, fmt.Errorf("failed to assert type to codec.ProtoMarshaler") } - return protoResponseType, nil + return protoMarshaler, nil } -func setWhitelistedQuery(queryPath string, protoType codec.ProtoMarshaler) { - stargateWhitelist.Store(queryPath, protoType) +type protoTypeG[T any] interface { + *T + codec.ProtoMarshaler +} + +// setWhitelistedQuery sets the whitelisted query at the provided path. +// This method also creates a sync.Pool for the provided protoMarshaler. +// We use generics so we can properly instantiate an object that the +// queryPath expects as a response. +func setWhitelistedQuery[T any, PT protoTypeG[T]](queryPath string, _ PT) { + stargateResponsePools[queryPath] = &sync.Pool{ + New: func() any { + return PT(new(T)) + }, + } +} + +// returnStargateResponseToPool returns the provided protoMarshaler to the appropriate pool based on it's query path. +func returnStargateResponseToPool(queryPath string, pb codec.ProtoMarshaler) { + stargateResponsePools[queryPath].Put(pb) } func GetStargateWhitelistedPaths() (keys []string) { // Iterate over the map and collect the keys - stargateWhitelist.Range(func(key, value interface{}) bool { - keyStr, ok := key.(string) - if !ok { - panic("key is not a string") - } - keys = append(keys, keyStr) - return true - }) - + keys = make([]string, 0, len(stargateResponsePools)) + for k := range stargateResponsePools { + keys = append(keys, k) + } return keys }