Fix/dividends and docs issues - #323
Merged
valoryyaa-byte merged 5 commits intoAug 31, 2026
Merged
Conversation
dividends::list took Path<u64> for the asset id only, so there was no way to fetch a single distribution and nothing exercised a malformed distribution-id path segment the way assets.rs covers a malformed asset id. Adds dividends::get_one(Path<(u64, u64)>), which 404s when the asset or the distribution is unknown, and mounts it at GET /assets/:id/dividends/:did. A non-numeric :did (e.g. /assets/1/dividends/abc) is rejected with 400 by axum's path extractor before the handler runs, mirroring the existing :id behavior; adds a router-level test for that plus the two 404 paths. Also restores the metrics/metrics-exporter-prometheus dependency and imports in Cargo.toml, main.rs and indexer/mod.rs, dropped by a bad merge conflict resolution in 59c9fdd — the crate referenced PrometheusHandle/PrometheusBuilder without them, so it (and any test target) could not compile. Adds tower/http-body-util dev-dependencies and a routes::test_support helper for building a router-testable AppState without a live Soroban RPC or the global Prometheus recorder. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TKebYLnJ68V8moDpmN8bfa
…i.json The router mounted all data routes unprefixed (/stats, /assets, ...) while nothing versioned the API, so there was no way to evolve the route shape without breaking every existing consumer in place. Nests all snapshot-backed routes under /v1 (GET / , /health and /metrics stay unversioned), and adds docs/public/openapi.json documenting the /v1 paths, request/response schemas, and errors, so a generated client (RWA-ToolKit#262) matches the real routes. Updates every curl example and <ApiEndpoint> path across the docs site (getting-started, integration guide, and the assets/holders/compliance/dividends/ overview API pages) to the /v1 prefix, and documents the new GET /v1/assets/:id/dividends/:did endpoint. Router paths, the root index's endpoint list, and the OpenAPI spec now share one list (DATA_ROUTE_PATHS in routes/mod.rs) as their source of truth. Adds a test asserting the spec's "paths" keys equal that list (converted to /v1/{id}-style) and that every documented path resolves to a live handler on the router, to prevent this drift from recurring. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TKebYLnJ68V8moDpmN8bfa
Existing coverage for GET /assets/:id/dividends/:did only exercised an unknown distribution id and an unknown asset id in the middle of the range. Distribution ids come from on-chain state, so a malicious or malformed id should 404 cleanly rather than panic or behave unexpectedly at the u64 boundaries — the same guarantee already required for asset ids (RWA-ToolKit#194/RWA-ToolKit#210). Adds boundary tests for did=0 and did=u64::MAX against both a known asset (exercising the "asset found, distribution not found" branch) and an unknown asset (exercising the "asset not found" branch first). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TKebYLnJ68V8moDpmN8bfa
The compliance summary docs listed total_records and the status fields (approved/suspended/rejected/pending) without noting they can diverge: total_records counts every allowlisted address, but the indexer only increments a status field when that address's KYC record is successfully read and parsed in the current cycle (api/src/indexer/mod.rs: total_records increments unconditionally per address, the status match only runs inside the record read's Ok/Some branch). A reader could otherwise assume the four status counts sum to total_records and treat a gap as a bug. Adds a callout mirroring the existing approved-vs-on-chain-is_allowed clarification (RWA-ToolKit#114) so consumers don't build that assumption in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TKebYLnJ68V8moDpmN8bfa
|
@Valreb001 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
Closes #303
Closes #304
Closes #305
Closes #310
SUMMARY
GET /assets/:id/dividends/:didand test non-numeric:did** —added
dividends::get_one, which 404s on an unknown asset or distribution,plus a router-level test that a non-numeric
:did(e.g./assets/1/dividends/abc) returns 400, mirroring the existing:idcoverage./v1and adddocs/public/openapi.json** —moved all snapshot-backed routes under
/v1, added an OpenAPI specdocumenting them, updated the docs site's examples to match, and added a
test asserting the spec's paths match the router's mounted routes.
did=0anddid=u64::MAXfordividends::get_one** —added boundary tests so malformed or malicious distribution ids 404 cleanly
at both ends of the
u64range, matching existing asset-id boundary coverage.total_recordsvs compliance status field counts** —documented that
total_recordscounts every allowlisted address whileapproved/suspended/rejected/pendingonly count addresses whose KYCrecord was read successfully that cycle, so the two should not be assumed
to sum to the same total.