Refactor merge http servlets - #20
Open
Sunny6889 wants to merge 21 commits into
Open
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Little-Peony
force-pushed
the
refactor_merge_http_servlets
branch
from
August 24, 2026 09:33
c513391 to
124abb6
Compare
Replace the hand-written HttpApiDef enum with HttpApiRegistry, built at class-load from a single @httpapi(value, access, surfaces) declaration on each servlet. The four http services mount from the derived registry, so an endpoint is declared once and every surface stays in sync by construction. - @httpapi is never @inherited and is read only via getDeclaredAnnotation, so a servlet subclass can never inherit its parent's exposure. - Every concrete servlet in the package must declare @httpapi or @HttpApiExcluded; the registry fails the boot otherwise, before any Jetty bind, and also refuses to boot on an empty scan. - The API x surface x access audit matrix is generated and checked against a committed snapshot; an independent fixture of the pre-refactor hand-written routes is the parity baseline, so the derived table cannot validate itself. That baseline caught five shielded read endpoints (getmerkletreevoucherinfo, scanandmarknotebyivk, scannotebyivk, scannotebyovk, isspend) that the enum had dropped from the PBFT surface though they are live on develop; restored. Tests: HttpApiRegistryTest with negative fixtures for each invariant, plus WalletCursorFilterTest and CursorFilterInstallationTest pinning the read-cursor filter behaviour and installation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Little-Peony
force-pushed
the
refactor_merge_http_servlets
branch
from
August 24, 2026 09:58
124abb6 to
04a92f7
Compare
createshieldedcontractparameters and its withoutask variant compose an unsigned transaction via wallet.createShieldedContractParameters*, so they are BUILD endpoints; they were mislabeled Access.READ.
Replace the hand-written HttpApiDef enum with HttpApiRegistry, built at class-load from a single @httpapi(value, access, surfaces) declaration on each servlet. The four http services mount from the derived registry, so an endpoint is declared once and every surface stays in sync by construction. - @httpapi is never @inherited and is read only via getDeclaredAnnotation, so a servlet subclass can never inherit its parent's exposure. - Every concrete servlet in the package must declare @httpapi or @HttpApiExcluded; the registry fails the boot otherwise, before any Jetty bind, and also refuses to boot on an empty scan. - The API x surface x access audit matrix is generated and checked against a committed snapshot; an independent fixture of the pre-refactor hand-written routes is the parity baseline, so the derived table cannot validate itself. Five sapling shielded note-scan endpoints (getmerkletreevoucherinfo, scanandmarknotebyivk, scannotebyivk, scannotebyovk, isspend) were live on develop's PBFT port only while disabled on every other surface; they stay disabled here too (@HttpApiExcluded), aligning all four surfaces. The parity test records this as an intentional PBFT removal vs the develop baseline. Tests: HttpApiRegistryTest with negative fixtures for each invariant, plus WalletCursorFilterTest and CursorFilterInstallationTest pinning the read-cursor filter behaviour and installation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Migration correctness is verified by diffing against upstream, so the repo no longer carries the frozen fixtures: remove pre-refactor-routes.txt and api-audit-matrix.txt, their two tests in HttpApiRegistryTest, and the now unused HttpApiRegistry.auditMatrix(). The rule-based invariant tests (non-READ is FULL-only, @inherited guard, completeness net, suffix and surface rejections) stay. Also adds nested / whitespace / wildcard probe fixtures under regtest.
An adversarial review of HttpApiRegistry found four ways a mistaken annotation could reach a running node instead of failing the boot. - Suffix syntax. The suffix is concatenated into a jetty path spec, but only '/' and blank were rejected. "*" passed validation and mounted the servlet as the prefix wildcard /wallet/*, swallowing every sibling endpoint and, because a wildcard match rewrites getServletPath(), slipping past the filters that key off it; a suffix carrying whitespace was checked with trim() but stored raw, mounting an endpoint at a path no client can request. Both are now rejected by a single path-token rule [A-Za-z0-9_.-]+, which all 120 live endpoints already satisfy (four of them are camelCase, so the rule is not lowercase-only). - Unmountable endpoint declarations. A servlet that is abstract, or nested inside another class, was dropped before its annotations were read, so an @httpapi on one produced no endpoint and no error — the silent omission this registry exists to prevent, and invisible to the completeness test because it iterated the same filtered list. Such classes are now scanned and must declare neither annotation. - Spring stereotypes. The @component check only accepted the literal annotation, so a servlet marked with a meta-annotated stereotype would have aborted the boot despite being a valid bean. It now uses get semantics (direct + meta, never inherited from a superclass). - Boot failure path. HttpService.start() calls addServlet() synchronously, so a registry failure escaped ServiceContainer's TronError wrapper and never reached the logged System.exit. Registry failures are now raised as TronError(API_SERVER_INIT); validation itself still throws IllegalStateException so it stays unit-testable. Adds negative tests for the wildcard, whitespace and nested cases, and asserts every live suffix satisfies the path-token rule.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
refer
tronprotocol#6922
Why are these changes required?
This PR has been tested by:
Follow up
Extra details