Skip to content

Sideshift: query both old and new affiliate accounts and merge completed orders#220

Open
j0ntz wants to merge 3 commits into
masterfrom
jon/sideshift-dual-account
Open

Sideshift: query both old and new affiliate accounts and merge completed orders#220
j0ntz wants to merge 3 commits into
masterfrom
jon/sideshift-dual-account

Conversation

@j0ntz

@j0ntz j0ntz commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Description

Asana: https://app.asana.com/0/1215088146871429/1216057011453392

The Sideshift partner plugin queried a single affiliate account, so after the
Sideshift affiliate-account rotation it would drop the previous account's
historical completed orders and miss in-flight shifts still routed under the
previous affiliateId during the App Store update lag.

This changes src/partners/sideshift.ts to query BOTH affiliate accounts and
merge their completed-order streams, per the rotation decision (query both and
merge to preserve complete history).

What changed:

  • Config carries an optional second account via flat string fields
    sideshiftAffiliateIdNew / sideshiftAffiliateSecretNew alongside the
    existing primary pair. apiKeys reaching a plugin is a flat string map
    (asPartnerInfo in types.ts), so a nested array is not possible; the
    explicit pair is the backward-compatible form. getSideshiftAccounts returns
    the primary account plus the optional added account, deduped by affiliateId.
  • The primary sideshiftAffiliateId is the PRE-EXISTING account: on the first
    run after deploy it inherits the legacy single latestIsoDate watermark, so it
    resumes incrementally and never re-scans its full history. The *New pair is a
    NEWLY-ADDED account with no recorded history, so it backfills from epoch.
    Operational consequence at a rotation: put the current (soon-to-be-old) account
    in sideshiftAffiliateId and the new account in *New. No progress-cache edit
    is needed, and the account with years of history never does a multi-hour
    from-epoch crawl. (Reversing the slots would send the established account
    through a full from-epoch re-scan.)
  • The existing per-account query/signature/retry/time-block loop is factored into
    querySideshiftAccount and run once per configured account; the streams are
    merged into one result.
  • The latestIsoDate cursor is tracked PER account in a new settings.accounts
    map so one account's cursor never skips the other's orders. Legacy progress
    docs (single top-level latestIsoDate, no map) fall back to that value on the
    first run after deploy, so migration is seamless. The top-level latestIsoDate
    is kept as the overall max for backward compatibility.

A second commit hardens processing. bsv (Bitcoin SV) is added to the network
and delisted-coin maps so historical BSV orders resolve instead of throwing
Unknown network: bsv. And a single unprocessable order is now skipped and
logged instead of throwing: previously a throw left the per-account cursor stuck
before the bad order, so the account retried then gave up every cycle forever.
From-epoch backfills make hitting such an order far more likely.

Backward compatible: a single-account config (no added account) behaves exactly
as before. The new account's id/secret are config values ops adds at rotation
time; the code supports two accounts now.

Testing

  • test/sideshift.test.ts (mocha + chai, deterministic, no live network):
    single-account, dual-account merge with max overall cursor, legacy progress-doc
    migration, per-account cursor isolation, and skip-an-unprocessable-order. 8
    passing.
  • Real-order validation against the live affiliate account: 1199 orders sampled
    across the full 5.5-year history (41 asset/network pairs) all process cleanly.
  • tsc clean on the changed files.

edge-reports-server is a backend server with no app/sim surface, so the
dual-account merge + cursor logic is covered by the unit tests above. The live
Sideshift API and CouchDB persistence paths are unchanged.


Note

Medium Risk
Changes partner ingestion cursors and can permanently skip unprocessable orders once the cursor passes them; mis-rotating primary vs *New slots could cause large backfills or missed history, though behavior is backward compatible for single-account configs.

Overview
Sideshift reporting now supports an optional second affiliate via sideshiftAffiliateIdNew / sideshiftAffiliateSecretNew, queries each account (deduped, with errors on half-configured pairs), merges completed orders, and persists per-affiliate cursors in settings.accounts while keeping top-level latestIsoDate as the max across accounts. Legacy progress docs still work: the primary account inherits the old watermark; a newly added account backfills from epoch so rotation does not drop history.

Processing reliability changes: coins-list failures throw SideshiftTransientError and retry without advancing the cursor; unprocessable orders (e.g. unmapped coin/network) are logged by order id, skipped, and the cursor advances past them (including forward progress when a whole page is bad). BSV is added to network and delisted-coin maps.

