feat(api): contract pause guard, batch invoice publishing, KYC audit log - #213
Open
ayinde38 wants to merge 1 commit into
Open
feat(api): contract pause guard, batch invoice publishing, KYC audit log#213ayinde38 wants to merge 1 commit into
ayinde38 wants to merge 1 commit into
Conversation
Closes StellarState#136, StellarState#137, StellarState#179, StellarState#181. StellarState#136 — emergency contract pause detector and lockdown handler Adds ContractGuardService, which reads a contract's persistent `Paused` storage entry over Soroban RPC and caches it for 15 seconds so a burst of API traffic collapses into one RPC call per contract instead of one per request. A contract that has never been paused has no entry at all, which reads as not paused. On RPC failure the service serves the last reading it has, even an expired one, and only assumes "not paused" when it has never had one. Failing closed on every blip would take the API down for a network wobble; failing open would let trades through during a real incident. Both fallbacks log at warn. checkContractNotPaused short-circuits with 503 CONTRACT_PAUSED and is applied to the state-changing investment and settlement endpoints only — browsing during a pause is harmless, and blocking reads would hide the state of the system from the people who need to see it. The guard is inert unless both SOROBAN_RPC_URL and SOROBAN_ESCROW_CONTRACT_ID are configured, so existing deployments behave exactly as before. StellarState#137 — batch invoice publishing POST /api/v1/invoices/batch-publish takes { invoiceIds } (1-100 uuids) and publishes them inside one TypeORM transaction. Every invoice is validated first — ownership, DRAFT status, and the pre-publish field rules — and if any one fails the whole batch is rejected with nothing written, so a seller can never end up with a half-published book. The 400 response names every rejected invoice and why, so all problems can be fixed in one pass. An invoice belonging to another seller is reported with the same wording as a missing one, so the response does not confirm that someone else's id exists. StellarState#179 — funding deadline validator The deadline check reported both an expired invoice and a merely tight one as DUE_DATE_TOO_SOON. Splits out DUE_DATE_IN_PAST, since the two mean different things to the seller: one is stale, the other is fixable by moving the date. Extracts validateFundingDeadline with an injectable `now` so the boundary is testable, and adds coverage at the exact 24h cutoff, either side of it by a millisecond, 7 days out, and at/just before now. Two tests pin that the validator reads the server clock and ignores any timestamp on the request payload. The existing suite's past-date expectations move to the new code; the rejection itself is unchanged. StellarState#181 — KYC status change audit log Approve, reject and revoke now emit one shared "KYC status change" entry carrying wallet, previous_status, new_status, reviewer_wallet and changed_at, always after the write has committed so an entry is never written for a decision that failed to persist. previous_status is captured before the update. Wallets are truncated the same way as elsewhere. This replaces the separate approval/rejection decision logs, which carried neither the previous status nor the reviewer's wallet. There was no revoke endpoint to log, so POST /api/v1/admin/revoke-kyc is added. It returns an approved user to PENDING rather than REJECTED — they are no longer cleared to trade, but the decision is "needs review again", not "rejected on the merits". Revoking a user who is not approved is a 409 rather than a no-op that still writes an audit entry. Both new routes are documented in docs/openapi.json.
|
@ayinde38 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
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 #136
Closes #137
Closes #179
Closes #181
Targets
devas the issues require.#136 — emergency pause detector and lockdown handler
src/services/stellar/contract-guard.service.ts+src/middleware/contract-pause-guard.middleware.tsContractGuardServicereads a contract's persistentPausedstorage entry over Soroban RPC (getLedgerEntries) and caches it for 15 seconds, so a burst of API traffic collapses into one RPC call per contract instead of one per request. Concurrent reads of the same contract share a single in-flight round trip. A contract that has never been paused has noPausedentry at all, which reads as not paused.On RPC failure the service serves the last reading it has, even an expired one, and only assumes "not paused" when it has never had one. Failing closed on every blip would take the whole API down for a network wobble; failing open would let trades through during a real incident. Both fallbacks log at
warnso the gap is visible during an investigation. A failed read never poisons the cache with afalse.checkContractNotPausedshort-circuits with503 CONTRACT_PAUSEDand is applied to the state-changing investment and settlement endpoints only. Read endpoints are deliberately left ungated — browsing during a pause is harmless, and blocking reads would hide the state of the system from exactly the people who need to see it.The guard is inert unless both
SOROBAN_RPC_URL(new) andSOROBAN_ESCROW_CONTRACT_IDare configured, so existing deployments behave exactly as before.#137 — batch invoice publishing
POST /api/v1/invoices/batch-publish, body{ invoiceIds }(1–100 uuids).Everything is validated first — ownership,
DRAFTstatus, and the pre-publish field rules — and only then are the writes performed inside onedataSource.transaction. If any single invoice fails, the whole batch is rejected with nothing written, so a seller can never end up with a half-published book and no way to tell which half.The 400 response names every rejected invoice and why, so all problems can be fixed in one pass rather than rediscovering the next one on each retry. An invoice belonging to another seller is reported with the same wording as a genuinely missing one, so the response doesn't confirm that someone else's invoice id exists. Duplicate ids in the request are collapsed.
Reuses the existing per-wallet publish rate limiter and KYC gating.
#179 — funding deadline validator
The deadline check reported both an already-expired invoice and a merely-tight one as
DUE_DATE_TOO_SOON. These mean different things to the seller: one is stale, the other is fixable by moving the date out. Splits outDUE_DATE_IN_PAST.Extracts
validateFundingDeadline(dueDate, now)with an injectablenowso the boundary is testable at all.tests/funding-deadline-validator.test.tscovers the exact 24h cutoff, one millisecond either side of it, 23h59m, 7 days out, exactlynow, and one millisecond in the past. Two tests pin that the validator reads the server clock: one advances the system clock and watches the same deadline flip from valid to expired, the other confirms a backdatednow/clientTimestampon the request payload can't make an expired deadline publishable.Existing tests changed.
tests/validate-invoice-for-publish.test.tshad three past-date assertions expectingDUE_DATE_TOO_SOON; they now expectDUE_DATE_IN_PAST. The rejection itself is unchanged in every case — only the code is more specific.Naming note. The issue says
deadline_too_soon/deadline_in_past; this repo's validation codes areSCREAMING_SNAKEand prefixed by field (FACE_VALUE_TOO_LOW,MISSING_DOCUMENT), so I followed that.#181 — KYC status change audit log
Approve, reject and revoke now emit one shared
"KYC status change"entry vialogKYCStatusChange, carryingwallet,previous_status,new_status,reviewer_walletandchanged_at— plusaction,reviewer_id, andreasonwhere there is one.previous_statusis captured before the update, and the log is emitted after the write commits, so an entry is never written for a decision that failed to persist. Both properties are asserted, the ordering one by recording actual call order. Wallets are truncated via the existingtruncateWalletAddress.This replaces the separate
"KYC approval decision"/"KYC rejection decision"logs, which carried neither the previous status nor the reviewer's wallet.tests/unit/kyc-admin-routes.test.tsis updated accordingly; two of its tests asserted the old exact key set.There was no revoke endpoint to log, so
POST /api/v1/admin/revoke-kycis added. It returns an approved user toPENDINGrather thanREJECTED: they're no longer cleared to trade, but the decision is "needs review again", not "rejected on the merits", and it leaves them able to re-submit. This also avoids adding aREVOKEDvalue to thekycStatusDB enum, which would need a migration. Revoking a user who isn't currently approved is a409rather than a no-op that still writes an audit entry.Both new routes are documented in
docs/openapi.json.Verification — please read
The TypeScript in this PR has not been compiled or tested locally. Two independent blockers:
npm cifails ondevbefore any of my changes —npm error Missing: dom-serializer@3.1.1 from lock file. The lockfile is out of sync withpackage.jsonon the default branch.npm installalso fails (network read ETIMEDOUT).So
npm run type-checkandjestcould not be run. The code is reviewed by reading, not by compiling — please lean on CI here.One design decision follows directly from that: the Soroban ledger-entry decoder is an injectable dependency (
decodeEntry, defaulting to the real XDR implementation) rather than something the tests hand-assemble. Constructingxdr.ContractDataEntryfixtures by hand is exactly the kind of SDK-surface guess I couldn't verify, so the tests inject a trivial decoder and exercise the caching, concurrency and degradation behaviour instead.buildPausedLedgerKeyis still tested against the real SDK. Worth a reviewer's eye ondecodePausedEntryspecifically, since it's the one piece no test pins.Fixing the lockfile felt out of scope for this PR, but it's blocking
npm cifor everyone — happy to open a separate issue.