feat(backend): transactional escrow orchestration service (#21)

[kris-nana]

Summary

Closes #21.

Implements a backend orchestration service that submits and confirms escrow-contract operations over Soroban RPC, with idempotency keys, exactly-once submission semantics, retry/backoff, and reconciliation of on-chain versus off-chain state.

• EscrowOrchestrationService — idempotent submission (idempotency_key unique constraint + REQUIRES_NEW insert guard, mirroring ChainEventInserter from On-Chain Event Ingestion Pipeline with Transactional Outbox & Replay #22), two independent scheduled claim/poll cycles (submitPending → SUBMITTED, pollSubmitted → CONFIRMED/FAILED), capped exponential backoff with a DEAD_LETTER terminal state.
• SorobanRpcClient — thin JSON-RPC 2.0 client (sendTransaction/getTransaction) built on the existing OkHttpClient bean. Transaction envelopes are relayed as opaque, already-signed base64 XDR — this service does not build or decode XDR itself.
• EscrowReconciliationService — flags a CONFIRMED request MISMATCHED if no corroborating PROCESSED on-chain event (from the On-Chain Event Ingestion Pipeline with Transactional Outbox & Replay #22 ingestion pipeline) appears within a configurable grace window.
• POST /api/v1/escrow/orchestrations (submit, idempotent) and GET /api/v1/escrow/orchestrations/{id} (status), both behind bearer auth.

Architecture decision, documented in backend-api/docs/ESCROW_ORCHESTRATION.md: no official Stellar/Soroban SDK is published to Maven Central for Java (checked org.stellar, network.stellar, java-stellar-sdk, stellar-android-sdk), so this PR deliberately avoids hand-rolling XDR encoding. Submission/confirmation only ever pass opaque strings through Soroban RPC (which is all sendTransaction/getTransaction need), and reconciliation reuses the already-ingested on-chain event stream from #22 instead of issuing raw getLedgerEntries reads.

Also fixes a pre-existing test-suite flake: @SpringBootTest classes that don't disable scheduling leave background @Scheduled pollers running against the shared test database for the rest of the JVM's life (Spring caches contexts), racing with whatever test runs next and mutating its rows out from under it — this was intermittently failing ChainEventServiceIntegrationTest on CI already (see the #22 PR's run history) and now hit the new escrow integration test too. Fixed via maven-surefire-plugin systemPropertyVariables defaulting all poller delays to 1h; explicit per-test @SpringBootTest(properties = ...) overrides still take precedence.

Test plan

• ./mvnw -B test — 93/93 passing locally against Postgres 16, run 3x to confirm the scheduler-leak fix holds
• ./mvnw -B verify — builds and packages cleanly
• New unit tests: EscrowOrchestrationServiceTest, EscrowReconciliationServiceTest, SorobanRpcClientTest (MockWebServer)
• New integration test: EscrowOrchestrationIntegrationTest — idempotency (incl. concurrent duplicate submits), full submit→confirm lifecycle, RPC failure → backoff → dead-letter, pessimistic-lock double-submission guard, reconciliation (matched/mismatched/pending-within-window)
• CI: Test workflow (.github/workflows/test.yml) runs on backend-api/** changes — pending on this PR

[meshackyaro]

Thanks — this is a very thorough implementation and I appreciate the attention to idempotency, the reconciliation model, and the end-to-end tests.

A few requests / suggestions before merging:

1. Database migrations / schema

   • Please confirm the migration that adds the idempotency_key unique constraint (and any new tables/columns) is included in the PR and provide the migration filename or path. I want to be sure deploys will be safe and that the migration is idempotent across environments.

2. Concurrency & locking

   • The REQUIRES_NEW insert-guard + pessimistic-lock approach looks reasonable, but could you add a short code comment (or README note) explaining why this approach was chosen over e.g. optimistic compare-and-swap? Also, mention expected behavior under DB deadlock and how callers should handle a transient lock failure.

3. Retry/backoff & observability

   • The capped exponential backoff and DEAD_LETTER terminal state are good. Please:
     • Ensure retries include jitter to avoid thundering herd.
     • Add metrics and structured logs for: submission attempts, retry counts, DEAD_LETTER transitions, reconciliation mismatches, and RPC error classes (timeout vs 5xx vs 4xx).
     • Confirm RPC timeouts and OkHttp retry settings are configured so a stuck RPC cannot eat a thread indefinitely.

4. Soroban RPC client safety

   • Since the client treats transaction envelopes as opaque base64 XDR, ensure any logged request/response payloads are sanitized (no secrets). Consider limiting logged body sizes and recording a correlation id for each RPC round-trip to ease debugging.

5. Reconciliation dependencies & window

   • The reconciliation relies on the on-chain event ingestion from On-Chain Event Ingestion Pipeline with Transactional Outbox & Replay #22. Please add (or point to) an operational note documenting:
     • The default grace window value and how operators should tune it.
     • What happens if the ingestion pipeline lags or is restarted (concrete recovery steps).

6. Tests & test-scheduler leak fix

   • Thanks for addressing the scheduler leak. For future maintainability, consider adding a short comment to the surefire configuration explaining the rationale (so it isn't accidentally removed).
   • In tests that assert timing/backoff behavior, please ensure the timing constants are injected (not hard-coded) so tests remain stable across environments.

7. Error surface & API behavior

   • For POST /api/v1/escrow/orchestrations: document the exact HTTP behavior on duplicate idempotent submits (201 vs 200 vs 409) and the returned body shape when the request is deduplicated.
   • For GET status: clarify whether the response includes an audit trail (timestamps for SUBMITTED/SUBMISSION_ATTEMPTs/poll attempts) or if that's only available via logs/metrics.

Minor

• Small nits: add a couple of inline code comments where complex business rules live (idempotency insertion, reconciliation matching logic) to help future readers.

[kris-nana]

Thanks for the thorough review — pushed a follow-up commit (47bd781) addressing everything actionable, and replying inline on the rest below.

1) Migrations/schema. No migration file — this repo has no Flyway/Liquibase; schema is managed entirely via spring.jpa.hibernate.ddl-auto=update, and escrow_orchestration_requests (including the uk_escrow_orch_idempotency_key unique constraint) is declared as JPA annotations in EscrowOrchestrationRequest.java, same as every other table in the app. This is the identical approach #22 used for on_chain_events/chain_event_outbox, so this PR isn't introducing a new schema-management strategy. Since it's a brand-new table, there's no existing data/column it could conflict with on deploy. Wrote this up in detail (including the known rename/type-change limitation of ddl-auto=update, which predates this PR) in docs/ESCROW_ORCHESTRATION.md under "Schema / migrations". Introducing Flyway is a reasonable idea but it's cross-cutting (affects every table) — happy to do it as a separate PR if the team wants it.

2) Concurrency & locking. Added Javadoc to EscrowOrchestrationInserter (why unique-constraint + REQUIRES_NEW beats an optimistic CAS — a CAS has a read/write race window that only a DB unique index can close atomically) and to EscrowOrchestrationRequestRepository#claimNext (why the pessimistic-lock claim can't deadlock against the inserter or against itself, and what happens under a stuck/held lock — bounded by soroban.rpc.request-timeout rather than an explicit Postgres lock-wait timeout).

3) Retry/backoff & observability.

• Jitter: done — backoff now has ± jitter (default 20%) applied, config-driven via new EscrowOrchestrationRetryProperties (escrow.orchestration.retry.max-attempts/base-delay/max-delay/jitter) instead of a hardcoded constant.
• Structured logs: done — INFO/WARN at every state transition (created, submitted, confirmed, on-chain failure, retry scheduled, DEAD_LETTER, reconciliation mismatch), all keyed by orchestration request id; RPC failures log the exception class so timeout vs. other causes are distinguishable.
• Metrics: deliberately not added. There's no Actuator/Micrometer dependency or instrumentation pattern anywhere else in this codebase — adding one is a cross-cutting infra decision (new dependency, /actuator exposure, security implications) that I don't think belongs in a feature PR without its own discussion. Flagged as a follow-up in the docs; happy to build it if the team wants it as a separate PR.
• RPC timeouts: caught a real gap here, thanks — soroban.rpc.request-timeout was defined but never actually wired into the OkHttp client. Fixed: SorobanRpcClient now builds its own OkHttpClient from the shared bean with callTimeout/connectTimeout/readTimeout/writeTimeout all set from it, so a stuck RPC call can't pin a thread (and the pessimistic lock it's holding) indefinitely.

4) Soroban RPC client safety. Every call now gets a correlation id (also sent as the JSON-RPC id) logged and included in exception messages, and response/error bodies are truncated to 500 chars before being logged or embedded in a message.

5) Reconciliation dependencies & window. Added an "Operations" section to the docs covering exactly this: how to tune escrow.reconciliation.window relative to expected indexer lag, and concrete recovery steps if the indexer lags/restarts (MISMATCHED is terminal by design — the sweep only reconsiders PENDING rows — so recovery today is confirming the event now exists and resetting reconciliation_status back to PENDING via a documented SQL statement; flagged a self-service "recheck" endpoint as a natural follow-up if that becomes frequent).

6) Tests & scheduler leak. The maven-surefire-plugin block in pom.xml already had a fairly detailed inline comment explaining the rationale (so it isn't accidentally removed) — left as-is, just referenced it from the docs. On timing constants: MAX_ATTEMPTS is gone as a hardcoded value — tests now construct their own EscrowOrchestrationRetryProperties (or rely on its documented default) instead of reaching into a constant, so behavior stays stable regardless of environment.

7) Error surface & API behavior. POST always returns 202 Accepted (new or replayed) — documented that choice explicitly rather than overloading 200/201/409, and added a X-Idempotent-Replay: true/false response header so clients can distinguish the two without inferring it from status code. submit() now returns a SubmitOutcome(request, replayed) internally to make that signal not require inference. For GET: documented explicitly that it returns current-state timestamps + attempt count, not a per-attempt audit trail — that level of detail is log-only for now (correlated by request id), with a persisted audit table flagged as a possible follow-up.

8) CI trigger. The Test workflow did run and pass on the original commit (https://github.com/workman-labs/guildworkman-core/actions/runs/30409797213) — it's gated behind GitHub's first-time-contributor "approve to run" check, so it needed a maintainer click, not a workflow-file change. This latest push (47bd781) is sitting in that same action_required state now and needs another approval to run — locally, ./mvnw test is green (93/93, run 3x to confirm no flakiness from the new jitter/timeout logic) and ./mvnw verify builds clean.

Minor (inline comments). Added comments on the idempotency-insertion path (EscrowOrchestrationInserter) and the reconciliation topic-matching logic (EscrowReconciliationService.reconcileOne) — the latter also surfaced a real (if narrow) gap: operationRef wasn't rejecting a literal ", which could have broken out of the JSON-quoted substring match used for reconciliation. Fixed with a validation constraint rather than just leaving it as a comment.

[meshackyaro]

Here are a few suggestions/improvements

• backend-api/src/main/java/com/guildworkman/api/escrow/service/EscrowOrchestrationService.java — deadLetter() (end of file): deadLetter sets status and lastError but does not persist the entity. Please either call repository.save(entity) inside deadLetter, or document that every caller must save afterwards. Persisting here is safer to avoid future omissions; also consider clearing nextAttemptAt or otherwise marking the entity terminal.
• backend-api/src/main/java/com/guildworkman/api/escrow/rpc/SorobanRpcClient.java — call(...) and exception paths: you truncate response bodies but not the request body. Request bodies (signed XDR) can be large and sensitive; do not embed the full request XDR in exception messages or logs. Please redact or truncate the request XDR before logging or including it in exceptions (e.g., show first N chars + total length), or omit it entirely and rely on rpcId for tracing.
• backend-api/src/main/java/com/guildworkman/api/escrow/api/SubmitOrchestrationRequest.java — signedTransactionXdr DTO: add an explicit maximum size validation (@SiZe(max = ...), e.g. 8192) to prevent very large payloads from being accepted (DoS/vector). Optionally add a base64 pattern validator if desired.
• backend-api/src/main/java/com/guildworkman/api/chain/repository/OnChainEventRepository.java — findByContractIdAndTopicsContaining(...): note that this uses LIKE on a JSON-encoded topics column; substring LIKE scans can be expensive for large event volumes. Consider documenting indexing guidance, or adding a computed column / normalized topics table if volumes increase.
• backend-api/pom.xml — surefire systemPropertyVariables comment: add a one-line note to the top-level README or CI docs explaining this is to avoid flaky scheduler leakage across @SpringBootTest contexts, so future contributors understand why these system properties exist.
• backend-api/src/main/resources/application.properties — schema management note: docs mention ddl-auto=update. Please add a short comment in application.properties (or docs/ESCROW_ORCHESTRATION.md) reminding operators that Hibernate ddl-auto=update is used and that production should consider an explicit migration strategy (Flyway/Liquibase) for controlled schema changes.
• Tests: add a unit test asserting SorobanRpcClient does not leak full request XDR in exception messages (sanitization), and add a DTO validation test for signedTransactionXdr size if you add the @SiZe constraint.
  Small improvements (non-blocking, recommended)

Consider persisting or incrementally exposing metrics (Micrometer counters/gauges) for submissions, retries, dead-letters, and reconciliation mismatches — logs are good, but metrics + alerts make ops easier. This is optional and can be a separate follow-up PR.
Consider adding a small admin endpoint or documented SQL snippet to requeue a MISMATCHED request (docs already mention this SQL; adding a small admin API could help operators).
Consider a circuit breaker/rate-limiter around SorobanRpcClient to avoid amplified retries if the RPC endpoint is unhealthy.

[kris-nana]

Thanks — pushed a follow-up commit (0acd4a5) with fixes/additions for all of these.

deadLetter() not persisting. Good catch — it wasn't actually a live bug (every current call site happened to repository.save(entity) right after), but it was exactly the kind of thing a future call site could easily forget. Fixed by having deadLetter() save internally and removing the now-redundant saves at its call sites, per your suggestion. nextAttemptAt is left as-is rather than cleared — it's already inert once DEAD_LETTER since claimNext only selects PENDING/SUBMITTED, and keeping it is a small diagnostic ("when would the next attempt have been").

Request-XDR leakage in SorobanRpcClient. Checked closely: the outgoing request body was never actually logged or embedded in an exception message to begin with (only method/rpcId are) — truncation only applied to the response. But you're right that it deserved a test, not just an assumption, so added three: a large signed XDR never appears in the exception message on HTTP-error, JSON-RPC-error, or IOException paths, plus a pathological case where the mock server's error response echoes the request back (still gets truncated to 500 chars, same as any other response).

signedTransactionXdr size cap. Added @Size(max = 8192) plus a base64-pattern @Pattern constraint, with a unit test suite validating the DTO directly via jakarta.validation.Validator (accepts a valid envelope, accepts exactly-at-limit, rejects oversized/non-base64/blank).

findByContractIdAndTopicsContaining LIKE-scan cost. Added a Javadoc note on the method explaining it's a sequential scan per contract (existing indexes can narrow by contract_id but can't help the LIKE itself), why a bigger index wouldn't fix a leading-wildcard search, and what the real fix looks like if volume ever demands it (normalized child table or jsonb + GIN).

Surefire comment → contributor docs. Added a "Background @Scheduled pollers default to off in tests" note to backend-api/README.md's Testing section explaining the rationale and explicitly telling future contributors not to remove the systemPropertyVariables block without an equivalent replacement.

ddl-auto=update reminder. Added a comment directly above spring.jpa.hibernate.ddl-auto in application.properties (not just in the docs) covering the same points as the "Schema / migrations" section — what it does/doesn't do safely, and that Flyway is a reasonable but cross-cutting follow-up.

Tests. Added the sanitization tests and the DTO validation tests as requested (see above).

Non-blocking suggestions:

• Metrics: still holding off for the reason described in the docs (no existing Micrometer/Actuator precedent in this codebase) — happy to do it as a follow-up PR.
• Requeue endpoint: implemented this one rather than leaving it as just documented SQL, since it came up twice now — POST /api/v1/escrow/orchestrations/{id}/requeue-reconciliation, ADMIN-gated via @PreAuthorize (same pattern as the existing AdminController), 409 if the request isn't currently MISMATCHED. Wraps the same update the docs describe; the manual SQL is still documented as a fallback. Added unit + integration coverage (including a full "flag MISMATCHED → requeue → corroborating event arrives → MATCHED" integration test).
• Circuit breaker: documented as a deliberate non-decision for now, same reasoning as metrics (no existing Resilience4j precedent, cross-cutting infra choice). Wrote up what's already mitigating the risk without one (bounded retries → DEAD_LETTER, bounded RPC timeout, one-row-per-tick claim throttling) and what a circuit breaker would add on top (failing fast across requests, not just within one).

Local suite is green — 106/106, run 3x (./mvnw test) to check for flakiness from these changes, plus ./mvnw verify.

[meshackyaro]

Fantastic work — this is a well-designed, well-tested feature and a real engineering-quality contribution.

Highlights I appreciated

• Clear scope and reasoning: the PR description and ESCROW_ORCHESTRATION.md explain the architecture decisions (especially the choice to treat XDR as opaque and avoid hand-rolling an SDK), which makes the tradeoffs easy to understand.
  Robust correctness focus: idempotency via a DB unique constraint + REQUIRES_NEW guard, exactly-once submission semantics, and the separate submit/poll cycles show careful thought about concurrency and failure modes.
• Excellent test coverage: unit tests for the service and client, MockWebServer for RPC, and a comprehensive integration test that covers idempotency under concurrency, submit→confirm lifecycle, backoff→dead-letter, and reconciliation scenarios. Tests like these make me confident this will behave correctly in production.
• Operationally minded: reconciliation against the ingested on-chain event stream, a configurable grace window for MISMATCHED detection, and a DEAD_LETTER terminal state are practical choices for real-world operation and recovery.
• Nice fix for a long-standing flake: the scheduler-leak/test-interference fix shows attention to CI stability — that kind of polish matters.
  Packaging / build: mvnw verify passes and the new code is organized and documented; that makes review and adoption easier.

This PR is high quality and ready to land from a design, test, and documentation perspective — great job carrying this through end-to-end, @kris-nana