Files
dev-procfile/README-REFACTORING.md
T

421 lines
23 KiB
Markdown

# Wallet, Authentication, and API Refactoring Guide
This document records the current architectural and correctness risks in Darano's wallet, authentication, and API services, and proposes a safe sequence for refactoring them.
The recommended approach is an evolutionary refactor rather than a simultaneous rewrite. First stabilize financial and authentication flows, then establish explicit contracts and an append-only ledger, and finally migrate individual operations behind the new boundaries.
The existing [Spring 1404 technical report](docs/docs/%DA%AF%D8%B2%D8%A7%D8%B1%D8%B4%20%D9%87%D8%A7/%DA%AF%D8%B2%D8%A7%D8%B1%D8%B4-%D8%A8%D9%87%D8%A7%D8%B1-%DB%B1%DB%B4%DB%B0%DB%B4.md) is directionally correct about introducing an internal ledger. Its proposed schema should be extended with accounting invariants, reservations, idempotency, reversals, and reconciliation before implementation.
## Executive recommendation
Do not split the system into more independently deployed services yet. Keep the wallet as a modular service while the financial invariants are established. Prematurely separating ledger, market, settlement, and blockchain concerns would introduce more distributed failure modes without fixing the current consistency problems.
The first architectural decision must be the authoritative source of balances:
1. **Recommended:** an internal double-entry ledger is authoritative for available and reserved balances; Stellar is the custody and settlement layer.
2. **Alternative:** Stellar remains authoritative and PostgreSQL is a read-only projection.
The current system mixes both approaches: wallet reads retrieve Stellar balances and write them back to PostgreSQL, while other flows mutate PostgreSQL balances directly. This ambiguity must be removed.
## Highest-risk findings
### 1. Financial operations are not safely repeatable
- Asset purchase returns success before execution completes. Work runs in an in-process goroutine, so a restart can lose the operation. The accepted-agreement condition also does not reject a normally accepted agreement: [`wallet/core/walletImp/buy.go`](wallet/core/walletImp/buy.go#L390).
- The payment callback has no atomic terminal-state transition. A replay can repeat settlement and potentially initiate another on-chain credit, depending on provider behavior: [`wallet/core/walletImp/ipg.go`](wallet/core/walletImp/ipg.go#L131).
- Transfer confirmation loads a transaction using a caller-supplied ID without validating its owner, original asset, amount, recipient, type, or current state: [`wallet/core/walletImp/transfer.go`](wallet/core/walletImp/transfer.go#L208).
- IRT withdrawal confirmation has the same ownership and state-binding problem: [`wallet/core/walletImp/irt.go`](wallet/core/walletImp/irt.go#L110).
- `DepositIRT` creates a pending transaction and then calls another function that creates a second transaction: [`wallet/core/walletImp/irt.go`](wallet/core/walletImp/irt.go#L192).
Every financial command must accept an idempotency key, bind approval or MFA to the exact immutable transaction intent, and use a compare-and-set state transition.
### 2. The blockchain streamer has broken duplicate handling
The normal `record not found` result returns before inserting a deposit. That return also occurs before the mutex unlock is deferred, leaving the lock held until its TTL expires: [`wallet/port/stellar/stream.go`](wallet/port/stellar/stream.go#L91).
The streamer also needs a persistent blockchain cursor, a unique constraint on the external operation identity, replay-safe ingestion, and reconciliation against missed operations.
### 3. Database errors can be silently converted into success
The transaction helper wraps the named `err`, which is still nil, rather than `result.Error`: [`wallet/repository/db/postgres/tx.go`](wallet/repository/db/postgres/tx.go#L10).
Similar mistakes exist in transaction processing, including wallet fetch and commit failures: [`wallet/core/walletImp/transaction.go`](wallet/core/walletImp/transaction.go#L172).
Errors from `Begin`, `Commit`, `Rollback`, row-count checks, Redis, queues, and provider calls must never be ignored. Financial state transitions should fail closed.
### 4. Balance authority is ambiguous
Wallet reads query Stellar, merge blockchain and database state, calculate locks, and then update database balances as a side effect of reading: [`wallet/core/walletImp/wallet.go`](wallet/core/walletImp/wallet.go#L335).
Other paths directly mutate database balances. A refactor must define one authority, make projections explicitly disposable and rebuildable, and continuously reconcile the authority against custody and settlement systems.
### 5. Financial amounts use binary floating point
PostgreSQL `NUMERIC` values are represented as Go `float64` for wallet balances and transaction amounts:
- [`wallet/domain/db/wallet.go`](wallet/domain/db/wallet.go#L19)
- [`wallet/domain/db/transaction.go`](wallet/domain/db/transaction.go#L35)
Use integer minor units where the asset permits it, or an exact decimal type with an explicit scale per asset. Rounding rules must be named, centralized, and tested at every external boundary.
### 6. Concurrency protection is insufficient
- Balance reads inside database transactions do not lock rows using `SELECT ... FOR UPDATE` or an equivalent atomic update.
- The custom Redis lock is a non-atomic GET followed by SET, ignores Redis failures, and has no ownership token: [`wallet/repository/lock.go`](wallet/repository/lock.go#L13).
- Stellar sequence-number locking is commented out: [`wallet/port/stellar/transfer.go`](wallet/port/stellar/transfer.go#L130).
- Important uniqueness constraints are absent, including one wallet per `(user_id, asset_id)` and one accounting record per user.
Prefer database invariants and atomic conditional updates over distributed locks. When a distributed lock is unavoidable, it must use an owner token, safe release, bounded lease renewal, and fencing where applicable.
### 7. Migration and database transport policies are unsafe
Auth and wallet run GORM `AutoMigrate` during service startup. Wallet also disables PostgreSQL TLS and GORM's default transaction behavior: [`wallet/repository/db/postgres/main.go`](wallet/repository/db/postgres/main.go#L21).
Replace runtime migration with reviewed, versioned migrations. Use expand/contract deployments, migration tests, explicit rollback or forward-fix procedures, and TLS outside local development.
### 8. Internal gRPC trusts caller-supplied identity
Wallet RPC messages contain `InternalIAM`, but the wallet gRPC server does not install an authentication or authorization interceptor: [`wallet/cmd/cmdServe/wallet.go`](wallet/cmd/cmdServe/wallet.go#L74).
Service clients use plaintext `grpc.WithInsecure()`: [`api/service/main.go`](api/service/main.go#L107).
Any peer able to reach the gRPC ports may be able to forge an IAM payload. Internal communication should use:
- mTLS with service identities;
- per-RPC service authorization;
- a signed, short-lived user principal in metadata;
- network policies that make internal RPCs unreachable from the public edge;
- server-side derivation of identity instead of trusting protobuf body fields.
### 9. Authentication sessions and OTP need redesign
- OTP generation uses `math/rand`; development mode uses a fixed code: [`auth/usecase/tfa.go`](auth/usecase/tfa.go#L51).
- Failed OTP attempts rewrite the Redis entry with a 24-hour TTL: [`auth/repository/otp.go`](auth/repository/otp.go#L54).
- No per-mobile, IP, device, ASN, or system-wide OTP rate limiting is visible.
- Refresh-token rotation is not atomic. Concurrent reuse can mint multiple valid access tokens: [`auth/usecase/authorization.go`](auth/usecase/authorization.go#L221).
- JWT verification validates audience but does not explicitly constrain issuer and permitted algorithms. Signing keys are reread for each request, and no `kid`/JWKS rotation model exists: [`auth/util/auth.go`](auth/util/auth.go#L18).
- Permission checks are bypassed outside production. API routes are converted into permissions at startup, and newly created system roles are assigned to every user: [`auth/usecase/permission.go`](auth/usecase/permission.go#L57).
- Login synchronously calls wallet to provision a public key, coupling auth availability to wallet availability: [`auth/usecase/authorization.go`](auth/usecase/authorization.go#L166).
Auth should implement refresh-token families with hashed tokens, atomic rotation and replay detection, logout and revoke-all, device metadata, key rotation, secure OTP generation, abuse controls, and static deny-by-default policy definitions.
Wallet provisioning should be lazy or triggered by a durable `UserKYCVerified` event instead of blocking login.
### 10. API edge protections are incomplete
- Request and response logging can capture OTPs, access tokens, refresh tokens, banking data, and other PII: [`api/middlewares/logger.go`](api/middlewares/logger.go#L13).
- APM attaches national ID and mobile to traces.
- Gin uses its default HTTP server without explicit read, write, header, idle, and shutdown timeouts or request body limits: [`api/cmd/serve.go`](api/cmd/serve.go#L82).
- Metrics, Swagger, and WebSocket endpoints are registered on the public router without visible endpoint-specific protection.
- Multiple CORS origins are joined into one invalid response header: [`api/middlewares/cors.go`](api/middlewares/cors.go#L11).
- gRPC connections are deliberately closed after two minutes while generated clients keep references to the closed connection: [`api/service/main.go`](api/service/main.go#L128).
The API gateway should enforce request size and time limits, rate limits, security headers, redaction, idempotency, strict validation, bounded downstream deadlines, and a consistent public error model.
### 11. Key custody has a large compromise radius
The wallet configuration contains the master key and server secret keys, and user private keys are derived inside the wallet process from the master key and national ID:
- [`wallet/config/config.go`](wallet/config/config.go#L74)
- [`wallet/port/stellar/recoverKey.go`](wallet/port/stellar/recoverKey.go#L36)
Compromise of the wallet process or its configuration can therefore expose every derived user key. Define a custody model using an HSM, KMS, or isolated signing service; key versioning and rotation; hot/cold separation; quorum or maker-checker rules for high-value operations; backup and recovery; and an incident response procedure.
### 12. AdminPanel is a hidden wallet and auth consumer
AdminPanel reads unmanaged wallet and auth tables and directly modifies transaction status from a Django signal: [`AdminPanel/src/coreLogic/signals.py`](AdminPanel/src/coreLogic/signals.py#L14).
Schema and state-machine changes cannot safely proceed until AdminPanel is included in the migration plan. Prefer an authenticated internal command API for mutations and a dedicated read model for reporting.
### 13. Protobuf contracts and builds are not reproducible
API and auth generate protobufs from a moving `v2` branch, while wallet references an absolute path from another development machine:
- [`api/buf.gen.yaml`](api/buf.gen.yaml#L18)
- [`auth/buf.gen.yaml`](auth/buf.gen.yaml#L18)
- [`wallet/buf.gen.yaml`](wallet/buf.gen.yaml#L18)
Use one versioned protobuf module pinned to an immutable commit or release. Add Buf lint and breaking-change checks on every pull request, generate all clients from the same contract version, and publish deprecation metadata.
### 14. Test and CI coverage is effectively absent
The only Go test found is empty: [`wallet/repository/db/redis/lock_test.go`](wallet/repository/db/redis/lock_test.go#L9).
The service CI workflows build and deploy images without explicit unit tests, integration tests, race detection, vet, lint, migration checks, or contract compatibility checks: [`api/.gitea/workflows/ci.yaml`](api/.gitea/workflows/ci.yaml#L27).
The test strategy must be built before changing financial behavior.
## Recommended target boundaries
| Component | Responsibility |
| --- | --- |
| API gateway | HTTP validation, rate limits, idempotency, principal propagation, error mapping, and API compatibility. It should contain no financial logic or permission migrations. |
| Auth | Users, KYC, OTP/MFA, sessions, token issuance and revocation, and static policies. |
| Wallet core | Wallet commands, double-entry ledger, reservations, transaction state machines, and balance queries. |
| Payment orchestration | PSP callbacks, settlement, refunds, reversals, and reconciliation. |
| Chain adapter | Stellar-specific signing, submission, sequence management, confirmation, and chain reconciliation. |
| Workers | Durable processing of outbox records, callbacks, notifications, and retryable external work. |
| Admin | Authenticated internal command APIs and read-only reporting projections; no direct writes to core tables. |
Inside wallet, start with explicit modules rather than new services:
```text
wallet/
ledger/ append-only journals, entries, balances, holds
application/ commands and transaction state machines
payments/ IPG and bank orchestration
settlement/ chain intents and confirmation
chain/stellar/ Stellar adapter and signing client
reconciliation/ PSP, custody, supply, and ledger comparisons
projections/ user and admin read models
```
## Ledger requirements
The ledger must be append-only. Completed financial records should never be rewritten to represent a correction; use explicit reversal transactions.
At minimum, model:
- accounts scoped by owner, asset, purpose, and custody location;
- journals with a unique business operation and idempotency key;
- entries using exact amounts and explicit debit/credit direction;
- reservations or holds with purpose, status, and expiration;
- transaction state history;
- external settlement attempts and provider identifiers;
- outbox events committed in the same PostgreSQL transaction;
- optional materialized balance projections rebuildable from entries.
Required database invariants include:
- entries for each completed journal balance to zero per asset;
- every account has one asset and one defined scale;
- business and provider idempotency keys are unique;
- external blockchain hashes and PSP references are unique when present;
- state transitions use compare-and-set semantics;
- available balance cannot be overspent;
- immutable journal and entry rows cannot be updated or deleted by application roles.
## Transaction state machines
External operations cannot be made atomic with PostgreSQL. Model them as durable state machines or sagas.
An example withdrawal lifecycle is:
```text
requested
-> authorized
-> funds_reserved
-> signing
-> submitted
-> confirmed
-> completed
```
It must also support explicit failure states:
```text
expired
rejected
submission_unknown
failed
reversal_pending
reversed
manual_review
```
Each transition must record who or what initiated it, its timestamp, the previous state, an idempotency key, external references, and an auditable reason. Retries must resume from persisted state rather than restart the operation.
MFA approval should be bound to a hash of the exact intent, including user, operation type, asset, amount, recipient, fee, expiry, and nonce. An OTP tied only to a generic reason or record ID is insufficient.
## Refactoring sequence
### Phase 0: Stabilize and measure
- Freeze nonessential changes to financial flows.
- Patch the buy, callback, transfer, withdrawal, streamer, and transaction-error issues.
- Stop logging secrets and PII.
- Add missing uniqueness and positive-amount constraints after checking production data.
- Add correlation and idempotency IDs.
- Record metrics for transaction states, duplicates, balance drift, callback replay, OTP abuse, and chain lag.
### Phase 1: Add characterization tests
Before changing behavior, cover:
- OTP issuance, retry, expiry, and rate limiting;
- login, refresh rotation, concurrent refresh, logout, and revocation;
- internal and external transfers;
- IRT deposit, callback replay, settlement failure, and withdrawal;
- buy retries, compensation, and service restart;
- streamer restart, duplicate operations, cursor recovery, and missed events;
- wallet creation concurrency;
- AdminPanel mutations that currently affect wallet and auth state.
Add property tests for ledger invariants and concurrency tests for overspending and duplicate callbacks.
### Phase 2: Make contracts reproducible
- Pin protobuf inputs and generator versions.
- Add Buf lint and breaking checks.
- Define one structured error model using gRPC status details and a stable HTTP mapping.
- Add schema-level validation rules.
- Define pagination, filtering, public IDs, idempotency headers, and API deprecation rules.
- Generate and test UI clients from the same release.
### Phase 3: Secure service boundaries
- Add mTLS and service identities.
- Add gRPC authentication, authorization, deadline, recovery, metrics, and tracing interceptors.
- Remove caller-controlled IAM bodies.
- Restrict internal services using network policies.
- Replace manual connection expiry with normal long-lived gRPC connections and backoff.
- Move keys and provider credentials to managed secret storage.
### Phase 4: Refactor auth independently
- Introduce refresh-token families and store only token hashes.
- Rotate refresh tokens atomically and detect reuse.
- Support logout, revoke-all, device sessions, and administrative revocation.
- Validate issuer, audience, token type, timestamps, and permitted algorithms.
- Support `kid`-based signing-key rotation and JWKS distribution.
- Use cryptographically secure OTP generation and store OTP hashes.
- Add mobile, IP, device, and global abuse controls.
- Move permissions to reviewed, static, deny-by-default policy definitions.
- Remove wallet provisioning from the synchronous login path.
### Phase 5: Introduce the ledger
- Create versioned ledger migrations and invariants.
- Backfill opening balances with a documented source and timestamp.
- Build balance and statement projections.
- Introduce reservations for orders, withdrawals, purchases, BNPL, and redeem operations.
- Add transactional outbox publishing.
- Run old and new balance calculations in parallel and compare them continuously.
### Phase 6: Convert external flows
Migrate one flow at a time:
1. IRT deposit and payment callback.
2. IRT withdrawal.
3. Internal token transfer.
4. External token transfer.
5. Primary asset purchase.
6. Redeem and commission.
7. Market order reservation and settlement.
For each flow, require idempotency, durable state, compensation, reconciliation, and operational recovery procedures before cutover.
### Phase 7: Remove direct database consumers
- Route AdminPanel mutations through internal APIs.
- Move reporting to projections or a read replica.
- Remove old mutable balance code after parallel reconciliation reaches an agreed threshold.
- Separate ledger, settlement, or market into independent deployments only if scaling, ownership, or isolation requirements justify it.
## Missing considerations checklist
### Financial integrity
- [ ] Exact amount representation and asset-specific scale
- [ ] Double-entry and zero-sum enforcement
- [ ] Reservations, expirations, releases, and partial capture
- [ ] Reversal instead of mutation or deletion
- [ ] Unique business, provider, and blockchain idempotency keys
- [ ] Atomic compare-and-set state transitions
- [ ] Daily fiat, token-supply, PSP, and custody reconciliation
- [ ] Opening balance and migration reconciliation
- [ ] Negative balance and rounding policies
- [ ] Fee, tax, discount, and commission accounting
### Authentication and authorization
- [ ] Refresh-token family rotation and replay detection
- [ ] Logout, revoke-all, session expiry, and device management
- [ ] JWT key versioning, rotation, and emergency revocation
- [ ] Secure OTP generation, hashing, TTL, attempt limits, and abuse controls
- [ ] MFA bound to exact financial intent
- [ ] Static deny-by-default permissions
- [ ] Service-to-service identities and per-RPC authorization
- [ ] High-value transaction step-up policy
### Blockchain and custody
- [ ] HSM, KMS, or isolated signing service
- [ ] Master-key versioning and rotation
- [ ] Hot/cold wallet and treasury policy
- [ ] Sequence-number concurrency management
- [ ] Submission-unknown recovery
- [ ] Confirmation/finality definition
- [ ] Persistent stream cursor and replay procedure
- [ ] Gas and fee accounting
- [ ] Supply and custody reconciliation
- [ ] Key backup, restore drills, and compromise response
### Payments and messaging
- [ ] Webhook signature validation, timestamp, nonce, and replay protection
- [ ] Provider-specific idempotency and reference uniqueness
- [ ] Settlement, reversal, and refund state machines
- [ ] Transactional outbox and consumer inbox
- [ ] Durable queues and persistent messages
- [ ] Publisher confirms and mandatory routing
- [ ] Retry limits, exponential backoff, DLQ, and manual replay
- [ ] Event schema versioning and compatibility
### API and privacy
- [ ] Request body limits and HTTP server timeouts
- [ ] Per-route and per-identity rate limits
- [ ] Strict request validation and unknown-field handling
- [ ] Stable public error codes and status mapping
- [ ] Idempotency-key semantics and retention
- [ ] API versioning, deprecation, and consumer inventory
- [ ] Correct CORS and security headers
- [ ] Protected metrics, profiling, Swagger, and WebSocket endpoints
- [ ] PII encryption, redaction, retention, and deletion policy
- [ ] Immutable security and administrative audit trail
### Compliance and operations
- [ ] AML and suspicious-activity hooks
- [ ] User, asset, daily, and velocity limits
- [ ] Sanctions and risk screening integration points
- [ ] Maker-checker approval for administrative financial actions
- [ ] RPO, RTO, backup, restore, and disaster-recovery testing
- [ ] Incident response and key-compromise procedures
- [ ] Stuck-transaction and reconciliation-drift alerts
- [ ] Queue lag, chain lag, provider latency, and error budgets
- [ ] Capacity and load testing for high-volume events
### Delivery and testing
- [ ] Versioned database migrations and rollback/forward-fix policy
- [ ] Unit, integration, contract, concurrency, and property tests
- [ ] Deterministic Stellar, Redis, RabbitMQ, PostgreSQL, and PSP test environments
- [ ] Callback replay and service-restart tests
- [ ] Failure injection and reconciliation tests
- [ ] `go test`, race detector, vet, lint, and vulnerability scanning in CI
- [ ] Buf lint and breaking checks in CI
- [ ] Deployment health checks, readiness, graceful shutdown, and rollback gates
## Migration acceptance criteria
A flow should move to the new implementation only when:
- all monetary values use exact representation;
- repeat requests and callbacks produce one economic result;
- concurrent requests cannot overspend;
- every external action can be resumed after process restart;
- every state transition is auditable;
- ledger, PSP, and chain reconciliation is automated;
- old and new calculations agree within an explicitly approved tolerance;
- operational procedures exist for stuck, failed, unknown, and reversed operations;
- compatibility tests cover API, UI, AdminPanel, and protobuf consumers.
## Current verification status
This assessment is based on static inspection of the API, auth, wallet, protobuf, UI, AdminPanel, documentation, and CI configuration.
An attempt was made to run `go test ./...` for API, auth, and wallet. The configured internal Go module proxy timed out during dependency download, so the current compile status could not be independently verified. No source files were changed as part of that verification attempt.