Solution/karthika darvin#107
Open
karthikadarvin wants to merge 11 commits into
Open
Conversation
…ncy, failure cases)
There was a problem hiding this comment.
Pull request overview
Implements a Go/Echo/PostgreSQL wallet-to-wallet transfer service with transactional execution, double-entry ledger recording, and an idempotency mechanism, plus optional read endpoints for wallet balance and transfer history.
Changes:
- Added PostgreSQL schema migrations (wallets, transfers, ledger entries, idempotency records) plus seed wallets SQL.
- Implemented transfer execution flow (single DB transaction) with row-level locking and idempotency response caching, plus service-level integration/concurrency tests.
- Added optional GET endpoints for wallet balance and wallet transfer history, with supporting repositories/services/handlers and routing.
Reviewed changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds AI usage disclosure and transcript pointer. |
| migrations/000001_extensions.up.sql | Enables pgcrypto extension for UUID generation. |
| migrations/000001_extensions.down.sql | Rolls back pgcrypto extension. |
| migrations/000002_create_wallets.up.sql | Creates wallets table with stored balance and constraints. |
| migrations/000002_create_wallets.down.sql | Drops wallets table. |
| migrations/000003_create_transfers.up.sql | Creates transfers table and status enum with indexes/constraints. |
| migrations/000003_create_transfers.down.sql | Drops transfers table and status enum. |
| migrations/000004_create_ledger_entries.up.sql | Creates ledger_entries table and entry type enum with indexes. |
| migrations/000004_create_ledger_entries.down.sql | Drops ledger_entries table and entry type enum. |
| migrations/000005_create_idempotency_records.up.sql | Adds idempotency status enum and idempotency_records table. |
| migrations/000005_create_idempotency_records.down.sql | Drops idempotency_records table and idempotency status enum. |
| migrations/seed/seed_wallets.sql | Seeds two deterministic wallets for manual testing. |
| internal/domain/errors.go | Defines shared domain errors for consistent error handling. |
| internal/domain/wallet.go | Implements wallet debit/credit rules used by transfer flow. |
| internal/domain/transfer.go | Implements transfer state machine and validation. |
| internal/domain/ledger_entry.go | Implements double-entry pair constructor. |
| internal/domain/idempotency.go | Defines idempotency record/status domain types. |
| internal/repository/interfaces.go | Defines repository interfaces and DB querier abstraction. |
| internal/repository/postgres/wallet_repository.go | Implements wallet queries, including row locking and balance updates. |
| internal/repository/postgres/transfer_repository.go | Implements transfer persistence and wallet history listing. |
| internal/repository/postgres/ledger_repository.go | Implements atomic insert of debit+credit ledger rows. |
| internal/repository/postgres/idempotency_repository.go | Implements idempotency record insert/get/complete operations. |
| internal/service/transfer_service.go | Orchestrates transactional transfer execution + idempotency logic. |
| internal/service/wallet_service.go | Adds read-only wallet query service (balance/history). |
| internal/handler/transfer_handler.go | Adds POST /transfers endpoint and error mapping. |
| internal/handler/wallet_handler.go | Adds GET balance and transfer history endpoints. |
| internal/router/router.go | Registers health, transfer, and wallet routes. |
| internal/db/db.go | Adds PostgreSQL connection helper. |
| internal/db/migrate.go | Adds golang-migrate based migration runner. |
| internal/config/config.go | Adds env-based config loading. |
| cmd/server/main.go | Wires config/db/migrations/repos/services/handlers and starts Echo server. |
| internal/service/testutil_test.go | Provides test DB setup/reset helpers for service tests. |
| internal/service/transfer_service_test.go | Adds integration tests for idempotency, failure outcomes, and invariants. |
| internal/service/concurrency_test.go | Adds concurrent transfer tests verifying no lost updates/double-spend. |
| go.mod | Declares module and dependencies (Go version currently incorrect). |
| go.sum | Locks dependency checksums. |
| DESIGN.md | Documents schema, idempotency, concurrency, and API endpoints. |
| ai-transcript.md | Includes full AI transcript per assignment disclosure requirement. |
Comment on lines
+57
to
+74
| var rec domain.IdempotencyRecord | ||
| var transferID sql.NullString | ||
| var snapshot sql.NullString | ||
|
|
||
| if err := row.Scan(&rec.IdempotencyKey, &rec.RequestFingerprint, &transferID, &rec.Status, | ||
| &snapshot, &rec.CreatedAt, &rec.UpdatedAt); err != nil { | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return nil, repository.ErrNotFound | ||
| } | ||
| return nil, fmt.Errorf("scan idempotency record: %w", err) | ||
| } | ||
|
|
||
| if transferID.Valid { | ||
| rec.TransferID = &transferID.String | ||
| } | ||
| if snapshot.Valid { | ||
| rec.ResponseSnapshot = []byte(snapshot.String) | ||
| } |
Comment on lines
+19
to
+30
| m, err := migrate.NewWithDatabaseInstance( | ||
| "file://"+migrationsDir, | ||
| "postgres", | ||
| driver, | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf("create migrate instance: %w", err) | ||
| } | ||
|
|
||
| if err := m.Up(); err != nil && !errors.Is(err, migrate.ErrNoChange) { | ||
| return fmt.Errorf("run migrations: %w", err) | ||
| } |
| condition under concurrent duplicate submission. | ||
| 3. If the insert succeeds, proceed with the transfer. | ||
| 4. If the insert fails on a unique violation, look up the existing record: | ||
| - `COMPLETED` → return the cached `response_snapshot` (byte-identical replay). |
Comment on lines
+71
to
+79
| // Step 1: reserve the idempotency key. A unique constraint violation | ||
| // means a duplicate request (in-flight or already completed). | ||
| _, err = s.idempotency.Insert(ctx, tx, input.IdempotencyKey, fingerprint) | ||
| if err != nil { | ||
| if errors.Is(err, postgres.ErrKeyAlreadyExists) { | ||
| return s.resolveExistingKey(ctx, input.IdempotencyKey, fingerprint) | ||
| } | ||
| return nil, fmt.Errorf("reserve idempotency key: %w", err) | ||
| } |
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.
Summary
Implements a wallet-to-wallet transfer service in Go with the Echo framework and PostgreSQL. Supports idempotent transfer requests, double-entry ledger recording, concurrency-safe balance updates via row-level locking, and a transfer state machine (PENDING → PROCESSED/FAILED). Includes optional wallet balance and transfer history read endpoints.
AI disclosure
1. Tool used: Claude (claude.ai chat interface).
2. How I generally use the tool: I used Claude primarily for design discussion before writing any code — working through schema design, layered architecture, idempotency strategy, and concurrency/locking strategy in conversation before implementation began. Once the design was settled, I had Claude generate code file-by-file, which I reviewed, compiled, and tested myself before committing each piece. I made the final calls on design decisions myself (e.g. choosing golang-migrate over Goose, fixing the Go module import path, deciding stored balance vs. derived balance), and caught and corrected issues in Claude's output along the way — including an approach for service-layer tests that wouldn't have compiled (fake repositories that didn't match the actual service dependencies), which I flagged and redirected toward real-Postgres integration tests instead. I also asked for Windows-specific command equivalents where Claude defaulted to bash/Unix syntax.
3. Transcript: The full session transcript is included in this repository at
ai-transcript.md.Schema Design
Four tables:
wallets,transfers,ledger_entries,idempotency_records. Full details, including column rationale, constraints, and indexing decisions, are inDESIGN.md.wallets.balanceis a stored, denormalized column (not derived from the ledger), which also serves as the row locked viaSELECT ... FOR UPDATEduring transfers.CHECK (balance >= 0)is a DB-level backstop.transfersmodels the transfer lifecycle as a Postgres ENUM (PENDING/PROCESSED/FAILED), with aCHECK (from_wallet_id != to_wallet_id)guard against self-transfers.ledger_entriesis an immutable, append-only double-entry record — every transfer writes exactly one DEBIT and one CREDIT row in a single insert.idempotency_recordsusesidempotency_keyas its primary key, stores a request fingerprint (SHA-256 of the meaningful request fields) to detect key reuse with a different payload, and a JSONB response snapshot for byte-identical replay.Idempotency Strategy
The idempotency key is reserved via an
INSERTinside the same transaction as the transfer; the unique constraint onidempotency_keyis the actual deduplication mechanism, not an application-level check-then-insert. A unique violation triggers a lookup: aCOMPLETEDrecord returns the cached response; anIN_PROGRESSrecord (concurrent duplicate) returns a conflict; a fingerprint mismatch on the same key returns a distinct conflict error. A transfer that resolves toFAILED(e.g. insufficient balance) is a completed, idempotent outcome — retries return the same failure rather than re-attempting. Full details inDESIGN.md.Concurrency Strategy
Both wallets in a transfer are locked with
SELECT ... FOR UPDATE, in a consistent order (sorted by wallet ID) regardless of transfer direction, to prevent deadlocks between opposing transfers on the same wallet pair. Balance checks and updates happen only after locks are acquired. Verified with two concurrency tests against a real Postgres instance: concurrent transfers within available balance (all succeed, exact final balance) and concurrent transfers collectively exceeding balance (correct partial success/failure split, balance never negative). Full details inDESIGN.md.How to Run
DATABASE_URL(see.envexample / README)migrate -path migrations -database "$DATABASE_URL" uppsql "$DATABASE_URL" -f migrations/seed/seed_wallets.sqlgo run cmd/server/main.goHow to Test
TEST_DATABASE_URLto a separate Postgres test database, migrated the same waygo test ./... -vgo test ./internal/service/... -v -run ConcurrentTradeoffs / Assumptions
updated_atset explicitly in application code rather than via DB trigger, for simplicity.DESIGN.md.Checklist