Release: develop -> main#4190
Open
github-actions[bot] wants to merge 9 commits into
Open
Conversation
…onality (#4188) * fix(kyc): auto-complete NationalityData from the account's known nationality The NationalityData KYC step is a standalone entity that is only satisfied by an explicit submission through the step machinery. A RealUnit registration writes userData.nationality but never completes that step, and initiateStep — unlike CONTACT_DATA (from mail) and PERSONAL_DATA (from user fields) — had no auto-completion branch for it. The nationality captured on the registration form was therefore asked again on the dedicated nationality step (double capture) for every fresh RealUnit account. Auto-complete NATIONALITY_DATA in initiateStep when the account's nationality is already known and clean (allowed, non-residence-permit, no step errors), mirroring the sibling CONTACT_DATA/PERSONAL_DATA auto-completes. Residence-permit or disallowed nationalities and merged/blocked accounts are deliberately left IN_PROGRESS so they still go through the normal nationality step, which routes them to internal/manual review and pulls in the residence-permit follow-up step — never a blind complete. The step result stores the same slim { id, symbol } shape as the manual path. * test(kyc): initialize global Config for the NationalityData auto-complete tests The NATIONALITY_DATA branch reads Config.kyc.residencePermitCountries, and the global Config is a module-level let that is undefined until a ConfigService is constructed. Initialize it in beforeAll, matching the existing pattern. * test(kyc): use absolute mock import, drop vacuous review-spy assertion Import createCustomCountry via an absolute path (CONTRIBUTING; matches the convention in the sibling specs and the Country entity import). Remove the reviewNationalityData spy and its not-called assertion: the auto-complete branch never routes through that path, so the assertion was vacuous — the COMPLETED status and single save already cover the intended behaviour.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
#4189) * feat(realunit): show current REALU balance in the compliance dashboard The customer list and the detail dossier now carry the customer's current REALU holdings: sum of the ponder-indexer balances over all wallet addresses of the account (share count, asset decimals applied). The holder set is cached for 5 minutes; when the indexer or the asset lookup is unavailable the balance is reported as unknown instead of failing the list. New bulk user query getUsersByUserDataIds avoids per-customer address lookups. * docs(realunit): note the deliberate check-evidence exception in the reduced-scope header
…ompliance balances (#4191) * fix(realunit): fail closed on empty or truncated indexer sweeps for compliance balances Follow-up to #4189. The REALU balance shown per compliance customer is resolved from the Ponder indexer and cached for 5 minutes; three fail-open weaknesses let a degraded indexer response render a wrong authoritative value instead of "unknown": - A successful-but-empty sweep (HTTP 200, no holders during resync/cold start) was cached and shown as a definitive 0 for real shareholders, contradicting the documented undefined=unresolved contract. Empty sweeps now throw and stay unresolved (not cached). - Pagination stopped silently on a missing/repeated cursor and had no page cap, risking a silent undercount. It now fails closed on a stalled cursor and caps at 100 pages, mirroring the deuro/juice clients but throwing instead of returning a partial set (balance correctness over completeness). - A single unparsable holder balance made fromWeiAmount throw and blanked the whole batch; malformed records are now skipped per address. Adds tests for the empty-sweep, malformed-holder, missing-cursor and dossier-detail paths. * fix(realunit): fail closed per member on an unparsable holder balance Skipping a malformed balance left the affected member on a false authoritative 0 (or a partial-sum undercount), contradicting the undefined=unresolved contract the rest of the change enforces. Any unparsable address now marks that member's total as unresolved (undefined) while the other members stay correct. Adds tests for member-level isolation, the no-partial-sum case, and the 100-page cap guard. * test(realunit): isolate the page-cap and repeated-cursor pagination guards The page-cap test used empty pages, so its undefined result was equally produced by the empty-sweep guard; it now returns non-empty foreign holders so only the page-cap guard can yield undefined. Adds a test for the repeated-cursor stall, asserting it is caught on the second page rather than only after the 100-page cap. * fix(realunit): treat an empty-string holder balance as unresolved, not a false 0 The truthiness guard (`if (!raw) continue`) swallowed an empty-string balance the same way as an absent address, masking an unusable indexer value as an authoritative 0. Now only a truly absent address short-circuits; a present-but-empty balance flows through toShareCount and fails closed (member -> unresolved). Also adds a positive test for cursor-advance + cross-page holder aggregation. * style(realunit): satisfy prettier in the balance spec format:check flagged the object-literal cast in the page-cap test; prettier 3 formats it as `({...}) as HoldersDto`.
…nt KYC lift (#4195) * test(realunit): cover the registration forward self-heal and idempotent KYC lift Two gaps remained after the forward-outside-transaction change: - self-heal on retry: a persist failure after a successful (idempotent-upsert) POST returns false and leaves no blocking row, so the client retry re-POSTs and completes. The load-bearing self-heal property was only asserted in prose. - the in-lock idempotent outcome (a concurrent completion of the same wallet) must still lift the caller's KYC level so its buy/sell gate opens; the existing idempotent test used a level-999 user and never exercised the lift. * test(realunit): correct the self-heal assertion rationale The registerUser POST runs unconditionally outside the persist transaction, so its call count does not prove the retry was not short-circuited. Attribute the self-heal to the second attempt persisting a COMPLETED row instead.
…structural broadcast boundary (all chains) (#4181) * fix(payout): designate before broadcast to prevent double payout on restart Payout strategies broadcast on-chain before persisting any state, so a restart between the broadcast and the save leaves the order re-selectable by the 30s cron and triggers a second broadcast (double payout). Persist PAYOUT_DESIGNATED before the broadcast (mirrors the Bitcoin path) in the EVM, Solana, Tron, Cardano, ICP, Arkade, Spark and Lightning strategies. The !payoutTxId guard leaves the TX_SPEEDUP/expired-retry path untouched so nonce-reuse stays intact; the catch stays fail-closed and never auto-rolls-back. Guard speedupTransaction on PAYOUT_PENDING. No migration needed (PAYOUT_DESIGNATED already exists). * fix(payout): guard speedup to replacement-capable chains, extract designate helper Address review: speedupTransaction now rejects chains without nonce-reuse (supportsSpeedup=false, EVM-only) and when TX_SPEEDUP is disabled, so a manual speedup never broadcasts a second, independent transaction (double payout). Extract the designate-before-broadcast block into a shared PayoutStrategy helper to remove the 8-fold duplication and the EVM-specific comment that was misleading for the non-EVM chains. * test(payout): use toHaveBeenCalled matchers per repo convention * test(payout): cover the reboot double-spend case across all strategies Exhaustive tests for the designate-before-broadcast fix: all 8 payout strategies x the variant matrix (success = designate persisted before the broadcast; broadcast-throws stays PAYOUT_DESIGNATED with no second broadcast and no rollback; payoutTxId-set skips re-designation; designate-save-throws never reaches the broadcast; Lightning also isHealthy=false). Plus the PayoutStrategy helper and supportsSpeedup, speedupTransaction (all guard branches incl. not-found), the EVM retry x guard interaction (OOG fresh nonce vs. expired nonce reuse), and the cron-level reboot guarantee (payoutOrders skips PAYOUT_DESIGNATED, processFailedOrders escalates it to PAYOUT_UNCERTAIN). base/payout.strategy.ts reaches 100% coverage. * test(payout): reach 100% coverage on all changed payout files Cover the case-adjacent code in the touched files so every changed file hits 100% statement/branch/function/line coverage: the full PayoutService cron (doPayout, checkOrderCompletion, fee estimation, prepare/completion phases, getRecentPayoutSentCorrelationIds, group/mail helpers, per-group error isolation across two groups) and estimateFee / checkPayoutCompletionData for all eight strategies (incl. the ICP input-asset and TOKEN/COIN specifics and Lightning's undefined assetType). 225 payout tests pass on Node 20. * fix(payout): self-heal transient pre-broadcast failures via broadcast boundary Chain clients now throw TxBroadcastError only around the actual on-chain send call; the payout service maps it to PayoutBroadcastException. The strategy catch discriminates: an ambiguous broadcast error (at or after send) leaves the order PAYOUT_DESIGNATED for fail-closed escalation, while a provably pre-broadcast error rolls the designation back to PREPARATION_CONFIRMED so the cron self-heals transient RPC failures — only on the first attempt and capped by maxPreBroadcastRetries. Restores the develop-era self-healing that the designate-before-broadcast fix had turned into a permanent strand for transient dispatch errors (RPC timeout, gas estimation, nonce fetch). Covers EVM, Cardano and Solana; adds an empty-tx-hash guard inside the EVM send boundary. * fix(payout): extend broadcast boundary to ICP and Lightning Apply the TxBroadcastError / PayoutBroadcastException boundary to the ICP and Lightning payout paths, so a transient pre-broadcast failure self-heals while an ambiguous at-or-after-send failure stays PAYOUT_DESIGNATED for fail-closed escalation. - ICP: hoist address/amount validation out of the try so only the ledger update call is wrapped; a post-submit network error is ambiguous and the call carries no created_at_time dedup, so it stays fail-closed. - Lightning: wrap the transport-level send in lightning-client; an in-band LND payment_error ("no route") remains a plain error and self-heals. Keysend (LN_NID) has no payment_hash for LND dedup, so its failures are wrapped fail-closed to prevent a double-pay on retry. - Solana: treat an empty tx hash after send as a broadcast error (fail-closed) instead of letting it fall through to a pre-broadcast-style rollback. * fix(payout): extend broadcast boundary to Bitcoin, Firo, Monero, Zano Replace the fragile substring heuristic (e.message.includes('timeout')) in the batch-based bitcoin path with the structural PayoutBroadcastException marker, so the pre/post-broadcast decision no longer depends on error wording: any at-or-after-send failure now keeps the batch PAYOUT_DESIGNATED (fail-closed) while a provable pre-broadcast failure rolls back for self-heal. - Bitcoin/Firo/Testnet4: wrap the actual node broadcast (Core `send` is atomic; Firo's sendrawtransaction / mintspark) in TxBroadcastError, mapped to PayoutBroadcastException in the payout services; fee-estimation stays outside. - Monero/Zano: wrap the atomic wallet-RPC transfer, including response mapping, so a malformed body cannot fall through as a pre-broadcast-style rollback. * fix(payout): extend broadcast boundary to Tron, Arkade, Spark Tron's gateway POST and the Arkade/Spark SDK sends are atomic (sign+broadcast in one call), so any failure from the send on is ambiguous and now surfaces as TxBroadcastError -> PayoutBroadcastException (fail-closed). Their strategies move from a blanket fail-closed catch to handleBroadcastError, so a provable pre-broadcast failure (wallet resolution, decimals, leaf sync) self-heals. - Arkade/Spark: extract the actual broadcast (wallet.sendBitcoin / wallet.transfer) out of the internal reconnect-and-retry wrapper, which would otherwise resend an already-broadcast tx on a connection/channel error -> double-spend. - Guard every send against an empty txId (200 OK without a tx id): treat it as an ambiguous broadcast error instead of returning an empty id that would later roll the order back for re-broadcast, mirroring the Solana empty-hash guard. * test(payout): reach 100% coverage across the payout execution domain Cover the normal (happy-path) execution of every payout strategy and service the broadcast-boundary fix touches or sits next to: the 40+ chain-derivation strategies (dispatch, getters, fee asset), all payout services (completion data, gas, nonce, expiry), the base strategies (batch send, designate/rollback, grouping), and the payout order entity and repository. Every in-scope file is now 100% in statements, branches, functions and lines. * fix(payout): fail-closed on empty Lightning keysend payment hash A keysend (LN_NID) carries no invoice payment_hash, so LND cannot deduplicate a re-broadcast and a self-healing retry would double-pay. sendTransfer returns an empty string for a blank payment hash without throwing, which would otherwise roll the order back to PREPARATION_CONFIRMED and re-broadcast. Guard the empty return (and keep wrapping every keysend error) as a PayoutBroadcastException so the order stays PAYOUT_DESIGNATED (fail-closed). Invoice payouts (LN_URL/LND_HUB) keep self-healing, since their payment_hash lets LND drop a duplicate.
* fix(realunit): require tax residence to cover address country Validate that addressCountry is among the declared tax residences (swissTaxResidence covers CH; countryAndTINs covers other countries). Always persist countryAndTINs on user_data.tin, including when personal data already exists. Multi-residence remains allowed. * fix(realunit): reject CH in countryAndTINs and validate multi-residence TINs Swiss tax residence is covered only by swissTaxResidence. countryAndTINs entries must be non-CH with a non-empty tin, including when multi-residence is declared alongside swissTaxResidence true. * fix(realunit): make user_data.tin updates audit-safe and non-destructive Never clear a non-null TIN by writing null (Swiss-only submissions must not erase prior foreign TINs). Write user_data.tin only after the registration row is durable, and always log previous→next before the column update so every mutation remains reconstructible. * docs(contributing): require auditable, non-destructive DB overwrites Document that mutating stored values is only allowed when the previous value remains recoverable, with before→after auditability and fail-closed behaviour when history would be lost.
* feat(country): enable Philippines onboarding * test(country): harden Philippines onboarding migration
… hit (#4207) * fix(aml): distinguish Scorechain provider failure from real high-risk hit Screening failures (provider 5xx, timeout, quota, misconfiguration) were reduced to the same boolean as a real high-risk hit and surfaced as AmlError.SCORECHAIN_HIGH_RISK, so compliance saw Scorechain hits for transactions that were never screened (no screening row, no PDF). Replace the boolean channel with a ScorechainOutcome enum (Pass/HighRisk/Unavailable) and add AmlError.SCORECHAIN_UNAVAILABLE. Both outcomes stay fail-closed (CRUCIAL -> PENDING / generic ManualCheck, no tipping-off), but the internal classification is now distinct. Closes #4205 * docs(aml): scope the ScorechainUnavailable rationale to the actual failure paths A missing risk threshold makes isHighRisk throw only after the screening row and the compliance PDF have been persisted, so the blanket claim that neither exists was wrong for that path. Also drop a tautological assertion from both preparation specs. * docs(aml): keep provider/infrastructure on one line in the Scorechain spec A soft wrap right after the slash rendered as "provider/ infrastructure".
…data.tin on a rejected request (#4210) * fix(realunit): bound the tax-residence payload and stop writing user_data.tin on a rejected request Three defects in the tax-residence contract: - user_data.tin is varchar(256), but neither CountryAndTin.tin nor countryAndTINs was length-bounded and the client allows an unbounded number of tax residences. Seven entries with an 11-digit TIN already serialize to 260 characters, so the UPDATE threw 'value too long for type character varying(256)' — AFTER forwardRegistration had already persisted the registration, turning a completed registration into a 500 that every retry reproduced. Bound the input (10 entries, 64-character TIN), widen the column to varchar(1024) to cover that bounded maximum, and reject an oversized payload up front in validateRegistrationDto so it can never fail once the registration is durable. - The idempotent branch of completeRegistration wrote user_data.tin BEFORE idempotentRegistrationResult rejects a mismatching signature, so a request answered with a 400 had already mutated the column. countryAndTINs is not part of the EIP-712 envelope, so the body cannot describe what was actually registered either. Check the signature first and sync the snapshot from the durable registration row instead of the request body. - @ValidateIf(!swissTaxResidence) disabled every validator on countryAndTINs — including @isarray — whenever swissTaxResidence was true, so a non-array body reached the service and crashed it with a TypeError (500 instead of 400). Validate whenever the field is required or present, and keep a defensive Array.isArray guard in the service. Also replace the unguarded JSON.parse(user_data.tin) in the registration prefill: a malformed legacy value took down the only endpoint through which the user could submit a corrected declaration. It now degrades loudly (PII-free error log) to 'no known tax residences'. * fix(realunit): trim tin without crashing on a non-string value The @Transform(Util.trim) added to CountryAndTin.tin runs in class-transformer before class-validator, so a non-string tin (e.g. a number) made value.trim() throw a TypeError => HTTP 500 before @IsString could reject it as a 400 — the exact 500-instead-of-400 regression this change set exists to remove. Trim only actual strings and let any other type fall through to @IsString. * style(realunit): apply prettier to the non-string-tin regression test
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
3 participants
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist