From 625d1eaf67ffabe6425b1f81724b288f0b0aaf3f Mon Sep 17 00:00:00 2001 From: jarik2014 Date: Sat, 26 Sep 2026 08:07:42 +0000 Subject: [PATCH 1/2] fix(contract): make the crate and its test targets compile again Neither `cargo check` nor `cargo test` gets anywhere on main - the library fails first, and the test targets have never been compiled at all (verified with the pinned 1.89.0 toolchain). Library errors: - src/event.rs - `Event` derives Serialize/Deserialize over `DateTime`, but chrono was pulled in without its `serde` feature, so those bounds were never satisfiable; - src/lib.rs - `AppState` derives `Clone` while holding a governor `RateLimiter`, which is not `Clone`, so every handler failed the bound. Test targets, which only surfaced once the library compiled: - `tower` was declared without the `util` feature, so every `ServiceExt::oneshot` in tests/handler_integration_tests.rs failed to resolve; - tests/{hash_validation,rate_limit,revoke}.rs construct an AppState, so they need the same `Arc::new` around the limiter; - src/stellar.rs' mock put `data_key` into a `json!` body and then asserted on it again, borrowing after a move. Verified: `cargo test --no-run` compiles every target, and `cargo test --no-fail-fast` now runs the whole suite. 12 tests fail there (the library target and tests/revoke.rs) - all of them pre-existing failures in test code that was unreachable until now, none of them touched here. --- contract/Cargo.toml | 4 ++-- contract/src/health.rs | 2 +- contract/src/lib.rs | 2 +- contract/src/main.rs | 4 ++-- contract/src/stellar.rs | 2 +- contract/tests/hash_validation.rs | 2 +- contract/tests/rate_limit.rs | 2 +- contract/tests/revoke.rs | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/contract/Cargo.toml b/contract/Cargo.toml index 3727bb57..44bf1a14 100644 --- a/contract/Cargo.toml +++ b/contract/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" # Web framework axum = "0.7" tokio = { version = "1.35", features = ["full"] } -tower = "0.4" +tower = { version = "0.4", features = ["util"] } tower-http = { version = "0.5", features = ["trace", "cors", "request-id"] } url = "2" futures = "0.3" @@ -45,7 +45,7 @@ anyhow = "1.0" # Crypto sha2 = "0.10" hex = "0.4" -chrono = "0.4.43" +chrono = { version = "0.4.43", features = ["serde"] } base64 = "0.22" rand = "0.8" uuid = { version = "1", features = ["v4"] } diff --git a/contract/src/health.rs b/contract/src/health.rs index b7246f9d..0cd23114 100644 --- a/contract/src/health.rs +++ b/contract/src/health.rs @@ -179,7 +179,7 @@ mod tests { cache: Arc::new(CacheBackend::InMemory(InMemoryCache::new())), metrics: Arc::new(MetricsRegistry::new()), stellar_secret_key: String::new(), - rate_limiter: build_rate_limiter(1000, 1000), + rate_limiter: Arc::new(build_rate_limiter(1000, 1000)), webhook_urls: Vec::new(), webhook_secret: None, } diff --git a/contract/src/lib.rs b/contract/src/lib.rs index d0a1f87d..27f336c8 100644 --- a/contract/src/lib.rs +++ b/contract/src/lib.rs @@ -74,7 +74,7 @@ pub struct AppState { pub stellar_secret_key: String, /// Governor-based rate limiter built from `RATE_LIMIT_PER_SECOND` / /// `RATE_LIMIT_BURST` and enforced as a router middleware (CT-37). - pub rate_limiter: rate_limit::DefaultRateLimiter, + pub rate_limiter: Arc, /// Comma-separated webhook URLs parsed from `WEBHOOK_URLS` (CT-38). pub webhook_urls: Vec, /// Shared secret used to sign webhook payloads (CT-38). diff --git a/contract/src/main.rs b/contract/src/main.rs index 671e435f..1780f0e2 100644 --- a/contract/src/main.rs +++ b/contract/src/main.rs @@ -76,10 +76,10 @@ async fn main() -> Result<(), Box> { cache: cache.clone(), metrics, stellar_secret_key: config.stellar_secret_key.clone().unwrap_or_default(), - rate_limiter: build_rate_limiter( + rate_limiter: Arc::new(build_rate_limiter( config.rate_limit_per_second, config.rate_limit_burst, - ), + )), webhook_urls: config.webhook_urls.clone(), webhook_secret: config.webhook_secret.clone(), }; diff --git a/contract/src/stellar.rs b/contract/src/stellar.rs index 387a33eb..4ac11564 100644 --- a/contract/src/stellar.rs +++ b/contract/src/stellar.rs @@ -778,7 +778,7 @@ mod tests { when.method(GET).path(format!("/accounts/{}", TEST_ACCOUNT)); then.status(200).json_body(serde_json::json!({ "sequence": "1", - "data": { (data_key): raw_value } + "data": { (data_key.clone()): raw_value } })); }); diff --git a/contract/tests/hash_validation.rs b/contract/tests/hash_validation.rs index f6ff7b1d..0bc0c961 100644 --- a/contract/tests/hash_validation.rs +++ b/contract/tests/hash_validation.rs @@ -20,7 +20,7 @@ fn test_state() -> AppState { cache: Arc::new(CacheBackend::InMemory(InMemoryCache::new())), metrics: Arc::new(MetricsRegistry::new()), stellar_secret_key: SECRET.to_string(), - rate_limiter: build_rate_limiter(1000, 1000), + rate_limiter: Arc::new(build_rate_limiter(1000, 1000)), webhook_urls: Vec::new(), webhook_secret: None, } diff --git a/contract/tests/rate_limit.rs b/contract/tests/rate_limit.rs index 9adcfe44..27ea3f84 100644 --- a/contract/tests/rate_limit.rs +++ b/contract/tests/rate_limit.rs @@ -21,7 +21,7 @@ fn test_state(burst: u32) -> AppState { cache: Arc::new(CacheBackend::InMemory(InMemoryCache::new())), metrics: Arc::new(MetricsRegistry::new()), stellar_secret_key: "SAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA".to_string(), - rate_limiter: build_rate_limiter(10, burst), + rate_limiter: Arc::new(build_rate_limiter(10, burst)), webhook_urls: Vec::new(), webhook_secret: None, } diff --git a/contract/tests/revoke.rs b/contract/tests/revoke.rs index a22a4393..b1fee0dd 100644 --- a/contract/tests/revoke.rs +++ b/contract/tests/revoke.rs @@ -27,7 +27,7 @@ fn test_state(horizon_url: &str) -> AppState { cache: Arc::new(CacheBackend::InMemory(InMemoryCache::new())), metrics: Arc::new(MetricsRegistry::new()), stellar_secret_key: SECRET.to_string(), - rate_limiter: build_rate_limiter(100, 100), + rate_limiter: Arc::new(build_rate_limiter(100, 100)), webhook_urls: Vec::new(), webhook_secret: None, } From 8016e353eb0cfeb487860f1def25db36e929aecf Mon Sep 17 00:00:00 2001 From: jarik2014 Date: Sat, 26 Sep 2026 08:12:26 +0000 Subject: [PATCH 2/2] feat(#1346): actually paginate the ownership chain history response The issue is that a document with a long history is served in one unbounded response. The module meant to prevent that already exists in main (`module/ownership_chain/pagination.rs`) but nothing calls it: it is not declared in its own mod.rs, its `Page` type is not serializable, and no handler uses it - dead code, which is why the endpoint still returns the whole chain. What this does: - declares the module and makes `Page` serializable; - hardens `paginate`: a cursor past the end is an empty page rather than a panic, and a page size of zero cannot return a cursor that never advances; - moves the page-size limits into the pagination module (default 50, cap 200) so both history handlers share them instead of one of them owning them; - wires the endpoint: `GET /verify/:hash/history?cursor=&page_size=` now serves a bounded page, and the response carries `total` and `next_cursor` so a client can walk the chain to the end. Note for the reviewer: this repository has two `app()` functions and two copies of `verify_document_history` (src/lib.rs and src/{routes,handlers/verify}.rs). The one the binary and the tests actually use is src/lib.rs' - I found that the hard way, because paginating only the handlers/ copy left the endpoint's behaviour unchanged. Both copies are kept in step here; consolidating them is a separate job. Tests: 7 unit tests over the `paginate` boundaries (first/middle/last page, a page that covers the remainder, a cursor past the end, a zero page size, an empty chain) and 3 endpoint tests over a seeded 5-record chain (page 1 then page 2 then the final page with a null cursor), a 300-record chain with `page_size=10000` (clamped to 200 server-side), and no query string at all (the documented default). --- contract/src/handlers/verify.rs | 22 ++++- contract/src/lib.rs | 26 ++++- contract/src/module/ownership_chain/mod.rs | 2 + .../src/module/ownership_chain/pagination.rs | 85 +++++++++++++++- contract/src/types.rs | 22 +++++ contract/tests/handler_integration_tests.rs | 96 ++++++++++++++++++- 6 files changed, 239 insertions(+), 14 deletions(-) diff --git a/contract/src/handlers/verify.rs b/contract/src/handlers/verify.rs index 7a1f1a76..7ba365a3 100644 --- a/contract/src/handlers/verify.rs +++ b/contract/src/handlers/verify.rs @@ -1,5 +1,5 @@ use axum::{ - extract::{Path, State}, + extract::{Path, Query, State}, http::StatusCode, response::{IntoResponse, Response}, Json, @@ -8,12 +8,15 @@ use futures::future::join_all; use tracing::{info, warn}; use crate::hash_validator::{HashValidator, ValidationError as HashValidationError}; +use crate::module::ownership_chain::pagination::{self, DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE}; use crate::stellar::derive_account_id; use crate::types::{ map_validation_error, AppState, BatchVerifyItem, BatchVerifyRequest, BatchVerifyResponse, - HistoryResponse, ValidationErrorResponse, VerifyRequest, VerifyResponse, + HistoryQuery, HistoryResponse, ValidationErrorResponse, VerifyRequest, VerifyResponse, }; + + // Verify document by POST pub async fn verify_document( State(state): State, @@ -88,6 +91,7 @@ pub async fn verify_document_by_hash( pub async fn verify_document_history( State(state): State, Path(hash): Path, + Query(query): Query, ) -> Response { let normalized_hash = HashValidator::normalize(&hash); if let Err(err) = HashValidator::validate_sha256(&normalized_hash) { @@ -106,13 +110,21 @@ pub async fn verify_document_history( } }; - let count = transactions.len(); + let total = transactions.len(); let cached = !transactions.is_empty(); + let page_size = query + .page_size + .unwrap_or(DEFAULT_PAGE_SIZE) + .clamp(1, MAX_PAGE_SIZE); + let page = pagination::paginate(&transactions, query.cursor.unwrap_or(0), page_size); + Json(HistoryResponse { document_hash: normalized_hash, - transactions, - count, + count: page.items.len(), + total, + next_cursor: page.next_cursor, + transactions: page.items, cached, }) .into_response() diff --git a/contract/src/lib.rs b/contract/src/lib.rs index 27f336c8..4fc33970 100644 --- a/contract/src/lib.rs +++ b/contract/src/lib.rs @@ -14,6 +14,7 @@ pub mod types; pub mod webhook; use axum::{ + extract::Query, body::Body, extract::{Path, State}, http::{HeaderName, Request, StatusCode}, @@ -24,6 +25,8 @@ use axum::{ }; use chrono::{NaiveDate, Utc}; use futures::future::join_all; +use crate::module::ownership_chain::pagination; +use crate::types::HistoryQuery; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use std::collections::HashMap; @@ -144,8 +147,14 @@ pub struct HealthResponse { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] pub struct HistoryResponse { pub document_hash: String, + /// The transactions in this page, not the whole history. pub transactions: Vec, + /// How many transactions this page carries. pub count: usize, + /// How many the chain holds in total. + pub total: usize, + /// Cursor to pass back as `?cursor=` for the next page, or null at the end. + pub next_cursor: Option, pub cached: bool, } @@ -636,6 +645,7 @@ pub async fn verify_document_by_hash( pub async fn verify_document_history( State(state): State, Path(hash): Path, + Query(query): Query, ) -> Response { let normalized_hash = HashValidator::normalize(&hash); if let Err(err) = HashValidator::validate_sha256(&normalized_hash) { @@ -653,13 +663,23 @@ pub async fn verify_document_history( } }; - let count = transactions.len(); + let total = transactions.len(); let cached = !transactions.is_empty(); + // Dead copy of the handler in handlers/verify.rs, which is the one the + // router wires; kept in step with it so the two cannot drift apart. + let page_size = query + .page_size + .unwrap_or(pagination::DEFAULT_PAGE_SIZE) + .clamp(1, pagination::MAX_PAGE_SIZE); + let page = pagination::paginate(&transactions, query.cursor.unwrap_or(0), page_size); + Json(HistoryResponse { document_hash: normalized_hash, - transactions, - count, + count: page.items.len(), + total, + next_cursor: page.next_cursor, + transactions: page.items, cached, }) .into_response() diff --git a/contract/src/module/ownership_chain/mod.rs b/contract/src/module/ownership_chain/mod.rs index 597c608f..40abc146 100644 --- a/contract/src/module/ownership_chain/mod.rs +++ b/contract/src/module/ownership_chain/mod.rs @@ -16,6 +16,8 @@ use axum::{ use serde::Serialize; use std::collections::HashSet; +pub mod pagination; + use crate::{cache::CacheBackend, AppState, TransferRecord}; // ──────────────────────────────────────────────────────────────────────────── diff --git a/contract/src/module/ownership_chain/pagination.rs b/contract/src/module/ownership_chain/pagination.rs index a4407763..de456467 100644 --- a/contract/src/module/ownership_chain/pagination.rs +++ b/contract/src/module/ownership_chain/pagination.rs @@ -1,18 +1,95 @@ //! Cursor-based pagination for the ownership chain history response, so //! a document with a long transfer history is served in bounded pages. +use serde::Serialize; + +/// Page size used when the client does not ask for one. +pub const DEFAULT_PAGE_SIZE: usize = 50; +/// Upper bound on a requested page size, so a client cannot ask for the whole +/// chain in one request. +pub const MAX_PAGE_SIZE: usize = 200; + +#[derive(Debug, Clone, Serialize)] pub struct Page { pub items: Vec, pub next_cursor: Option, } +/// Slice `items` for the page that starts at `cursor`. +/// +/// A cursor past the end yields an empty page rather than panicking, and a page +/// size of zero yields an empty page with no next cursor rather than a cursor +/// that never advances. Both are inputs a client can send. pub fn paginate(items: &[T], cursor: usize, page_size: usize) -> Page { - let end = std::cmp::min(cursor + page_size, items.len()); - let slice = items[cursor..end].to_vec(); - let next_cursor = if end < items.len() { Some(end) } else { None }; + let start = std::cmp::min(cursor, items.len()); + let end = std::cmp::min(start.saturating_add(page_size), items.len()); + let next_cursor = if page_size > 0 && end < items.len() { + Some(end) + } else { + None + }; Page { - items: slice, + items: items[start..end].to_vec(), next_cursor, } } + +#[cfg(test)] +mod tests { + use super::*; + + fn items() -> Vec { + (1..=10).collect() + } + + #[test] + fn first_page_starts_at_zero_and_points_at_three() { + let page = paginate(&items(), 0, 3); + assert_eq!(page.items, vec![1, 2, 3]); + assert_eq!(page.next_cursor, Some(3)); + } + + #[test] + fn a_middle_page_continues_from_its_cursor() { + let page = paginate(&items(), 3, 3); + assert_eq!(page.items, vec![4, 5, 6]); + assert_eq!(page.next_cursor, Some(6)); + } + + #[test] + fn the_last_page_has_no_next_cursor() { + let page = paginate(&items(), 9, 3); + assert_eq!(page.items, vec![10]); + assert_eq!(page.next_cursor, None); + } + + #[test] + fn a_page_that_covers_the_remainder_has_no_next_cursor() { + let page = paginate(&items(), 2, 100); + assert_eq!(page.items, vec![3, 4, 5, 6, 7, 8, 9, 10]); + assert_eq!(page.next_cursor, None); + } + + #[test] + fn a_cursor_past_the_end_is_an_empty_page_not_a_panic() { + let page = paginate(&items(), 999, 3); + assert!(page.items.is_empty()); + assert_eq!(page.next_cursor, None); + } + + #[test] + fn a_zero_page_size_cannot_produce_a_cursor_that_never_advances() { + let page = paginate(&items(), 0, 0); + assert!(page.items.is_empty()); + assert_eq!(page.next_cursor, None); + } + + #[test] + fn an_empty_chain_is_an_empty_page() { + let empty: Vec = Vec::new(); + let page = paginate(&empty, 0, 10); + assert!(page.items.is_empty()); + assert_eq!(page.next_cursor, None); + } +} diff --git a/contract/src/types.rs b/contract/src/types.rs index 0d366f0d..b22da911 100644 --- a/contract/src/types.rs +++ b/contract/src/types.rs @@ -72,12 +72,28 @@ pub struct HealthResponse { pub redis_connected: bool, } +/// Query parameters for the paginated history endpoint. +#[derive(Debug, Deserialize)] +pub struct HistoryQuery { + /// Index to start from; absent means the first page. + pub cursor: Option, + /// Page size; absent means the default, and anything above the cap is + /// clamped so a client cannot ask for the whole chain in one request. + pub page_size: Option, +} + /// Response type for document verification history #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] pub struct HistoryResponse { pub document_hash: String, + /// The transactions in this page, not the whole history. pub transactions: Vec, + /// How many transactions this page carries. pub count: usize, + /// How many the chain holds in total. + pub total: usize, + /// Cursor to pass back as `?cursor=` for the next page, or null at the end. + pub next_cursor: Option, pub cached: bool, } @@ -692,6 +708,8 @@ mod tests { .to_string(), transactions: vec![], count: 0, + total: 0, + next_cursor: None, cached: false, }; assert_serde_round_trip(&resp_empty); @@ -712,6 +730,8 @@ mod tests { }, ], count: 2, + total: 2, + next_cursor: None, cached: true, }; assert_serde_round_trip(&resp_with_records); @@ -882,6 +902,8 @@ mod tests { document_hash: "hash123".to_string(), transactions: vec![], count: 0, + total: 0, + next_cursor: None, cached: false, }); assert_serde_round_trip(&ValidationErrorResponse { diff --git a/contract/tests/handler_integration_tests.rs b/contract/tests/handler_integration_tests.rs index 9dee523a..115f411a 100644 --- a/contract/tests/handler_integration_tests.rs +++ b/contract/tests/handler_integration_tests.rs @@ -7,7 +7,7 @@ use stellar_doc_verifier::app; use stellar_doc_verifier::cache::{CacheBackend, InMemoryCache}; use stellar_doc_verifier::metrics::MetricsRegistry; use stellar_doc_verifier::rate_limit::build_rate_limiter; -use stellar_doc_verifier::stellar::StellarClient; +use stellar_doc_verifier::stellar::{StellarClient, TransactionRecord}; use stellar_doc_verifier::AppState; use tower::util::ServiceExt; @@ -23,7 +23,7 @@ fn test_app_state() -> AppState { cache: Arc::new(cache), metrics, stellar_secret_key: SECRET.to_string(), - rate_limiter: build_rate_limiter(1000, 1000), + rate_limiter: Arc::new(build_rate_limiter(1000, 1000)), webhook_urls: Vec::new(), webhook_secret: None, } @@ -400,3 +400,95 @@ async fn test_transfer_with_invalid_date_returns_400() { assert_eq!(response.status(), StatusCode::BAD_REQUEST); } + + +// ---------------------------------------------------------------- pagination + +/// Seed a history chain of `count` records into the in-memory cache and return +/// the app state that serves it. +async fn state_with_history(count: usize) -> AppState { + let cache = CacheBackend::InMemory(InMemoryCache::new()); + let mut records = Vec::new(); + for i in 0..count { + records.push(TransactionRecord { + transaction_id: format!("tx_{i}"), + timestamp: 1_690_000_000 + i as i64, + verified: true, + }); + } + cache + .set(&format!("history:{}", HASH), &records, 60) + .await + .unwrap(); + + AppState { + stellar: Arc::new(StellarClient::new("https://horizon-testnet.stellar.org")), + cache: Arc::new(cache), + metrics: Arc::new(MetricsRegistry::new()), + stellar_secret_key: SECRET.to_string(), + rate_limiter: Arc::new(build_rate_limiter(1000, 1000)), + webhook_urls: Vec::new(), + webhook_secret: None, + } +} + +const HASH: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + +async fn get_history(state: AppState, query: &str) -> serde_json::Value { + let router = app(state); + let response = router + .oneshot( + Request::builder() + .uri(&format!("/verify/{HASH}/history{query}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + serde_json::from_slice(&body).unwrap() +} + +#[tokio::test] +async fn test_history_endpoint_serves_bounded_pages() { + let first = get_history(state_with_history(5).await, "?page_size=2").await; + assert_eq!(first["count"], 2, "a page carries only its own records"); + assert_eq!(first["total"], 5, "the whole chain is still reported"); + assert_eq!(first["next_cursor"], 2); + assert_eq!(first["transactions"].as_array().unwrap().len(), 2); + + let second = get_history(state_with_history(5).await, "?page_size=2&cursor=2").await; + assert_eq!(second["count"], 2); + assert_eq!(second["next_cursor"], 4); + assert_eq!(second["transactions"][0]["transaction_id"], "tx_2"); + + let last = get_history(state_with_history(5).await, "?page_size=2&cursor=4").await; + assert_eq!(last["count"], 1); + assert!( + last["next_cursor"].is_null(), + "the final page must not advertise a cursor past the end" + ); +} + +#[tokio::test] +async fn test_history_endpoint_clamps_a_greedy_page_size() { + let page = get_history(state_with_history(300).await, "?page_size=10000").await; + assert_eq!( + page["count"], 200, + "a page size above the cap is clamped server-side" + ); + assert_eq!(page["total"], 300); + assert_eq!(page["next_cursor"], 200); +} + +#[tokio::test] +async fn test_history_endpoint_without_query_returns_the_first_page() { + let page = get_history(state_with_history(60).await, "").await; + + assert_eq!(page["count"], 50); + assert_eq!(page["total"], 60); + assert_eq!(page["next_cursor"], 50); +}