From c2f180c9e35027ed6987c071faf0ac41005a1685 Mon Sep 17 00:00:00 2001 From: Banx17 Date: Mon, 28 Sep 2026 22:17:40 +0100 Subject: [PATCH 1/5] fix(remittance_nft): emit parameter-change events from the two config setters (#1146) set_default_burn_threshold and set_min_repayment_amount mutate protocol-critical instance config but published nothing, so off-chain consumers (indexer, webhooks, audit trails) could not observe changes to either risk parameter. Every other admin action in this contract (authorize_minter, revoke_minter, pause, unpause, admin transfer, set_loan_manager) already emits an event. Both setters now capture the outgoing value before the overwrite and publish the full old -> new transition after the state write: set_default_burn_threshold -> topics (DefaultBurnThresholdUpdated, admin) data (old_threshold, new_threshold) set_min_repayment_amount -> topics (MinRepaymentUpdated, admin) data (old_amount, new_amount) The topics/payload shape follows the admin config-update convention already documented in contracts/loan_manager/src/events.rs and already decoded by backend/src/services/eventIndexer.ts for MinRepaymentUpdated, so no indexer change is required. Auth and validation semantics are unchanged: Self::admin(&env) is now bound to a local so it can be reused as the event actor, but it was already evaluated before .require_auth() when the call was inlined, so the evaluation and check order is identical. The publish happens after validation, auth and persistence, and only uses a Symbol topic plus a (u32, u32) / (i128, i128) data tuple, matching existing calls in this file, so it cannot panic. Refs #1146 --- contracts/remittance_nft/src/lib.rs | 42 +++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/contracts/remittance_nft/src/lib.rs b/contracts/remittance_nft/src/lib.rs index 9290637f..26599fa4 100644 --- a/contracts/remittance_nft/src/lib.rs +++ b/contracts/remittance_nft/src/lib.rs @@ -810,15 +810,34 @@ impl RemittanceNFT { Ok(()) } + /// Update the minimum repayment amount accepted by `update_score`. + /// + /// Emits a `MinRepaymentUpdated` event carrying both the previous and the + /// new amount (#1146) so this risk-parameter change is observable off-chain + /// by the indexer and by audit trails. pub fn set_min_repayment_amount(env: Env, amount: i128) { - Self::admin(&env).require_auth(); + // Hoisted so the already-read admin can be reused as the event's actor + // topic. Evaluation order is unchanged: `admin()` is read before + // `require_auth()`, exactly as it was when inlined. + let admin = Self::admin(&env); + admin.require_auth(); if amount < 0 { panic!("negative amount"); } + // Capture the outgoing value before the overwrite so the event reports + // the full old -> new transition rather than just the new value. + let old_amount = Self::min_repayment_amount(&env); env.storage() .instance() .set(&DataKey::MinRepaymentAmount, &amount); Self::bump_instance_ttl(&env); + // Topics `(event, admin)` and data `(old, new)` follow the admin + // config-update convention used by loan_manager/lending_pool so the + // existing indexer decoding of `MinRepaymentUpdated` applies as-is. + env.events().publish( + (Symbol::new(&env, "MinRepaymentUpdated"), admin), + (old_amount, amount), + ); } pub fn get_min_repayment_amount(env: Env) -> i128 { @@ -1208,18 +1227,37 @@ impl RemittanceNFT { Ok(()) } + /// Update the number of defaults after which an NFT is auto-burned. + /// + /// Emits a `DefaultBurnThresholdUpdated` event carrying both the previous + /// and the new threshold (#1146) so this risk-parameter change is + /// observable off-chain by the indexer and by audit trails. pub fn set_default_burn_threshold(env: Env, threshold: u32) -> Result<(), NftError> { if threshold == 0 || threshold > Self::MAX_ALLOWED_BURN_THRESHOLD { return Err(NftError::InvalidThreshold); } - Self::admin(&env).require_auth(); + // Hoisted so the already-read admin can be reused as the event's actor + // topic. Evaluation order is unchanged: `admin()` is read before + // `require_auth()`, exactly as it was when inlined. + let admin = Self::admin(&env); + admin.require_auth(); Self::assert_not_paused(&env)?; + // Capture the outgoing value before the overwrite so the event reports + // the full old -> new transition rather than just the new value. + let old_threshold = Self::default_burn_threshold(&env); env.storage() .instance() .set(&Self::burn_threshold_key(), &threshold); Self::bump_instance_ttl(&env); + // Topics `(event, admin)` and data `(old, new)` follow the admin + // config-update convention used by loan_manager/lending_pool. + env.events().publish( + (Symbol::new(&env, "DefaultBurnThresholdUpdated"), admin), + (old_threshold, threshold), + ); + Ok(()) } From f73237c9289ea89ee10cde49e04e3f4f780b3288 Mon Sep 17 00:00:00 2001 From: Banx17 Date: Mon, 28 Sep 2026 22:18:03 +0100 Subject: [PATCH 2/5] fix(remittance_nft): rename duplicate test that breaks the test target contracts/remittance_nft/src/test.rs defined test_transfer_rejects_burned_destination() twice, so the whole test target of this crate failed to compile and none of its unit tests could run: error[E0428]: the name `test_transfer_rejects_burned_destination` is defined multiple times The two definitions cover different code paths, so both are kept: - the auto-burn path (lowered burn threshold + record_default) is now test_transfer_rejects_auto_burned_destination - the explicit burn() path keeps test_transfer_rejects_burned_destination No test body was changed; the diff is just the name of the first one. The `contracts` CI job is currently a stub that only echoes "Contracts format, clippy, tests, and build passed", which is why this reached main. Refs #1146 --- contracts/remittance_nft/src/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/remittance_nft/src/test.rs b/contracts/remittance_nft/src/test.rs index e1700aa0..8cc47887 100644 --- a/contracts/remittance_nft/src/test.rs +++ b/contracts/remittance_nft/src/test.rs @@ -1227,7 +1227,7 @@ fn test_transfer_rejects_destination_with_existing_state() { } #[test] -fn test_transfer_rejects_burned_destination() { +fn test_transfer_rejects_auto_burned_destination() { // Regression test: transfer only checked has_any_remittance_state(to), // which looks at Metadata/Score only. burn_internal() removes those two // keys but leaves Burned(to) set, so a burned destination previously From cebaef82206488db6652d839360101f1bc9b8fcb Mon Sep 17 00:00:00 2001 From: Banx17 Date: Mon, 28 Sep 2026 22:18:12 +0100 Subject: [PATCH 3/5] test(remittance_nft): assert old/new values in the setter events (#1146) Adds four tests for the events emitted by the two config setters: - test_set_default_burn_threshold_emits_old_and_new_value: performs two real changes (3 -> 5, then 5 -> 9), asserting for each event the topics (DefaultBurnThresholdUpdated, admin) and the data (old, new), plus the persisted value. The second change proves `old` is read from storage rather than hard-coded. - test_set_min_repayment_amount_emits_old_and_new_value: same shape with an i128 (old_amount, new_amount) payload. - test_set_default_burn_threshold_rejected_value_emits_no_event: 0 and MAX_ALLOWED_BURN_THRESHOLD + 1 are rejected and publish nothing, which pins the "emit only after validation" ordering. - test_set_min_repayment_amount_requires_admin_auth: the setter is still admin-only. This setter previously had no test coverage at all. The tests assert topic equality against the expected Symbol/Address tuple and use env.events().all(), matching the existing event assertions for AdminTransferred and Mint in this file. Because the test host only keeps the most recent invocation's events, each capture happens immediately after the setter call, before any other contract call. cargo test -p remittance_nft --lib: 92 passed; 0 failed; 0 ignored (88 pre-existing + these 4). Refs #1146 --- contracts/remittance_nft/src/test.rs | 156 +++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/contracts/remittance_nft/src/test.rs b/contracts/remittance_nft/src/test.rs index 8cc47887..9da4c2dc 100644 --- a/contracts/remittance_nft/src/test.rs +++ b/contracts/remittance_nft/src/test.rs @@ -2880,3 +2880,159 @@ fn test_burn_removes_all_per_user_keys() { Err(Ok(NftError::CommitmentMissing)) ); } + +// ── #1146: admin parameter setter events ───────────────────────────────────── + +/// `set_default_burn_threshold` must publish an event that carries both the +/// previous and the new threshold, using the same `(event, admin)` topic shape +/// and `(old, new)` payload shape as the other admin config-change events. +#[test] +fn test_set_default_burn_threshold_emits_old_and_new_value() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + client.initialize(&admin); + + // initialize() seeds DEFAULT_BURN_THRESHOLD, so the first change is a real + // default -> new transition. + let initial = client.get_default_burn_threshold(); + + // The test host only retains the most recent invocation's events, so the + // event has to be captured before any other contract call is made. + client.set_default_burn_threshold(&5); + let first_events = env.events().all(); + assert_eq!(first_events.len(), 1); + let first_event = first_events.get(0).unwrap(); + let first_topic = Symbol::from_val(&env, &first_event.1.get(0).unwrap()); + let first_topic_1 = Address::from_val(&env, &first_event.1.get(1).unwrap()); + let first_data = <(u32, u32)>::from_val(&env, &first_event.2); + assert_eq!( + first_topic, + Symbol::new(&env, "DefaultBurnThresholdUpdated") + ); + assert_eq!(first_topic_1, admin); + assert_eq!(first_data, (initial, 5u32)); + + // State is persisted only after the event is published, and reading it back + // replaces the event buffer, so it is asserted after the capture above. + assert_eq!(client.get_default_burn_threshold(), 5); + + // A second change proves the `old` value is read from storage before the + // overwrite rather than being a hard-coded default. + client.set_default_burn_threshold(&9); + + let events = env.events().all(); + assert_eq!(events.len(), 1); + let event = events.get(0).unwrap(); + let topic_0 = Symbol::from_val(&env, &event.1.get(0).unwrap()); + let topic_1 = Address::from_val(&env, &event.1.get(1).unwrap()); + let data = <(u32, u32)>::from_val(&env, &event.2); + + assert_eq!(topic_0, Symbol::new(&env, "DefaultBurnThresholdUpdated")); + assert_eq!(topic_1, admin); + assert_eq!(data, (5u32, 9u32)); + assert_eq!(client.get_default_burn_threshold(), 9); +} + +/// `set_min_repayment_amount` must publish an event that carries both the +/// previous and the new amount, using the same convention as above. +#[test] +fn test_set_min_repayment_amount_emits_old_and_new_value() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + client.initialize(&admin); + + let initial = client.get_min_repayment_amount(); + + // The test host only retains the most recent invocation's events, so the + // event has to be captured before any other contract call is made. + client.set_min_repayment_amount(&1_000_000); + let first_events = env.events().all(); + assert_eq!(first_events.len(), 1); + let first_event = first_events.get(0).unwrap(); + let first_topic = Symbol::from_val(&env, &first_event.1.get(0).unwrap()); + let first_topic_1 = Address::from_val(&env, &first_event.1.get(1).unwrap()); + let first_data = <(i128, i128)>::from_val(&env, &first_event.2); + assert_eq!(first_topic, Symbol::new(&env, "MinRepaymentUpdated")); + assert_eq!(first_topic_1, admin); + assert_eq!(first_data, (initial, 1_000_000i128)); + + // State is persisted only after the event is published, and reading it back + // replaces the event buffer, so it is asserted after the capture above. + assert_eq!(client.get_min_repayment_amount(), 1_000_000); + + client.set_min_repayment_amount(&2_500_000); + + let events = env.events().all(); + assert_eq!(events.len(), 1); + let event = events.get(0).unwrap(); + let topic_0 = Symbol::from_val(&env, &event.1.get(0).unwrap()); + let topic_1 = Address::from_val(&env, &event.1.get(1).unwrap()); + let data = <(i128, i128)>::from_val(&env, &event.2); + + assert_eq!(topic_0, Symbol::new(&env, "MinRepaymentUpdated")); + assert_eq!(topic_1, admin); + assert_eq!(data, (1_000_000i128, 2_500_000i128)); + assert_eq!(client.get_min_repayment_amount(), 2_500_000); +} + +/// The event is emitted only after validation succeeds, so a rejected change +/// must leave no parameter-update event behind. +#[test] +fn test_set_default_burn_threshold_rejected_value_emits_no_event() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + client.initialize(&admin); + + // Both rejected calls are the most recent invocation, so an empty event + // buffer proves validation ran before any emission. + assert_eq!( + client.try_set_default_burn_threshold(&0), + Err(Ok(NftError::InvalidThreshold)) + ); + assert_eq!(env.events().all().len(), 0); + + assert_eq!( + client.try_set_default_burn_threshold(&(RemittanceNFT::MAX_ALLOWED_BURN_THRESHOLD + 1)), + Err(Ok(NftError::InvalidThreshold)) + ); + assert_eq!(env.events().all().len(), 0); + + // A rejected change must also leave the stored value untouched. + assert_eq!( + client.get_default_burn_threshold(), + RemittanceNFT::DEFAULT_BURN_THRESHOLD + ); +} + +/// The setter is still admin-only: adding the event emission did not weaken the +/// authorization check. +#[test] +#[should_panic] +fn test_set_min_repayment_amount_requires_admin_auth() { + let env = Env::default(); + let admin = Address::generate(&env); + + let contract_id = env.register(RemittanceNFT, ()); + let client = RemittanceNFTClient::new(&env, &contract_id); + + env.mock_all_auths(); + client.initialize(&admin); + + env.mock_auths(&[]); + client.set_min_repayment_amount(&1_000_000); +} From f261f16f7dd4856fb3db64c9697d637204fc3b4e Mon Sep 17 00:00:00 2001 From: Banx17 Date: Tue, 29 Sep 2026 10:10:06 +0100 Subject: [PATCH 4/5] style(backend): apply the prettier formatting the lint job requires The `backend` CI job fails at its Lint step with 8 prettier/prettier errors, so its Build, Type check and Run tests steps never execute and the whole pipeline stays red. The errors are pre-existing (they come from files unrelated to the contracts change in this branch) but they block this PR from going green, so they are fixed here. `prettier --write` on exactly the four files eslint flagged: - src/__tests__/remittanceFilters.test.ts (1 error, line 42) - src/services/__tests__/auditLogService.pagination.test.ts (2 errors, lines 103/125) - src/services/auditLogService.ts (1 error, line 27) - src/tests/idempotency.namespace.test.ts (4 errors, lines 2/36/78/176) Formatting only: line-wrap decisions, nothing else. No logic, no import added or removed, no assertion touched. Verified with `prettier --check` on the same four files, which now reports "All matched files use Prettier code style!". Refs #1146 --- .../src/__tests__/remittanceFilters.test.ts | 7 ++++--- .../auditLogService.pagination.test.ts | 8 ++----- backend/src/services/auditLogService.ts | 3 +-- .../src/tests/idempotency.namespace.test.ts | 21 +++++++------------ 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/backend/src/__tests__/remittanceFilters.test.ts b/backend/src/__tests__/remittanceFilters.test.ts index 080c3b12..aadc6172 100644 --- a/backend/src/__tests__/remittanceFilters.test.ts +++ b/backend/src/__tests__/remittanceFilters.test.ts @@ -37,9 +37,10 @@ jest.unstable_mockModule('../services/notificationService.js', () => ({ })); jest.unstable_mockModule('../utils/stellarEnvelope.js', () => ({ - parseAndValidateSignedEnvelope: jest - .fn() - .mockReturnValue({ source: 'GCWEPACYJLN7S3ZUXSVMXZBFKYXSHRGZ6O326HDDPDKBKZPXD45XNHC3', signatureCount: 1 }), + parseAndValidateSignedEnvelope: jest.fn().mockReturnValue({ + source: 'GCWEPACYJLN7S3ZUXSVMXZBFKYXSHRGZ6O326HDDPDKBKZPXD45XNHC3', + signatureCount: 1, + }), })); const mockQuery = jest.fn(); diff --git a/backend/src/services/__tests__/auditLogService.pagination.test.ts b/backend/src/services/__tests__/auditLogService.pagination.test.ts index d89cc90a..60645ecb 100644 --- a/backend/src/services/__tests__/auditLogService.pagination.test.ts +++ b/backend/src/services/__tests__/auditLogService.pagination.test.ts @@ -100,9 +100,7 @@ describe('getAuditLogs keyset pagination and totals (#1808)', () => { it('omits the count query unless withTotal is set', async () => { await getAuditLogs({ limit: 2 }); - const countCalls = mockQuery.mock.calls.filter(([text]) => - String(text).includes('COUNT(*)'), - ); + const countCalls = mockQuery.mock.calls.filter(([text]) => String(text).includes('COUNT(*)')); expect(countCalls).toHaveLength(0); }); @@ -122,9 +120,7 @@ describe('getAuditLogs keyset pagination and totals (#1808)', () => { limit: 2, }); - const countCall = mockQuery.mock.calls.find(([text]) => - String(text).includes('COUNT(*)'), - ); + const countCall = mockQuery.mock.calls.find(([text]) => String(text).includes('COUNT(*)')); const countSql = String(countCall?.[0]); const countValues = (countCall?.[1] as unknown[]) ?? []; diff --git a/backend/src/services/auditLogService.ts b/backend/src/services/auditLogService.ts index 1fcfc8d7..a7dfd33b 100644 --- a/backend/src/services/auditLogService.ts +++ b/backend/src/services/auditLogService.ts @@ -24,8 +24,7 @@ export function encodeCursor(row: Record | undefined): string | const id = row.id; const createdAt = row.created_at; if (id === undefined || id === null || !createdAt) return null; - const createdAtIso = - createdAt instanceof Date ? createdAt.toISOString() : String(createdAt); + const createdAtIso = createdAt instanceof Date ? createdAt.toISOString() : String(createdAt); return `${createdAtIso}${CURSOR_SEPARATOR}${String(id)}`; } diff --git a/backend/src/tests/idempotency.namespace.test.ts b/backend/src/tests/idempotency.namespace.test.ts index aedb3af7..3b58d4ed 100644 --- a/backend/src/tests/idempotency.namespace.test.ts +++ b/backend/src/tests/idempotency.namespace.test.ts @@ -1,5 +1,9 @@ import { Request, Response, NextFunction } from 'express'; -import { idempotencyMiddleware, computeFingerprint, namespacedKey } from '../middleware/idempotency.js'; +import { + idempotencyMiddleware, + computeFingerprint, + namespacedKey, +} from '../middleware/idempotency.js'; import { cacheService } from '../services/cacheService.js'; import { jest } from '@jest/globals'; @@ -33,8 +37,7 @@ describe('idempotencyMiddleware key namespacing (#1809)', () => { return request; }; - const cacheKeysRead = () => - asMock(cacheService.get).mock.calls.map(([key]) => String(key)); + const cacheKeysRead = () => asMock(cacheService.get).mock.calls.map(([key]) => String(key)); beforeEach(() => { req = buildRequest(ALICE); @@ -75,11 +78,7 @@ describe('idempotencyMiddleware key namespacing (#1809)', () => { jest.clearAllMocks(); asMock(cacheService.setNotExists).mockResolvedValue(true); - await idempotencyMiddleware( - buildRequest(BOB) as Request, - res as Response, - next, - ); + await idempotencyMiddleware(buildRequest(BOB) as Request, res as Response, next); const bobKey = cacheKeysRead()[0]; expect(aliceKey).not.toBe(bobKey); @@ -173,11 +172,7 @@ describe('idempotencyMiddleware key namespacing (#1809)', () => { jest.clearAllMocks(); asMock(cacheService.setNotExists).mockResolvedValue(true); - await idempotencyMiddleware( - buildRequest(undefined) as Request, - res as Response, - next, - ); + await idempotencyMiddleware(buildRequest(undefined) as Request, res as Response, next); expect(cacheKeysRead()[0]).toBe(first); expect(first).toContain('anon'); From 2fc4fc7a1f23cd5e035a790650034ec3b56264c5 Mon Sep 17 00:00:00 2001 From: Banx17 Date: Tue, 29 Sep 2026 10:23:59 +0100 Subject: [PATCH 5/5] test(backend): repair the 9 stale tests that keep the backend job red The backend job has never reached its Run tests step: Lint failed first, so 9 broken assertions sat undetected on main. With lint fixed they fail, and every one of them is a test-side bug - each implementation is internally consistent with its documented behaviour: auditLogService.pagination.test.ts (#1808) - 'pages with a (created_at, id) ...' called getAuditLogs with no cursor and then asserted the keyset predicate that only a cursor produces. - 'emits a composite nextCursor' split the cursor on the first ':' although the documented format is `${iso}:${id}` and an ISO timestamp contains ':'; it now splits on the last one, exactly as decodeCursor reads it back. - 'resumes correctly from a cursor it previously issued' inspected the first SELECT of the test instead of the resumed one; the pageQuery() helper now returns the most recent page query. - 'counts an unfiltered table as a single plain query' compared against a string without the trailing space the builder interpolates; it now asserts there is no WHERE clause and compares the trimmed statement. - 'keeps the documented filter surface' expected 8 filters, but both AuditLogFilters and the swagger docs expose 7; it now pins those 7 names. idempotency.test.ts (pre-#1809 expectations) - expected `idemp:${key}` and `idemp:${key}:lock`; #1809 namespaces both keys per wallet (this fixture is unauthenticated, hence the `anon` namespace). idempotency.namespace.test.ts (#1809's own tests) - the cache and lock mocks answered identically for every key, which cannot model a keyed store: 'does not replay another wallet's cached response' handed Bob's entry to Alice, and 'does not reject a user with 409' hit Bob's lock. Both mocks are now key-aware and the assertions pin the namespaced key plus the fact that Alice's handler still runs. Verified locally: the three files pass under jest (28 tests) and `eslint .` reports 0 errors / 30 pre-existing warnings. No production code changes. Refs #1808, #1809 --- .../auditLogService.pagination.test.ts | 37 ++++++++++++++----- .../src/tests/idempotency.namespace.test.ts | 36 ++++++++++++------ backend/src/tests/idempotency.test.ts | 9 +++-- 3 files changed, 58 insertions(+), 24 deletions(-) diff --git a/backend/src/services/__tests__/auditLogService.pagination.test.ts b/backend/src/services/__tests__/auditLogService.pagination.test.ts index 60645ecb..89cd8357 100644 --- a/backend/src/services/__tests__/auditLogService.pagination.test.ts +++ b/backend/src/services/__tests__/auditLogService.pagination.test.ts @@ -17,11 +17,12 @@ const PAGE_ROWS = [ { id: '298', created_at: '2026-03-01T00:00:00.000Z' }, ]; -/** Last call to query() — always the SELECT page statement. */ +/** Most recent SELECT page statement issued by getAuditLogs. */ const pageQuery = () => { - const call = mockQuery.mock.calls.find( + const pageCalls = mockQuery.mock.calls.filter( ([text]) => typeof text === 'string' && text.includes('SELECT * FROM audit_logs'), ); + const call = pageCalls[pageCalls.length - 1]; return { text: String(call?.[0]), values: (call?.[1] as unknown[]) ?? [] }; }; @@ -48,7 +49,8 @@ describe('getAuditLogs keyset pagination and totals (#1808)', () => { }); it('pages with a (created_at, id) row comparison, not id alone', async () => { - await getAuditLogs({ limit: 2 }); + // Resume from the composite cursor the previous page ended on. + await getAuditLogs({ limit: 2, cursor: '2026-03-02T00:00:00.000Z:298' }); const { text, values } = pageQuery(); expect(text).toMatch(/\(created_at, id\)\s*<\s*\(\$\d+, \$\d+\)/); @@ -67,11 +69,13 @@ describe('getAuditLogs keyset pagination and totals (#1808)', () => { const result = await getAuditLogs({ limit: 2 }); expect(result.nextCursor).not.toBeNull(); - // The cursor carries the timestamp *and* the id it is paging from. - expect(result.nextCursor).toContain(':'); - const [createdAt, id] = String(result.nextCursor).split(':'); - expect(createdAt).toBe('2026-03-02T00:00:00.000Z'); - expect(id).toBe('299'); + // The cursor carries the timestamp *and* the id it is paging from. An ISO + // timestamp contains ':' itself, so read it back from the *last* + // separator — exactly how decodeCursor parses it. + const cursor = String(result.nextCursor); + const separatorAt = cursor.lastIndexOf(':'); + expect(cursor.slice(0, separatorAt)).toBe('2026-03-02T00:00:00.000Z'); + expect(cursor.slice(separatorAt + 1)).toBe('299'); }); it('returns a null cursor on the last page', async () => { @@ -167,7 +171,10 @@ describe('getAuditLogs keyset pagination and totals (#1808)', () => { const countSql = String( mockQuery.mock.calls.find(([text]) => String(text).includes('COUNT(*)'))?.[0], ); - expect(countSql).toBe('SELECT COUNT(*) as count FROM audit_logs'); + // No filters → no WHERE clause. The builder interpolates an empty clause, + // so compare the trimmed statement instead of its trailing space. + expect(countSql).not.toContain('WHERE'); + expect(countSql.trim()).toBe('SELECT COUNT(*) as count FROM audit_logs'); }); }); @@ -199,6 +206,16 @@ describe('AuditLogFilters shape (#1808)', () => { limit: 1, withTotal: true, }; - expect(Object.keys(filters)).toHaveLength(8); + // Exactly the 7 query parameters documented for GET /admin/audit-logs + // (src/swagger/adminSwagger.ts). + expect(Object.keys(filters).sort()).toEqual([ + 'action', + 'actor', + 'cursor', + 'from', + 'limit', + 'to', + 'withTotal', + ]); }); }); diff --git a/backend/src/tests/idempotency.namespace.test.ts b/backend/src/tests/idempotency.namespace.test.ts index 3b58d4ed..b068689a 100644 --- a/backend/src/tests/idempotency.namespace.test.ts +++ b/backend/src/tests/idempotency.namespace.test.ts @@ -87,30 +87,44 @@ describe('idempotencyMiddleware key namespacing (#1809)', () => { }); it('does not replay another wallet’s cached response', async () => { - // Bob's response is already cached under his namespace… - asMock(cacheService.get).mockResolvedValue({ - status: 201, - body: { id: 'bob-loan' }, - fingerprint: computeFingerprint(buildRequest(BOB) as Request).fingerprint, - }); + // Bob's response is already cached under *his* namespace. Redis is keyed, + // so only Bob's namespace holds it — the mock models that by answering + // per key instead of returning Bob's entry for any key. + asMock(cacheService.get).mockImplementation((key: unknown) => + String(key).includes(BOB) + ? { + status: 201, + body: { id: 'bob-loan' }, + fingerprint: computeFingerprint(buildRequest(BOB) as Request).fingerprint, + } + : null, + ); // …so Alice sending the identical key, path and body gets a cache miss - // and runs the handler instead of receiving Bob's response. + // and runs the handler instead of receiving Bob's response. `res.json` is + // captured up-front because the middleware wraps it on a cache miss. + const jsonSpy = asMock(res.json); await idempotencyMiddleware(req as Request, res as Response, next); - expect(cacheKeysRead()[0]).not.toContain('bob-loan'); - expect(res.json).not.toHaveBeenCalledWith({ id: 'bob-loan' }); + expect(cacheKeysRead()[0]).toContain(ALICE); + expect(cacheKeysRead()[0]).not.toContain(BOB); + expect(jsonSpy).not.toHaveBeenCalledWith({ id: 'bob-loan' }); + expect(next).toHaveBeenCalled(); }); it('does not reject a user with 409 because another user holds the key', async () => { - // Bob's in-flight lock is held under his namespace. + // Bob's in-flight lock is held under his namespace, so the reservation is + // only refused for Bob's namespaced lock key. asMock(cacheService.get).mockResolvedValue(null); - asMock(cacheService.setNotExists).mockResolvedValue(false); + asMock(cacheService.setNotExists).mockImplementation((key: unknown) => + Promise.resolve(!String(key).includes(BOB)), + ); await idempotencyMiddleware(req as Request, res as Response, next); // Alice is unaffected by Bob's lock: the handler still runs. expect(asMock(res.status)).not.toHaveBeenCalledWith(409); + expect(next).toHaveBeenCalled(); }); it('namespaces the lock key as well as the cache key', async () => { diff --git a/backend/src/tests/idempotency.test.ts b/backend/src/tests/idempotency.test.ts index 918ca12a..4f2a2d14 100644 --- a/backend/src/tests/idempotency.test.ts +++ b/backend/src/tests/idempotency.test.ts @@ -62,7 +62,9 @@ describe('Idempotency Middleware', () => { await idempotencyMiddleware(req as Request, res as Response, next); - expect(cacheService.get).toHaveBeenCalledWith(`idemp:${key}`); + // #1809: the cache key is namespaced by the caller's wallet; this fixture is + // unauthenticated, so it reads from the shared `anon` namespace. + expect(cacheService.get).toHaveBeenCalledWith(`idemp:anon:${key}`); expect(res.status).toHaveBeenCalledWith(201); expect(res.set).toHaveBeenCalledWith('X-Idempotency-Cache', 'HIT'); expect(res.json).toHaveBeenCalledWith(cachedResponse.body); @@ -169,9 +171,10 @@ describe('Idempotency Middleware', () => { (res.json as unknown as (b: unknown) => void)({ success: true }); await finishHandler(); - expect(cacheService.delete).toHaveBeenCalledWith(`idemp:${key}:lock`); + // Both keys are namespaced by caller (#1809); this fixture is unauthenticated. + expect(cacheService.delete).toHaveBeenCalledWith(`idemp:anon:${key}:lock`); const setCall = (cacheService.set as jest.Mock).mock.calls[0]; - expect(setCall[0]).toBe(`idemp:${key}`); + expect(setCall[0]).toBe(`idemp:anon:${key}`); const stored = setCall[1] as { fingerprint: string; body: unknown }; expect(stored.body).toEqual({ success: true }); expect(stored.fingerprint).toBe(computeFingerprint(req as Request).fingerprint);