Repository navigation
chore: revert the #1703 squash so the four fixes can be re-landed separately - #1880
Merged
github-actions[bot] merged 1 commit intoOct 22, 2026
Merged
Conversation
…per-issue split PR Smartdevs17#1703 squash-merged the fixes for four issues into a single commit (5afe754). This commit mechanically undoes that squash so each issue can be re-landed on its own pull request. The revert is computed as \main - Smartdevs17#1703\: for every file in the Smartdevs17#1703 patch the pre-Smartdevs17#1703 content is folded back into the current file, so every change merged after Smartdevs17#1703 is preserved. Files that Smartdevs17#1703 added are removed again. This is a preparation commit only and must be merged first; the four follow-up pull requests re-land the work one issue at a time.
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
## Context This is the first of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time. #1703 was merged as a single commit (`5afe754`) covering four unrelated issues. The companion pull request ([the revert](#1703)) undoes that squash so each issue can be merged on its own. This pull request restores **only** the part of #1703 that belongs to: - #1683 — crypto payment resilience ## What this changes ### Transaction confirmation (`src/services/transactionConfirmation.ts`, new) Waiting for a transaction is now a first-class operation instead of a fire-and-forget `tx.wait()`: - chain-aware confirmation thresholds (`requiredConfirmations`), a default timeout, and distinct error types — `TransactionRevertedError`, `TransactionTimeoutError`, `TransactionInsufficientConfirmationsError` — so a caller can tell "the chain rejected it" from "we stopped waiting". - `isDefinitiveFailure()` separates failures that can never succeed on retry (revert, invalid params) from transport-level problems, which is what the queue below relies on. - `bestEffortTransactionHash()` recovers the hash when the wallet broadcast succeeded but the receipt was never observed. ### Decimal-safe amounts (`src/utils/decimalAmount.ts`, new) Token amounts were compared as JavaScript floats. `normalizeDecimal`, `compareDecimalAmounts` and `exceedsLimit` do the comparison on the decimal string instead, so an amount that is exactly at the limit is no longer reported as over the limit because of binary float drift. ### Resilient RPC provider (`src/services/rpcProvider.ts`) - The circuit breaker now has a `half-open` state with a success threshold, so a node that flaps is not immediately trusted again. - Only transport-level errors count towards the failure threshold. A legitimate JSON-RPC failure (revert, insufficient funds) means the node answered correctly and must not evict a healthy endpoint. - Endpoint switching is serialised through a swap chain. The base ethers provider talks to a single mutable `connection.url`, so two concurrent calls (e.g. `getBalancesAcrossChains`, which fans out in parallel) previously interleaved and one of them was sent to a node on a different chain. ### Payment method limits (`src/services/paymentMethodService.ts`) - The token's own `decimals()` is read instead of assuming "USDC = 6, everything else = 18", which under- or over-reported balances and let a charge through that could not settle. - `maxSpendPerInterval` is enforced with `exceedsLimit()` instead of a float comparison. - Balance reads go through the resilient provider. ### Wallet operations (`src/services/walletService.ts`) Stream creation and token approvals now go through `confirmTransaction()` and fail loudly when the chain does not confirm them, instead of resolving optimistically. ### Transaction queue (`src/store/transactionQueueStore.ts`, `src/store/walletStore.ts`) - A `broadcastedAt` marker is persisted **before** the transaction is handed to the wallet. A broadcast can succeed on-chain even when the app dies before the receipt is observed, so the next queue pass can tell "never sent" apart from "sent, outcome unknown" and refuse to re-broadcast the latter — which would otherwise charge the user twice. - Failed attempts are retried with exponential backoff (5 s, doubling, capped at 5 min) and only when the failure is not definitive, so a reverted transaction is not retried forever. ## Merge order This is the second of five pull requests in the split of #1703. It is based on `main` **after** the revert pull request ([#1880](#1880)) was merged, so its diff contains the #1683 work and nothing else. The remaining three pull requests (#1684, #1686, #1685) are opened on top of this one, in that order, and must be merged after it. The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
## Context This is the second of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time. #1703 was merged as a single commit (`5afe754`) covering four unrelated issues. The companion pull request (the revert) undoes that squash so each issue can be merged on its own. This pull request restores **only** the part of #1703 that belongs to: - #1684 — subscription billing correctness ## What this changes ### One place that computes invoice totals (`src/utils/invoiceTotals.ts`) `createInvoice` and `updateInvoice` each rolled their own arithmetic and the two did not agree — `createInvoice` subtracted the discount, `updateInvoice` did not. Editing one line on a discounted invoice therefore silently increased the amount payable, and because the discount was never persisted on the record there was nothing left to subtract on the second pass. `calculateInvoiceTotals()` is now the single source of truth: - every component is rounded to minor units (half-up, no binary float drift); - discount line items (`kind: 'discount'`) are kept out of the subtotal and summed into `discount` instead, so an itemised discount is subtracted exactly once; - the discount is clamped to the subtotal. Without the clamp a discount larger than the invoice produced a negative tax base and a negative total, which downstream reconciliation reads as a credit owed rather than an over-discounted invoice. ### Invoice service (`src/services/invoiceService.ts`) Both the create and the update path go through the shared totals function, and the discount is persisted on the record so a second pass subtracts the same amount. ### Billing cycle maths (`src/utils/billingCycleMath.ts`, new) `monthlyEquivalent`, `yearlyEquivalent`, `isAnnualCycle` and `normalizeBillingCycle` replace the ad-hoc cycle conversions that were scattered across the store and the analytics service, which disagreed about what a weekly or yearly cycle was worth. ### Revenue recognition (`backend/billing/jobs/monthlyRevenueRecognitionJob.ts`, `backend/billing/jobs/index.ts`) The job now tracks cumulative per-entry recognition progress (`recordEntryProgress`, `recognisedToDate`). Previously each run re-recognised the whole elapsed portion of an entry, so recognised revenue was inflated on every run and the deferred balance grew without bound. ### Metering and group billing (`backend/services/billing/meteringService.ts`, `groupBilling.ts`, `interfaces.ts`) - Metered billing aggregates over the requested window instead of the last N days from "now", so a period that ended long ago no longer picks up today's usage. - `aggregateCharges(group, from, to)` takes an explicit window. - Amounts are rounded to minor units on the way out. ### Analytics (`src/services/analyticsService.ts`, `src/store/slices/analyticsSlice.ts`, `src/store/subscriptionStore.ts`, `backend/services/analytics/subscriptionAnalyticsService.ts`) Monthly and yearly equivalents now come from the shared billing-cycle helpers, so the dashboard, the store and the backend report the same numbers. ### Batch charges (`backend/services/batchChargeService.ts`) Uses the shared rounding helper. ### Test (`src/services/__tests__/groupBilling.test.ts`) Updated for the explicit-window `aggregateCharges` signature. ## Merge order This is the third of five pull requests in the split of #1703. It is based on `main` **after** the revert pull request ([#1880](#1880)) and the #1683 pull request ([#1881](#1881)) were merged, so its diff contains the #1684 work and nothing else. The remaining two pull requests (#1686, #1685) are opened on top of this one, in that order, and must be merged after it. The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
## Context This is the third of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time. #1703 was merged as a single commit (`5afe754`) covering four unrelated issues. The companion pull request (the revert) undoes that squash so each issue can be merged on its own. This pull request restores **only** the part of #1703 that belongs to: - #1686 — platform security ## What this changes ### Rate-limit tier cannot be spoofed (`backend/server.ts`, `backend/services/shared/rateLimitingService.ts`) The caller's tier was read from the client-supplied `x-subscription-tier` header verbatim, so any caller presenting a key could send `X-Subscription-Tier: enterprise` and receive the 10,000 req/hr allowance instead of the FREE tier's 100 req/hr — a 100× rate-limit bypass requiring no privilege at all. The tier now comes from the server-side record for that key (`rateLimitingService.getTierForApiKey`). The header is honoured only when the explicit `TRUST_TIER_HEADER=true` opt-in is configured, which suits deployments where an authenticating gateway sits in front of this process and strips the header from inbound requests. ### Outbound URL guard (`backend/shared/security/outboundUrlGuard.ts`, new) `validateOutboundUrl()` rejects URLs that resolve to private, loopback, link-local or otherwise non-routable addresses, including DNS names that resolve to them. `isPrivateHostname()` and `assertSafeIdentifier()` are the building blocks. Used by: - `backend/integrations/zapier/ZapierIntegrationService.ts` and `zapierRouter.ts` — the Zapier integration fetches a user-supplied URL, which was a server-side request forgery vector. - `backend/services/webhook/eventReplayWorker.ts` — the replay worker re-delivers to a stored webhook target. ### Cursor pagination (`backend/services/shared/pagination.ts`, new) `encodeCursor` / `decodeCursor` sign the cursor payload with a secret, so a client cannot craft one; `buildCursorClause` and `buildPage` provide keyset pagination, and `parseFieldSelection` / `selectFields` let a caller restrict the returned columns. ### API key rotation (`backend/services/auth/domain/ApiKeyRotationService.ts`) Key rotation records the previous key's state so it can be revoked and audited. ### Webhook body limits and raw payload handling (`backend/server/createApiServer.ts`) The two provider webhook registrations now cap the raw body size (`limit: MAX_JSON_BODY`), so a webhook POST cannot be used to exhaust memory. The registrations remain body-parser-only — no handler, no state change — and the HMAC verification still happens inside the Stripe and Paddle routers, which fail closed with 503 when unconfigured and 401 on a bad signature. ### Revalidation allow-list (`developer-portal/pages/api/revalidate.ts`) `path` is caller-supplied and goes straight into `res.revalidate()`, so an arbitrary value let a holder of the secret force revalidation of unrelated routes. Only the documented set of paths is accepted now, and the secret is compared in constant time. ### Gateway (`backend/gateway/index.ts`) and configuration (`.env.example`) The new toggles (`TRUST_TIER_HEADER`, the pagination cursor secret, the webhook body limit) are documented in `.env.example`. ## Merge order This is the fourth of five pull requests in the split of #1703. It is based on `main` **after** the revert pull request ([#1880](#1880)) and the #1683 ([#1881](#1881)) and #1684 ([#1882](#1882)) pull requests were merged, so its diff contains the #1686 work and nothing else. The last pull request (#1685) is opened on top of this one and must be merged after it. The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
## Context This is the last of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time. #1703 was merged as a single commit (`5afe754`) covering four unrelated issues. The companion pull request (the revert) undoes that squash so each issue can be merged on its own. This pull request restores **only** the part of #1703 that belongs to: - #1685 — automated test coverage ## What this changes Seven new test suites that cover the behaviour introduced by the other three issues: | Suite | Covers | | --- | --- | | `src/utils/__tests__/decimalAmount.test.ts` | decimal-safe normalisation and comparison, including values exactly at a limit | | `src/utils/__tests__/billingCycleMath.test.ts` | `monthlyEquivalent` / `yearlyEquivalent` for weekly, monthly and yearly cycles | | `src/utils/__tests__/invoiceTotals.test.ts` | `calculateInvoiceTotals`: rounding, an itemised discount subtracted exactly once, and the discount clamp | | `src/services/__tests__/transactionConfirmation.test.ts` | confirmation thresholds, timeouts, reverts and the definitive-failure classification | | `src/services/__tests__/rpcProvider.test.ts` | circuit-breaker transitions, the half-open success threshold and endpoint failover | | `src/store/_tests_/transactionQueueStore.test.ts` | the broadcast marker, exponential backoff and the refusal to re-broadcast an unknown-outcome transaction | | `backend/shared/security/__tests__/outboundUrlGuard.test.ts` | rejection of private, loopback and link-local outbound targets | ## Merge order This is the fifth and last pull request in the split of #1703. It is based on `main` **after** the revert pull request ([#1880](#1880)) and the #1683 ([#1881](#1881)), #1684 ([#1882](#1882)) and #1686 ([#1883](#1883)) pull requests were merged, so its diff contains the #1685 work and nothing else. The suites exercise the modules added by the #1683, #1684 and #1686 pull requests, so this pull request has to come last. Merging it completes the split. After all five pull requests are merged, `main` is identical to its state before the split. The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.
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.
Context
PR #1703 was squash-merged into
mainas a single commit (5afe754), which bundled the fixes for four separate issues:A single squash makes those four changes impossible to review, revert or merge independently: reverting one of them takes the other three with it, and none of them can be traced back to the issue that motivated it.
This pull request is the first step of a revert-and-re-land split. It undoes the squash mechanically and changes nothing else, so that the four issues can be re-landed as four separate pull requests.
What this does
The revert is computed as
main - #1703: for every file in the #1703 patch, the pre-#1703 content is folded back into the current file. Every change that was merged after #1703 is preserved — only the #1703 hunks are undone. The result is the code as it would have been if #1703 had never been merged.main).src/utils/invoiceTotals.tsis kept as a reduced compatibility shim that still exportsisDiscountLineItemandresolveLineTotal, because the itemised-discount work that was merged after fix(billing,payments,security): harden payment, billing and platform correctness #1703 imports them. The full totals module returns in the #1684 pull request.backend/billing/domain/InvoiceGenerator.tsis unchanged: the discount clamping it gained is required by the later itemised-discount work, so it stays where it is.This commit carries no behaviour of its own — it only makes the four issues separable again.
Merge order
This pull request must be merged first. The four follow-up pull requests re-land the work one issue at a time:
Each follow-up pull request is based on
mainafter the pull request before it has been merged, so its diff contains exactly one issue's work and nothing else. After all five are merged,mainis identical to its current state.Notes
mainalready carries two unrelated syntax errors (backend/server.tsandbackend/services/batchChargeService.ts) that predate this work. They are left untouched here.