server/v2/api/grpcgateway/uri_test.go (3)

15-92: Consider adding more edge cases to TestMatchURI

While the test cases are comprehensive for valid scenarios, consider adding:

1. Test case for invalid URL parsing
2. Test case for malformed wildcard patterns (e.g., {foo=**/*})
3. Test case for overlapping wildcard patterns

 testCases := []struct {
     name     string
     uri      string
     mapping  map[string]string
     expected *uriMatch
 }{
+    {
+        name:     "invalid URL",
+        uri:      "http://[::1]:8080:invalid",
+        mapping:  map[string]string{"/foo/bar": "query.Bank"},
+        expected: nil,
+    },
+    {
+        name:     "malformed wildcard pattern",
+        uri:      "https://localhost:8080/foo/bar",
+        mapping:  map[string]string{"/foo/{bar=**/*}": "query.Bank"},
+        expected: nil,
+    },

---

94-100: Add nil map test case to TestURIMatch_HasParams

Add a test case to verify behavior when Params is nil.

 func TestURIMatch_HasParams(t *testing.T) {
     u := uriMatch{Params: map[string]string{"foo": "bar"}}
     require.True(t, u.HasParams())

     u = uriMatch{}
     require.False(t, u.HasParams())
+
+    u = uriMatch{Params: nil}
+    require.False(t, u.HasParams())
 }

---

102-125: Enhance DummyProto implementation

The DummyProto implementation could be more robust:

1. String() method should return a meaningful string representation
2. Reset() method should actually reset the fields

-func (d DummyProto) Reset() {}
+func (d *DummyProto) Reset() {
+    *d = DummyProto{}
+}

-func (d DummyProto) String() string { return dummyProtoName }
+func (d DummyProto) String() string {
+    return fmt.Sprintf("DummyProto{Foo: %s, Bar: %v, Baz: %d}", d.Foo, d.Bar, d.Baz)
+}

server/v2/api/grpcgateway/server.go (2)

41-41: Enhance documentation for the new parameter.
The added parameter “appManager” significantly alters the function signature. Consider elaborating on its usage in the function docstring so that new contributors quickly understand its role.

---

142-143: Use standard naming conventions for gRPC metadata constants.
Consider prefixing the constant with a recognized prefix (e.g., “Grpc” or “GRPC”), or confirm it's consistent with any existing naming patterns from the rest of the codebase.

server/v2/api/grpcgateway/interceptor.go (4)

24-37: Ensure that gatewayInterceptor’s fields are documented.
Although there are short comments, we might consider more elaboration about concurrency and usage patterns, especially for public fields or ones that can be accessed outside the package.

---

39-51: Handle partial failures while building customEndpointMapping.
The current logic returns an error if getHTTPGetAnnotationMapping() fails entirely. If partial failures or missing proto definitions are possible, consider how best to proceed to avoid shutting down the entire server.

---

83-85: Check large header inputs.
When parsing block height in the header, consider if large or malicious values could impact the system. You may want to bound-check the height.

---

95-105: Log fallback for clarity.
When no handler is found in the app manager, you fall back to the gateway. Consider logging this scenario for troubleshooting.

server/v2/api/grpcgateway/uri.go (3)

20-31: Improve docstring for Params usage.
The field “Params” is central to the dynamic message creation. It might be helpful to clarify that the keys come from both path wildcards and query parameters.

---

38-85: Consider partial or incremental matching logic.
matchURL uses a simple loop with direct regex compilation. If the mapping grows large, this might become a bottleneck. Investigate indexing strategies or precomputed structures for better performance.

---

114-134: Validate JSON input size or structure.
Use caution under large or malformed JSON bodies, since you only limit the size to 1MB. This might still be large enough for certain denial-of-service conditions. If feasible, consider streaming or chunked processing if extremely large payloads are expected.

server/v2/go.mod (1)

37-37: Pin dependency version carefully.
The module google.golang.org/genproto/googleapis/api is pinned to a pseudo-version. If an officially tagged version is available, consider using that to avoid potential build reproducibility issues.

CHANGELOG.md (3)

Line range hint 1-1: Add version table of contents for better navigation

Consider adding a table of contents at the start of the changelog to help users quickly navigate to specific versions.

# Changelog

