Allow unlisted denoms in the /custom-direct-quote

CHANGELOG.md

@@ -35,6 +35,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 # Changelog
 
+## Unreleased
+
+- Allow unlisted denoms in the /custom-direct-quote endpoint.
+
 ## v26.1.0
 
 e42b32bc SQS-412 | Active Orders Query: SSE (#518)

domain/mocks/tokens_usecase_mock.go

@@ -31,6 +31,7 @@ type TokensUsecaseMock struct {
 	UpdateAssetsAtHeightIntervalSyncFunc func(height uint64) error
 	SetTokenRegistryLoaderFunc           func(loader domain.TokenRegistryLoader)
 	ClearPoolDenomMetadataFunc           func()
+	IsValidListedDenomFunc               func(chainDenom string) bool
 }
 
 var _ mvc.TokensUsecase = &TokensUsecaseMock{}
@@ -172,3 +173,11 @@ func (m *TokensUsecaseMock) ClearPoolDenomMetadata() {
 	}
 	panic("unimplemented")
 }
+
+// IsValidListedDenom implements mvc.TokensUsecase.
+func (m *TokensUsecaseMock) IsValidListedDenom(chainDenom string) bool {
+	if m.IsValidListedDenomFunc != nil {
+		return m.IsValidListedDenomFunc(chainDenom)
+	}
+	return false
+}

domain/mvc/tokens.go