Injectable fetch/process hooks and test/sideshift.test.ts cover dual-account merge, cursor migration, skip vs transient behavior.

Reviewed by Cursor Bugbot for commit 9763ac4. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread src/partners/sideshift.ts Outdated
@j0ntz
j0ntz force-pushed the jon/sideshift-dual-account branch from b21225d to c439b81 Compare June 25, 2026 23:52
@j0ntz

j0ntz commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Live dual-account smoke test (throwaway affiliate accounts)

Verified the old + new merge end to end against the live Sideshift API, using two throwaway affiliate accounts created via POST https://sideshift.ai/api/v2/create-account (no auth, no funds). affiliateIds Eg5PxMqMLh (new/primary) and QCZLA49Hkr (old/kept).

Check Result
(A) Live completedOrders accepts each account's HMAC-SHA1 signature PASS, HTTP 200 for both (0 orders, as expected for fresh accounts)
(B) querySideshift with the default live fetch queries both accounts PASS, per-account cursor map keyed by both affiliateIds, 0 fetch errors
(C) Both NEW and OLD affiliateIds hit the live endpoint in one run PASS
(D) Merge of non-empty streams through the real processSideshiftTx (live coins API) PASS, merged stream holds orders from both accounts; per-account cursors isolated (NEW 2026-06-20T10:00:00Z, OLD 2026-06-18T09:00:00Z, no cross-skip)

Fresh affiliate accounts have no order history, so (A)/(B)/(C) confirm the live query, signature, and cursor-map plumbing, while (D) feeds one realistic settled order per account into the live query loop so the real processing + merge path produces a combined non-empty stream. Combined with the deterministic unit tests in test/sideshift.test.ts, the dual-account merge is verified against the live API.

The throwaway accounts were used only for this read-only test and carry no funds.

…ed orders

Loop the per-account completed-orders query over each configured affiliate
account and merge the streams so a Sideshift affiliate-account rotation keeps
reporting the old account's historical and in-flight shifts. Track latestIsoDate
per account so neither account's cursor skips the other's orders, falling back to
the legacy single cursor for backward compatibility. Single-account config is
unchanged. Add a mocha test covering the dual-account merge and per-account
cursor behavior.

Claude-Session: https://claude.ai/code/session_01YNwNii1LxjqaeowP5d7dgJ
@j0ntz
j0ntz force-pushed the jon/sideshift-dual-account branch from c439b81 to ecde277 Compare June 29, 2026 23:13
Comment thread src/partners/sideshift.ts
Map bsv (Bitcoin SV) to the bitcoinsv plugin id and add BSV-bsv to the
delisted coins map so historical BSV orders resolve instead of throwing
"Unknown network: bsv".

Also skip and log a single unprocessable order rather than letting it
abort the page. A throw left the per-account cursor stuck before the bad
order, so the account retried then gave up every cycle forever. The
dual-account change makes from-epoch backfills routine, which makes
hitting such an order far more likely.
@j0ntz
j0ntz force-pushed the jon/sideshift-dual-account branch from 4e6e2b0 to b308478 Compare June 29, 2026 23:49
@j0ntz

j0ntz commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Code review

Found 4 issues:

  1. The new per-order catch in querySideshiftAccount logs a warning and unconditionally advances the cursor past any order that fails processOrder, including transient/unrelated errors. Previously such a throw propagated out and left the cursor un-advanced, so a later fix (e.g. adding a missing coin mapping) would let the order be re-fetched on the next run. Now, since orders are fetched with since=<cursor>, a skipped order's timestamp falls behind the advanced cursor and it is never re-fetched or recorded, even after the underlying problem is fixed. No CHANGELOG entry documents this behavior change.

