feat(strategy): add opt-in strategy marketplace with copy-trading (#285)#302
Open
Lansa-18 wants to merge 1 commit into
Open
feat(strategy): add opt-in strategy marketplace with copy-trading (#285)#302Lansa-18 wants to merge 1 commit into
Lansa-18 wants to merge 1 commit into
Conversation
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 #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 / −27Why this is more than CRUD
Every user's strategy is private today.
rebalanceStrategy/strategyConfiglive onUser, are read in exactly one place (agent/loop.ts), and — notably — are never written by any code insrc/. 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.tsimports nothing fromsrc/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 liverebalanceCheckJobrun asserting no wallet is opened, no key read,db.custodialWalletnever 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:
marketplaceSelectinstrategy/service.tsuserId, never includesusermapMarketplaceEntryToResponseinutils/api-formatters.tsstrategyLabelSchemainvalidators/strategy-validators.tslabelis the one self-doxx vector — rejects Stellar addresses and 32+ char hex runs, caps at 60The publisher's
userIdis loaded infollowStrategy— 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.tsreads onlyuserStrategyPreferences[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.tsbuilt preferences only whenstrategyNamewas truthy; otherwise it passedundefinedandexecuteRebalanceIfNeededskipped the strategy engine entirely. A follower whose ownrebalanceStrategyis 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 identicalexecuteRebalanceIfNeededcall.The metric, and a correctness trap
YieldSnapshot.apyis not a period return —snapshotter.tscomputes it as cumulative-yield-since-openedAtannualized, so consecutive values are a smoothed running average. Takingstdev()of that column would badly understate volatility and produce a flattering, gameable Sharpe.The correct series is portfolio value (
principal + yield), bucketed by exactsnapshotAtso 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 yieldInfinityand 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.tsis pure, zero-I/O, and is the single seam #225 replaces — #225 has not landed (exhaustive grep ofsrc/returns zero hits forsharpe/sortino/riskAdjusted). Do not add a third definition.Data model
Three tables + two
Userrelations.PublishedStrategy— one row per user (userId @unique); publish upserts.configVersionbumps only on a material change to the three agent-relevant keys; a label edit is cosmetic and notifies nobody.StrategyFollow— carries its ownappliedConfigsnapshot, not a read-through.publishedStrategyIdis nullable withonDelete: SetNullso a follower survives the publisher deleting their account.PublishedStrategyMetric— precomputed because the leaderboard mustORDER BYwithskip/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 asProtocolRiskScore+jobs/protocolRiskScoring.ts.Raw-SQL partial unique index (Prisma can't express it) — at most one active follow per user:
Hand-written
rollback.sqlincluded (CI-gated).API
New top-level
strategiesresource — one entry in theapiRoutestable, which yields the versioned and deprecated mounts for free. AllrequireAuth.POST/api/v1/strategies/publishPOST/api/v1/strategies/unpublishGET/api/v1/strategies/marketplace?sortBy=apy|sharpe&window=30d|90d&page&limitGET/api/v1/strategies/followingPOST/api/v1/strategies/:id/followPOST/api/v1/strategies/:id/unfollowDeviation from the issue: it proposed
/api/agent/strategies/*, butroutes/agent.tssits behindinternalAuthGuard— an operator/machine guard that authenticates byINTERNAL_SERVICE_TOKENand sets noreq.userId. User-facing routes can't live there.The param is
:id(aPublishedStrategyid), deliberately not:userId—enforceUserAccesscomparesreq.params.userIdagainst the caller and would reject every follow request.window=1yis rejected with a 400 naming the retention limit.snapshotter.tshard-deletes snapshots past 90 days, so1ywould 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 bumpsconfigVersionand 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
StrategyFollowsnapshots 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 proposedonDelete: Cascadewould delete the follow row outright.computeStrategyMetricstakesfirstObservedAtseparately. 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.GOAL_TRACKINGis not publishable — driven by the publisher's personal savings goal.Testing — 110 new tests
unit/agent/strategyMetrics.test.tsunit/agent/effectiveStrategy.test.tsunit/strategy/service.test.tsintegration/strategies.integration.test.tsG[A-Z2-7]{55}sweepintegration/agent/strategy-follow.integration.test.tsVerification
Every CI job run locally against a real Postgres:
migrate deploy(fresh DB) ✅ ·lint✅ ·format:check✅ ·build✅ ·npm test559/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
publishedStrategyIdand 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.security-scanwill fail: 30 pre-existing high-severity CVEs (js-yamlGHSA-pm4m-ph32-ghv5 and others).git diff main -- package.json package-lock.jsonis empty — this branch adds no dependencies, and the job fails onmaintoday. Needs its ownnpm audit fixPR.Deploy
npx prisma migrate deploy(orscripts/apply-migration.sh). No backfill script needed — the metrics job populatesPublishedStrategyMetricon its first run at startup. No new required env vars; both knobs have defaults, so.env.testand the CIenv:blocks need no change.STRATEGY_METRICS_INTERVAL_MS21600000(6 h)STRATEGY_RISK_FREE_RATE0Follow-ups (deliberately not done here)
(protocol, strategy)androuter.tsreads only index 0, so two users with the same strategy but differentriskCeilingalready 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.scripts/smoke-health.shdoescp .env.test .envand never cleans up. The leftover.envthen breakstests/unit/stellar/network-config.test.ts, becauseconfig/env.tscallsdotenv.config()and repopulates the var that test deletes. CI only escapes it becausesmokeandtestrun in separate jobs. Needs a trap-cleanup.jobs/alertRules.tsduplicates thevalueByInstantaggregation thatbucketByInstantnow generalizes — tested delivery path, refactor out of scope.