@@ -79,8 +79,12 @@ type TokensUsecase interface {
 	// RegisterPricingStrategy registers a pricing strategy for a given pricing source.
 	RegisterPricingStrategy(source domain.PricingSourceType, strategy domain.PricingSource)
 
+	// IsValidChainDenom checks if the chain denom is valid
 	IsValidChainDenom(chainDenom string) bool
 
+	// IsValidListedDenom checks if the chain denom is valid
+	IsValidListedDenom(chainDenom string) bool
+
 	// IsValidPricingSource checks if the pricing source is a valid one
 	IsValidPricingSource(pricingSource int) bool
 
@@ -101,8 +105,10 @@ type TokensUsecase interface {
 // ValidateChainDenomQueryParam validates the chain denom query parameter.
 // If isHumanDenoms is true, it converts the human denom to chain denom.
 // If isHumanDenoms is false, it validates the chain denom.
+// If doesAllowUnlistedDenoms is true, it allows unlisted denoms.
+// If doesAllowUnlistedDenoms is false, it does not allow unlisted denoms.
 // Returns the chain denom and an error if any.
-func ValidateChainDenomQueryParam(tokensUsecase TokensUsecase, denom string, isHumanDenoms bool) (string, error) {
+func ValidateChainDenomQueryParam(tokensUsecase TokensUsecase, denom string, isHumanDenoms bool, doesAllowUnlistedDenoms bool) (string, error) {
 	// Note that sdk.Coins initialization
 	// auto-converts base denom from human
 	// to IBC notation.
@@ -120,8 +126,14 @@ func ValidateChainDenomQueryParam(tokensUsecase TokensUsecase, denom string, isH
 			return tokensUsecase.GetChainDenom(denom)
 		}
 	} else {
-		if !tokensUsecase.IsValidChainDenom(denom) {
-			return "", fmt.Errorf("denom is not a valid chain denom (%s)", denom)
+		if !doesAllowUnlistedDenoms {
+			if !tokensUsecase.IsValidListedDenom(denom) {
+				return "", fmt.Errorf("denom is not a valid listed chain denom (%s)", denom)
+			}
+		} else {
+			if !tokensUsecase.IsValidChainDenom(denom) {
+				return "", fmt.Errorf("denom is not a valid chain denom (%s)", denom)
+			}
 		}
 	}
 
@@ -130,15 +142,15 @@ func ValidateChainDenomQueryParam(tokensUsecase TokensUsecase, denom string, isH
 }
 
 // ValidateChainDenomsQueryParam validates the chain denom query parameters.
-func ValidateChainDenomsQueryParam(c echo.Context, tokensUsecase TokensUsecase, denoms []string) ([]string, error) {
+func ValidateChainDenomsQueryParam(c echo.Context, tokensUsecase TokensUsecase, denoms []string, doesAllowUnlistedDenoms bool) ([]string, error) {
 	isHumanDenoms, err := domain.GetIsHumanDenomsQueryParam(c)
 	if err != nil {
 		return nil, err
 	}
 
 	chainDenoms := make([]string, len(denoms))
 	for i, denom := range denoms {
-		chainDenom, err := ValidateChainDenomQueryParam(tokensUsecase, denom, isHumanDenoms)
+		chainDenom, err := ValidateChainDenomQueryParam(tokensUsecase, denom, isHumanDenoms, doesAllowUnlistedDenoms)
 		if err != nil {
 			return nil, err
 		}

domain/mvc/tokens_test.go

@@ -0,0 +1,162 @@
+package mvc_test
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/osmosis-labs/sqs/domain/mocks"
+	"github.com/osmosis-labs/sqs/domain/mvc"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	denom    = "uatom"
+	ibcDenom = "ibc/" + denom
+)
+
+func TestValidateChainDenomQueryParam(t *testing.T) {
+
+	baseDenom, err := sdk.GetBaseDenom()
+	require.NoError(t, err)
+
+	defaultError := errors.New("default error")
+
+	// Test cases
+	tests := []struct {
+		name                    string
+		denom                   string
+		isHumanDenoms           bool
+		doesAllowUnlistedDenoms bool
+		mock                    *mocks.TokensUsecaseMock
+		expectedDenom           string
+		expectedError           string
+		baseError               bool // true if we expect base denom error
+	}{
+		{
+			name:                    "Error validating base denom",
+			denom:                   "not" + baseDenom,
+			isHumanDenoms:           true,
+			doesAllowUnlistedDenoms: true,
+			mock: &mocks.TokensUsecaseMock{
+				GetChainDenomFunc: func(humanDenom string) (string, error) {
+					return "", defaultError
+				},
+			},
+			expectedDenom: "",
+			expectedError: defaultError.Error(),
+		},
+		{
+			name:                    "Human denom conversion success",
+			denom:                   denom,
+			isHumanDenoms:           true,
+			doesAllowUnlistedDenoms: true,
+			mock: &mocks.TokensUsecaseMock{
+				GetChainDenomFunc: func(humanDenom string) (string, error) {
+					return "ibc/" + humanDenom, nil
+				},
+			},
+			expectedDenom: ibcDenom,
+			expectedError: "",
+		},
+		{
+			name:                    "Human denom conversion success",
+			denom:                   denom,
+			isHumanDenoms:           true,
+			doesAllowUnlistedDenoms: true,
+			mock: &mocks.TokensUsecaseMock{
+				GetChainDenomFunc: func(humanDenom string) (string, error) {
+					return ibcDenom, nil
+				},
+			},
+			expectedDenom: ibcDenom,
+			expectedError: "",
+		},
+		{
+			name:                    "Human denom conversion error",
+			denom:                   "invalid",
+			isHumanDenoms:           true,
+			doesAllowUnlistedDenoms: true,
+			mock: &mocks.TokensUsecaseMock{
+				GetChainDenomFunc: func(humanDenom string) (string, error) {
+					return "", fmt.Errorf("invalid denom")
+				},
+			},
+			expectedDenom: "",
+			expectedError: "invalid denom",
+		},
+		{
+			name:                    "Valid unlisted chain denom",
+			denom:                   ibcDenom,
+			isHumanDenoms:           false,
+			doesAllowUnlistedDenoms: true,
+			mock: &mocks.TokensUsecaseMock{
+				IsValidChainDenomFunc: func(chainDenom string) bool {
+					return true
+				},
+			},
+			expectedDenom: ibcDenom,
+			expectedError: "",
+		},
+		{
+			name:                    "Invalid chain denom with unlisted allowed",
+			denom:                   "invalid",
+			isHumanDenoms:           false,
+			doesAllowUnlistedDenoms: true,
+			mock: &mocks.TokensUsecaseMock{
+				IsValidChainDenomFunc: func(chainDenom string) bool {
+					return false
+				},
+			},
+			expectedDenom: "",
+			expectedError: "denom is not a valid chain denom (invalid)",
+		},
+		{
+			name:                    "Unlisted chain denom when not allowed",
+			denom:                   ibcDenom,
+			isHumanDenoms:           false,
+			doesAllowUnlistedDenoms: false,
+			mock: &mocks.TokensUsecaseMock{
+				IsValidListedDenomFunc: func(denom string) bool {
+					return false
+				},
+			},
+			expectedDenom: "",
+			expectedError: "denom is not a valid listed chain denom (" + ibcDenom + ")",
+		},
+		{
+			name:                    "Valid listed chain denom",
+			denom:                   ibcDenom,
+			isHumanDenoms:           false,
+			doesAllowUnlistedDenoms: false,
+			mock: &mocks.TokensUsecaseMock{
+				IsValidListedDenomFunc: func(denom string) bool {
+					return true
+				},
+			},
+			expectedDenom: ibcDenom,
+			expectedError: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// System under test
+			result, err := mvc.ValidateChainDenomQueryParam(tt.mock, tt.denom, tt.isHumanDenoms, tt.doesAllowUnlistedDenoms)
+
+			// Assert results
+			if tt.baseError {
+				assert.Empty(t, result)
+				assert.NoError(t, err)
+			} else if tt.expectedError != "" {
+				assert.Empty(t, result)
+				assert.EqualError(t, err, tt.expectedError)
+			} else {
+				assert.Equal(t, tt.expectedDenom, result)
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

router/delivery/http/router_handler.go

@@ -105,7 +105,9 @@ func (a *RouterHandler) GetOptimalQuote(c echo.Context) (err error) {
 		tokenIn, tokenOutDenom = req.TokenOut, req.TokenInDenom
 	}
 
-	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, []string{tokenIn.Denom, tokenOutDenom})
+	// We do not allow unlisted denoms since this is a production from the swap tool.
+	doesAllowUnlistedDenoms := false
+	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, []string{tokenIn.Denom, tokenOutDenom}, doesAllowUnlistedDenoms)
 	if err != nil {
 		return c.JSON(http.StatusBadRequest, domain.ResponseError{Message: err.Error()})
 	}
@@ -200,7 +202,10 @@ func (a *RouterHandler) GetDirectCustomQuote(c echo.Context) (err error) {
 	}
 
 	// Apply human denoms conversion if required.
-	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, append([]string{tokenIn.Denom}, tokenOutDenom...))
+	// We allow unlisted denoms since this is a production endpoint from the pools page
+	// where we want to support all chain denoms.
+	doesAllowUnlistedDenoms := true
+	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, append([]string{tokenIn.Denom}, tokenOutDenom...), doesAllowUnlistedDenoms)
 	if err != nil {
 		return c.JSON(http.StatusBadRequest, domain.ResponseError{Message: err.Error()})
 	}
@@ -250,7 +255,10 @@ func (a *RouterHandler) GetCandidateRoutes(c echo.Context) error {
 		return c.JSON(domain.GetStatusCode(err), domain.ResponseError{Message: err.Error()})
 	}
 
-	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, []string{tokenIn, tokenOutDenom})
+	// We do not allow unlisted denoms to maintain
+	// compatibility with the /quote endpoint.
+	doesAllowUnlistedDenoms := false
+	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, []string{tokenIn, tokenOutDenom}, doesAllowUnlistedDenoms)
 	if err != nil {
 		return c.JSON(domain.GetStatusCode(err), domain.ResponseError{Message: err.Error()})
 	}

tokens/delivery/http/tokens_delivery.go

@@ -141,7 +141,9 @@ func (a *TokensHandler) GetPoolDenomMetadata(c echo.Context) (err error) {
 
 	denoms := strings.Split(denomsStr, ",")
 	// Validate denom parameters and convert to chain denoms if necessary.
-	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, denoms)
+	// We allow unlisted denoms since this is a debug endpoint.
+	doesAllowUnlistedDenoms := true
+	chainDenoms, err := mvc.ValidateChainDenomsQueryParam(c, a.TUsecase, denoms, doesAllowUnlistedDenoms)
 	if err != nil {
 		return c.JSON(http.StatusBadRequest, domain.ResponseError{Message: err.Error()})
 	}

tokens/usecase/tokens_usecase.go

@@ -406,18 +406,8 @@ func (t *tokensUseCase) RegisterPricingStrategy(source domain.PricingSourceType,
 
 // IsValidChainDenom implements mvc.TokensUsecase.
 func (t *tokensUseCase) IsValidChainDenom(chainDenom string) bool {
-	metaData, ok := t.tokenMetadataByChainDenom.Load(chainDenom)
-	if !ok {
-		return false
-	}
-
-	v, ok := metaData.(domain.Token)
-	if !ok {
-		return false
-	}
-
-	// is valid only if token is found and is not unlisted
-	return !v.IsUnlisted
+	_, ok := t.tokenMetadataByChainDenom.Load(chainDenom)
+	return ok
 }
 
 // GetMinPoolLiquidityCap implements mvc.TokensUsecase.
@@ -466,3 +456,18 @@ func (t *tokensUseCase) GetCoingeckoIdByChainDenom(chainDenom string) (string, e
 
 	return v, nil
 }
+
+// IsValidListedDenom implements mvc.TokensUsecase.
+func (t *tokensUseCase) IsValidListedDenom(denom string) bool {
+	metaData, ok := t.tokenMetadataByChainDenom.Load(denom)
+	if !ok {
+		return false
+	}
+
+	v, ok := metaData.(domain.Token)
+	if !ok {
+		return false
+	}
+
+	return !v.IsUnlisted
+}