## Table of Contents
- [v0.47.0](#v0470)
- [v0.46.x](#v046x) 
- [v0.45.x](#v045x)
...

---

47-47: Fix typo in changelog entry

Change "feat(server/v2)!: grpcgateway autoregistration" to "feat(server/v2)!: gRPC gateway auto-registration" for better readability and consistent capitalization.

---

Line range hint 1-2000: Improve version header formatting consistency

The version headers have inconsistent formatting - some use H2 (##) while others use H3 (###). Consider standardizing all version headers to H2 level for better visual hierarchy.

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

#!/bin/bash
# Confirm usage of appmanager in server/v2/api/grpcgateway/server.go only
rg 'appmanager' -A 5

#!/bin/bash
# Check how appmanager is used in the file under review
rg -A 10 'appmanager' server/v2/api/grpcgateway/server.go

#!/bin/bash
# Grep for shared variables in gatewayInterceptor that might be mutated
rg -A 8 'type gatewayInterceptor'

#!/bin/bash
# Check the ServeHTTP implementation and any methods that might modify shared state
ast-grep --pattern 'func (i *gatewayInterceptor[$_]) ServeHTTP($_) {
  $$$
}'

# Also check if there are any methods that modify the customEndpointMapping
rg -A 5 'customEndpointMapping\[.*\]'

#!/bin/bash
# Check the entire interceptor.go file to see the implementation
cat server/v2/api/grpcgateway/interceptor.go

# Also check for any modifications to the map in the codebase
rg -g '*.go' 'customEndpointMapping.*='

-for getPattern, queryInputName := range getPatternToQueryInputName {
-    regexPattern, wildcardNames := patternToRegex(getPattern)
-    regex := regexp.MustCompile(regexPattern)
-    ...
+var compiledRegexes = map[string]*regexp.Regexp{}
+for getPattern, queryInputName := range getPatternToQueryInputName {
+    regexPattern, wildcardNames := patternToRegex(getPattern)
+    if _, exists := compiledRegexes[regexPattern]; !exists {
+        compiledRegexes[regexPattern] = regexp.MustCompile(regexPattern)
+    }
+    regex := compiledRegexes[regexPattern]
+    ...

#!/bin/bash
# Spot-check fallback usage in the codebase to confirm there's no mismatch
rg 'ServeHTTP' -A 5

#!/bin/bash
# Check matchURL implementation and customEndpointMapping usage
ast-grep --pattern 'func matchURL($_, $_) $_'

# Check error handling patterns
rg 'DefaultHTTPProtoErrorHandler' -A 3

#!/bin/bash
# Confirm references to stf in the entire server/v2 subtree
fd --extension go --exec rg 'server/v2/stf'

#!/bin/bash
# Let's find the actual Go files that import and use the stf package
rg -l "cosmossdk.io/server/v2/stf" --type go

# Also check for any actual usage patterns of stf
rg "stf\." --type go -A 2 -B 2

#!/bin/bash
# Search for all calls to grpcgateway.New to ensure they're updated
rg -A 2 "grpcgateway\.New\[.*\]"

#!/bin/bash
# Let's check the implementation of grpcgateway.New to understand the breaking change
ast-grep --pattern 'func New[$_]($$$) ($$$)'

#!/bin/bash
# Let's check for any other instances of grpcgateway.New to ensure all calls are updated
rg -B 5 -A 5 "grpcgateway\.New"

#!/bin/bash
# Search for similar height parameter validations
ast-grep --pattern 'cannot parse $_height as uint: strconv.ParseUint'

#!/bin/bash
# Let's search for error messages related to height validation using ripgrep
rg -i "height.*strconv\.ParseUint|ParseUint.*height" -A 2 -B 2

#!/bin/bash
# Search for test files containing height validation tests
fd ".*test.*\.go$" --exec rg -l "height.*invalid"

#!/bin/bash
# Search for error message strings containing height validation
rg "cannot parse.*height" -A 2 -B 2

server/v2/api/grpcgateway/interceptor.go (3)

39-50: Validate error handling for getHTTPGetAnnotationMapping.
The function gracefully returns an error if the annotation mapping retrieval fails. This approach is good, but ensure calling code logs relevant context for debugging.

---

83-92: Check user-supplied block height.
Great validation of user-supplied block height. Make sure to handle negative or extremely large values gracefully.

---

94-104: Brace for potential error types.
Falling back to the gateway mux when "no handler" is found is appropriate. For more robust error handling, consider typed errors rather than matching strings.

server/v2/api/grpcgateway/server.go (1)

79-83: Interceptor creation is well-placed.
This intercepts all requests via mux.Handle and ensures a single source of truth for custom routing.

Consider adding more explanatory logs so operators can distinguish fallback vs. custom interception in logs.

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

server/v2/api/grpcgateway/uri_test.go (2)

15-92: Consider adding edge cases to TestMatchURI

While the test cases are comprehensive, consider adding these scenarios:

1. Empty URI path
2. URI with special characters in parameters (e.g., URL-encoded values)
3. URI with multiple catch-all wildcards (to verify proper handling of invalid patterns)

 func TestMatchURI(t *testing.T) {
     testCases := []struct {
         name     string
         uri      string
         mapping  map[string]string
         expected *uriMatch
     }{
+        {
+            name:     "empty path",
+            uri:      "https://localhost:8080",
+            mapping:  map[string]string{"/": "query.Root"},
+            expected: &uriMatch{QueryInputName: "query.Root", Params: map[string]string{}},
+        },
+        {
+            name:     "special characters in parameters",
+            uri:      "https://localhost:8080/foo/bar?q=hello%20world&type=%2Fbank%2Fsend",
+            mapping:  map[string]string{"/foo/bar": "query.Bank"},
+            expected: &uriMatch{QueryInputName: "query.Bank", Params: map[string]string{"q": "hello world", "type": "/bank/send"}},
+        },
+        {
+            name:     "multiple catch-all wildcards should not match",
+            uri:      "https://localhost:8080/foo/bar/baz",
+            mapping:  map[string]string{"/foo/{bar=**}/{baz=**}": "query.Invalid"},
+            expected: nil,
+        },

---

94-100: Add nil map test case to TestURIMatch_HasParams

Add a test case for nil params map to ensure robust handling.

 func TestURIMatch_HasParams(t *testing.T) {
     u := uriMatch{Params: map[string]string{"foo": "bar"}}
     require.True(t, u.HasParams())

     u = uriMatch{}
     require.False(t, u.HasParams())
+
+    u = uriMatch{Params: nil}
+    require.False(t, u.HasParams())
 }

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

 func TestCreateMessage(t *testing.T) {
+    t.Cleanup(func() {
+        gogoproto.RegisterType(nil, dummyProtoName)
+    })
     gogoproto.RegisterType(&DummyProto{}, dummyProtoName)
     
     testCases := []struct {
         // ... existing test cases ...
+        {
+            name: "invalid nested field path",
+            uri: uriMatch{
+                QueryInputName: dummyProtoName,
+                Params: map[string]string{"page..nest.foo": "5"},
+            },
+            expErr: true,
+        },
+        {
+            name: "non-existent field",
+            uri: uriMatch{
+                QueryInputName: dummyProtoName,
+                Params: map[string]string{"nonexistent": "value"},
+            },
+            expErr: true,
+        },
+        {
+            name: "invalid nested field type",
+            uri: uriMatch{
+                QueryInputName: dummyProtoName,
+                Params: map[string]string{"page.nest.foo": "not_a_number"},
+            },
+            expErr: true,
+        },

 func TestCreateMessageFromJson(t *testing.T) {
+    t.Cleanup(func() {
+        gogoproto.RegisterType(nil, dummyProtoName)
+    })
     gogoproto.RegisterType(&DummyProto{}, dummyProtoName)
     
     testCases := []struct {
         // ... existing test cases ...
+        {
+            name: "empty request body",
+            uri:  uriMatch{QueryInputName: dummyProtoName},
+            request: func() *http.Request {
+                return &http.Request{Body: io.NopCloser(bytes.NewReader(nil))}
+            },
+            expErr: true,
+        },
+        {
+            name: "invalid field types",
+            uri:  uriMatch{QueryInputName: dummyProtoName},
+            request: func() *http.Request {
+                return &http.Request{Body: io.NopCloser(bytes.NewReader([]byte(`{"baz":"not_a_number"}`)))}
+            },
+            expErr: true,
+        },