Skip to content

feat(strategy): add opt-in strategy marketplace with copy-trading (#285)#302

Open
Lansa-18 wants to merge 1 commit into
Neurowealth:mainfrom
Lansa-18:feat/issue-285-strategy-marketplace
Open

feat(strategy): add opt-in strategy marketplace with copy-trading (#285)#302
Lansa-18 wants to merge 1 commit into
Neurowealth:mainfrom
Lansa-18:feat/issue-285-strategy-marketplace

Conversation

@Lansa-18

@Lansa-18 Lansa-18 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Closes #285

Strategy Marketplace / Opt-In Copy-Trading (#285)

Adds an opt-in marketplace where a user publishes an anonymized snapshot of their agent configuration, others follow it, and a follower's agent applies that configuration on its next scheduled run.

Configuration only — never funds, never custody, never keys. That boundary is the core security property and is enforced structurally, not just documented.

27 files changed, +5013 / −27


Why this is more than CRUD

Every user's strategy is private today. rebalanceStrategy / strategyConfig live on User, are read in exactly one place (agent/loop.ts), and — notably — are never written by any code in src/. There was no way to set them, no way for a new user to adopt a proven configuration, and no reason for a successful user to stay engaged past their own portfolio.


The two boundaries

Custody. src/strategy/service.ts imports nothing from src/stellar/ — no wallet, no contract, no key custody — so there is no code path from a follow to another user's funds. Asserted by a test that parses the import graph and fails if a stellar/wallet import appears, plus a live rebalanceCheckJob run asserting no wallet is opened, no key read, db.custodialWallet never touched, and no downstream call carrying the publisher's id.

Anonymization. Three independent layers, because this is the first feature exposing one user's data to another:

Layer Where What it does
Query marketplaceSelect in strategy/service.ts Never selects userId, never includes user
Mapper mapMarketplaceEntryToResponse in utils/api-formatters.ts Hand-written allowlist, never a spread
Input strategyLabelSchema in validators/strategy-validators.ts label is the one self-doxx vector — rejects Stellar addresses and 32+ char hex runs, caps at 60

The publisher's userId is loaded in followStrategy — solely for the self-follow comparison. It never reaches a response.


Two real hazards in the agent loop

Both would have silently corrupted behaviour on the money-moving path.

1. Batching collision. router.ts reads only userStrategyPreferences[0]. The batching key was ${protocol}:${strategy}, so two followers of different published strategies sharing a protocol would collapse into one batch and both be rebalanced on index-0's config — including index-0's risk ceiling. Key is now ${protocol}:${effectiveStrategy}:${followId ?? 'none'}.

Keyed on the per-user follow id, not the followed strategy id, deliberately: two followers of the same strategy can still have different effective ceilings, because a follow clamps to the stricter of publisher and follower. Costs some batching; buys risk correctness.

2. The preferences guard. loop.ts built preferences only when strategyName was truthy; otherwise it passed undefined and executeRebalanceIfNeeded skipped the strategy engine entirely. A follower whose own rebalanceStrategy is null — the common case for a new user, exactly who this feature targets — would have had their followed config silently ignored. Guard is now "effective strategy or active follow".

The risk-ceiling invariant: higher score = lower risk, so the stricter ceiling is the larger number and that wins (Math.max). A follow can only ever tighten a follower's risk, never widen it. Copying a stranger's looser ceiling onto someone's funds because they clicked "follow" isn't a trade-off this feature is allowed to make.

Precedence: active SavingsGoal > followed strategy > own preference.

Backward-compat: a user with no follow reads their own config through the exact same raw expressions as before, and the follow component of the key is the constant 'none', so grouping is unchanged. A regression test asserts an identical executeRebalanceIfNeeded call.


The metric, and a correctness trap

YieldSnapshot.apy is not a period return — snapshotter.ts computes it as cumulative-yield-since-openedAt annualized, so consecutive values are a smoothed running average. Taking stdev() of that column would badly understate volatility and produce a flattering, gameable Sharpe.

The correct series is portfolio value (principal + yield), bucketed by exact snapshotAt so a point is the whole portfolio rather than one position, then differenced into period returns. Intervals starting at ≤ 0 are skipped — a portfolio funded from empty is a deposit, not a return, and would hand the publisher an unbounded Sharpe.

Sharpe is null — never 0 — when not computable (< 2 returns, or zero variance). A flat series dividing by zero stdev would yield Infinity and sort straight to the top.

Eligibility gate: trackRecordDays >= 30 && sampleCount >= 14 && sharpe !== null. Ineligible strategies are excluded entirely rather than ranked low, so a one-day strategy posting a misleading APY never appears.

src/agent/strategyMetrics.ts is pure, zero-I/O, and is the single seam #225 replaces#225 has not landed (exhaustive grep of src/ returns zero hits for sharpe/sortino/riskAdjusted). Do not add a third definition.


Data model

Three tables + two User relations.

  • PublishedStrategy — one row per user (userId @unique); publish upserts. configVersion bumps only on a material change to the three agent-relevant keys; a label edit is cosmetic and notifies nobody.
  • StrategyFollow — carries its own appliedConfig snapshot, not a read-through. publishedStrategyId is nullable with onDelete: SetNull so a follower survives the publisher deleting their account.
  • PublishedStrategyMetric — precomputed because the leaderboard must ORDER BY with skip/take (a JS value can't be ordered in SQL), and recomputing every publisher's history per request is a DoS vector on a semi-public endpoint. Same precedent as ProtocolRiskScore + jobs/protocolRiskScoring.ts.

Raw-SQL partial unique index (Prisma can't express it) — at most one active follow per user:

CREATE UNIQUE INDEX "strategy_follows_active_follower_key"
  ON "strategy_follows"("followerUserId") WHERE "unfollowedAt" IS NULL;

Hand-written rollback.sql included (CI-gated).


API

New top-level strategies resource — one entry in the apiRoutes table, which yields the versioned and deprecated mounts for free. All requireAuth.

Method Path
POST /api/v1/strategies/publish
POST /api/v1/strategies/unpublish
GET /api/v1/strategies/marketplace?sortBy=apy|sharpe&window=30d|90d&page&limit
GET /api/v1/strategies/following
POST /api/v1/strategies/:id/follow
POST /api/v1/strategies/:id/unfollow

Deviation from the issue: it proposed /api/agent/strategies/*, but routes/agent.ts sits behind internalAuthGuard — an operator/machine guard that authenticates by INTERNAL_SERVICE_TOKEN and sets no req.userId. User-facing routes can't live there.

The param is :id (a PublishedStrategy id), deliberately not :userIdenforceUserAccess compares req.params.userId against the caller and would reject every follow request.

window=1y is rejected with a 400 naming the retention limit. snapshotter.ts hard-deletes snapshots past 90 days, so 1y would return a 90-day figure under a wrong label.

Two new webhook events: strategy.updated, strategy.unpublished, plus WhatsApp formatters. On a material change, one transaction bumps configVersion and rewrites every active follower's snapshot; notifications fire outside it, fire-and-forget — a Twilio outage must not roll back a publish.


Deviations from the issue spec

  1. StrategyFollow snapshots the config. The issue's schema has no snapshot column, yet its own edge case requires followers to keep their last-applied config when a publisher unpublishes — and its proposed onDelete: Cascade would delete the follow row outright.
  2. computeStrategyMetrics takes firstObservedAt separately. The window governs return statistics; the track record governs trust. Deriving the track record from the windowed series would make the 30-day window structurally incapable of reaching a 30-day track record (oldest in-window sample sits ~30 days back, floors to 29) — the 30-day leaderboard would stay permanently empty.
  3. GOAL_TRACKING is not publishable — driven by the publisher's personal savings goal.
  4. Unfollow tolerates an orphaned follow, so a follower whose publisher deleted their account isn't stuck with no id to name.

Testing — 110 new tests

Suite Count Pins
unit/agent/strategyMetrics.test.ts 27 Eligibility boundaries at exactly 30 days / 14 samples; null-not-zero Sharpe; zero-variance and single-point series don't divide by zero
unit/agent/effectiveStrategy.test.ts 23 Risk ceiling clamps to stricter in both directions; no-follow is the identity
unit/strategy/service.test.ts 22 Version bumps only on material change; label edit notifies nobody; self-follow 409; re-follow swaps atomically
integration/strategies.integration.test.ts 25 PII assertion — per-field checks plus a whole-body G[A-Z2-7]{55} sweep
integration/agent/strategy-follow.integration.test.ts 13 Custody assertion; no-follow regression; cross-contamination

Verification

Every CI job run locally against a real Postgres:

migrate deploy (fresh DB) ✅ · lint ✅ · format:check ✅ · build ✅ · npm test 559/559, 51/51 suites ✅ · license-checker ✅ · redocly lint + bundle ✅ · check-migration-rollback.sh ✅ · docs fence + link checks ✅ · migration-smoke ✅ · npm run smoke ✅ · api-contract 401 checks ✅

Migration verified on a Postgres 14 container — apply → rollback → re-apply clean; partial unique index rejects a second active follow; a closed follow doesn't block a new one; deleting the publisher nulls publishedStrategyId and keeps the follow row with its config intact.

Real server boots in 3s; new job logs strategy_metrics completed in 0.021s status=success; graceful shutdown clears the timer.


⚠️ Unrelated CI failure

security-scan will fail: 30 pre-existing high-severity CVEs (js-yaml GHSA-pm4m-ph32-ghv5 and others). git diff main -- package.json package-lock.json is empty — this branch adds no dependencies, and the job fails on main today. Needs its own npm audit fix PR.


Deploy

npx prisma migrate deploy (or scripts/apply-migration.sh). No backfill script needed — the metrics job populates PublishedStrategyMetric on its first run at startup. No new required env vars; both knobs have defaults, so .env.test and the CI env: blocks need no change.

Env var Default
STRATEGY_METRICS_INTERVAL_MS 21600000 (6 h)
STRATEGY_RISK_FREE_RATE 0

Follow-ups (deliberately not done here)

  1. Pre-existing risk-control bug: the agent batches by (protocol, strategy) and router.ts reads only index 0, so two users with the same strategy but different riskCeiling already collapse into one batch and both get index-0's ceiling. This PR doesn't worsen it (follows split per-follow) but it deserves its own issue.
  2. Pre-existing: scripts/smoke-health.sh does cp .env.test .env and never cleans up. The leftover .env then breaks tests/unit/stellar/network-config.test.ts, because config/env.ts calls dotenv.config() and repopulates the var that test deletes. CI only escapes it because smoke and test run in separate jobs. Needs a trap-cleanup.
  3. jobs/alertRules.ts duplicates the valueByInstant aggregation that bucketByInstant now generalizes — tested delivery path, refactor out of scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strategy Marketplace / Opt-In Copy-Trading

1 participant