standardTx = await processOrder(rawTx)
} catch (e) {
// A single unprocessable order (e.g. a coin or network missing from
// the plugin maps) must not stall the account. Without skipping, the
// throw aborts the page, the cursor never advances past it, and the
// account retries-then-gives-up every cycle forever. Log the gap so
// it can be mapped, then advance the cursor past this order's own
// timestamp so the window still moves forward even when an entire
// page is unprocessable.
log.warn(
`${account.affiliateId} skipping unprocessable order: ${String(e)}`
)
const skippedIsoDate = rawOrderIsoDate(rawTx)
if (skippedIsoDate != null && skippedIsoDate > latestIsoDate) {
latestIsoDate = skippedIsoDate
}
continue

  1. Inside that same catch, rawOrderIsoDate(rawTx) re-runs smartIsoDateFromTimestamp on the same createdAt that just made processOrder throw. If createdAt is unparseable, this second call throws again, uncaught, escaping the per-order catch into the outer network-retry path. The account then burns through MAX_RETRIES against an unchanged startTime and stalls permanently on this order, the exact failure mode this PR's skip logic was meant to prevent.

log.warn(
`${account.affiliateId} skipping unprocessable order: ${String(e)}`
)
const skippedIsoDate = rawOrderIsoDate(rawTx)
if (skippedIsoDate != null && skippedIsoDate > latestIsoDate) {
latestIsoDate = skippedIsoDate
}
continue

  1. querySideshift now queries both accounts sequentially inside one call, but the caller wraps the entire call in a single promiseTimeout, and persistence (dbProgress.insert) only happens after the call returns. If a newly-added second account has to backfill from epoch and exceeds the remaining timeout budget, the whole call rejects and the primary account's already-completed work for that run is discarded too, not just the new account's backfill.

let overallLatestIsoDate = legacyLatestIsoDate
for (let index = 0; index < accounts.length; index++) {
const account = accounts[index]

'reports_transactions'
)
let docIds: string[] = []
let startIndex = 0
for (let i = 0; i < transactions.length; i++) {
const transaction = transactions[i]
transaction.orderId = transaction.orderId.toLowerCase()
const key = `${pluginId}:${transaction.orderId}`
docIds.push(key)

  1. getSideshiftAccounts silently drops the second account when only one of sideshiftAffiliateIdNew/sideshiftAffiliateSecretNew is set (typo, partial config), with no log or error to signal the incomplete rotation.

]
if (
sideshiftAffiliateIdNew != null &&
sideshiftAffiliateSecretNew != null &&
sideshiftAffiliateIdNew !== sideshiftAffiliateId
) {
accounts.push({
affiliateId: sideshiftAffiliateIdNew,

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@j0ntz

j0ntz commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review findings above (fixup commit pushed, unit tests added):

  • Finding 2 (rethrow inside the skip handler): fixed. rawOrderIsoDate now guards the smartIsoDateFromTimestamp call, so an unparseable createdAt degrades to undefined (cursor still advances via the block-end fallback) instead of escaping into the retry path and stalling the account. Covered by a new unit test that fails without the guard.
  • Finding 4 (silent partial dual-account config): fixed. getSideshiftAccounts now throws a config error when exactly one of sideshiftAffiliateIdNew/sideshiftAffiliateSecretNew is set, so an incomplete rotation config fails loudly instead of silently querying only the primary. Covered by a new unit test.
  • Finding 1 (skipped orders are permanently dropped): partially addressed. The skip is the intended trade-off (the alternative is the pre-existing permanent stall this PR fixes), but the log line is upgraded from warn to error and now includes the full raw order JSON, so a dropped order is recoverable from logs after the mapping is fixed. The CHANGELOG now documents the behavior change explicitly.
  • Finding 3 (shared timeout discards the primary account's completed work): acknowledged, not changed. The deployed default for timeoutOverrideMins is 1200 minutes (20 hours) via src/config.ts, not the 5-minute fallback in util.ts, so a first-run epoch backfill fitting inside the budget is the expected case; and for this rotation specifically the new account is brand new with near-zero history. The structural fix (persisting per-account progress mid-run) requires queryEngine changes beyond this PR's scope.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9f244ba. Configure here.

Comment thread src/partners/sideshift.ts

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Comment thread src/partners/sideshift.ts
@j0ntz
j0ntz force-pushed the jon/sideshift-dual-account branch from 9f244ba to ee1c93b Compare July 9, 2026 18:00
@j0ntz
j0ntz force-pushed the jon/sideshift-dual-account branch from ee1c93b to 9763ac4 Compare July 9, 2026 18:30

@peachbits peachbits left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should skip orders so maybe we eliminate the third commit and most of the 2nd.

Comment thread src/partners/sideshift.ts
const {
sideshiftAffiliateId,
sideshiftAffiliateSecret,
sideshiftAffiliateIdNew,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this should be numbered (sideshiftAffiliateId2) instead of adding 'new'

Comment thread src/partners/sideshift.ts
let standardTx: StandardTx
try {
standardTx = await processOrder(rawTx)
} catch (e) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually prefer the throw. We're not going to notice some stray log mentioning a skipped order but we would notice if revenue stopped getting counted.

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.

2 participants