Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Missing any of these = changes requested.
- **Clean API** — use DTOs, never expose entities, document with ApiProperty
- **No over-engineering** — don't build it if the existing solution works
- **Use existing packages** — use `@dfx.swiss/*` packages instead of duplicating logic
- **Auditable DB mutations** — no destructive overwrites; previous values must stay recoverable (see [Database / TypeORM → Auditable mutations](#auditable-mutations--no-destructive-overwrites-critical))

---

Expand Down Expand Up @@ -563,6 +564,29 @@ async process(data: OrderData): Promise<Transaction> { ... }

## Database / TypeORM

### Auditable mutations — no destructive overwrites (CRITICAL)

Overwriting a stored value is only allowed when the **previous** value remains
**recoverable elsewhere in the database**. Every mutation must stay reconstructible:
it must always be clear **when** a field changed, **from which** previous value, and
**to which** new value.

| Rule | Requirement |
|------|-------------|
| **No silent data loss** | If an overwrite can destroy information that is not still queryable on another durable row/event, it is a **bug**. |
| **Event before snapshot** | Write the immutable event first (e.g. registration `signedPayload`, superseded history rows, append-only log). Only then update a denormalised “current” column. |
| **Before → after audit** | For mutable snapshot columns, record `previous` and `next` (and identity/context) **before** the column update. If the audit write fails, do **not** change the column (fail closed). |
| **No destructive clear** | Do not set a non-null field to `null`/empty when the new business event does not itself retain that prior payload (e.g. clearing `user_data.tin` on a Swiss-only registration that omits `countryAndTINs`). |

**Allowed pattern:** dual-store current snapshot + immutable history.

- **History / event of record** — append-only or supersede-with-history rows (old row stays, `active = false`, payload intact).
- **Snapshot** — query-friendly “current” column, updated only after history is durable and only with an audit trail for the transition.

**Not allowed:** in-place `UPDATE` that replaces or nulls a field when the old value exists nowhere else; “stale cleanup” that drops data without a recoverable prior record.

When reviewing PRs that touch persistence, ask: *If this write runs, can an investigator still reconstruct the previous value and the change time from the DB alone?* If not, reject the change.

### Entity Patterns

```typescript
Expand Down
25 changes: 21 additions & 4 deletions docs/specs/scorechain-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,19 @@ and consistent — no untyped failures, no silent passes:
it must never be returned to customers or partner webhooks; only Support/Compliance read it
(e.g. the `RoleGuard(SUPPORT)`-gated support-issue data endpoint). Do not add a Scorechain-named
`AmlReason` and do not surface `comment` in any customer-facing DTO.

**HighRisk vs. Unavailable (DFXswiss/api#4205):** a real Scorechain hit and a
provider/infrastructure failure (5xx, timeout, quota reached, misconfiguration,
unexpected throw) are different events and must not collapse into the same classification —
a Prod incident showed a Buy-Crypto tx routed to manual review with comment
`ScorechainHighRisk` although Scorechain had returned HTTP 500 and no
`scorechain_screening` row or compliance PDF ever existed for it. The
call-site now returns a `ScorechainOutcome` (`PASS` / `HIGH_RISK` / `UNAVAILABLE`,
`scorechain-outcome.enum.ts`) instead of a boolean; `HIGH_RISK` still maps to
`AmlError.SCORECHAIN_HIGH_RISK`, and the new `AmlError.SCORECHAIN_UNAVAILABLE` (mapped
identically: `{ type: CRUCIAL, amlCheck: PENDING, amlReason: MANUAL_CHECK }`) covers the failure
case. Customer-facing fail-closed behaviour is unchanged (both still route to manual review);
only the internal classification — visible in `comment` and stats — is now accurate.
2. **TMS depth:** the **synchronous `scoringAnalysis` gate is the only implemented flow**. The
async TMS workflow (`register*` → scenarios/alerts) is a license-gated feature
(`NotIncludedInLicense`) that DFX does **not** license. Its placeholder client methods, the
Expand All @@ -381,10 +394,14 @@ and consistent — no untyped failures, no silent passes:
`SCORECHAIN_MONTHLY_CHECK_LIMIT` (or a provider/transport error) throws
`ServiceUnavailableException` inside the screening service. The AML call-site
(`screenScorechain` in `buy-crypto-preparation` / `buy-fiat-preparation`) catches it and
returns high risk, so the tx is routed to manual review (`AmlError.SCORECHAIN_HIGH_RISK` →
`CheckStatus.PENDING`) instead of throwing out of the AML computation. Letting the throw escape
the compute path would silently stall settlement of every otherwise-passing tx on every cron
run, so the error is deliberately caught at the seam.
returns `ScorechainOutcome.UNAVAILABLE`, so the tx is routed to manual review
(`AmlError.SCORECHAIN_UNAVAILABLE` → `CheckStatus.PENDING`) instead of throwing out of the AML
computation — deliberately **not** `SCORECHAIN_HIGH_RISK`, since no risk verdict could be
established: for a transport/quota error neither a `scorechain_screening` row nor a compliance
PDF exists, but for a missing risk-threshold configuration both may already have been
persisted before the throw. Letting the throw escape the compute path would
silently stall settlement of every otherwise-passing tx on every cron run, so the error is
deliberately caught at the seam.
5. **ikna coexistence:** run both fully in parallel (no routing rule); Scorechain is additive.
6. **Live pipeline injection (call-sites):** the async screening is wired into the synchronous
AML pipeline (`screenScorechain` is threaded through `amlCheckAndFillUp` for every buy/sell that
Expand Down
41 changes: 41 additions & 0 deletions migration/1784000000000-WidenUserDataTin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @typedef {import('typeorm').MigrationInterface} MigrationInterface
* @typedef {import('typeorm').QueryRunner} QueryRunner
*/

/**
* Widens `user_data.tin` from varchar(256) to varchar(1024). The RealUnit multi-tax-residence
* declaration is stored as a JSON array of {country, tin} in this column; 256 characters is no
* longer enough from ~7 entries on (seven 11-digit TINs already serialize past 256). Widening a
* varchar length in Postgres is a catalog-only change with no table rewrite.
*
* down() fails loud if longer values already exist — that is intentional (fail-closed: do not
* silently truncate tax-residence snapshots).
*
* @class
* @implements {MigrationInterface}
*/
module.exports = class WidenUserDataTin1784000000000 {
name = 'WidenUserDataTin1784000000000'

/**
* @param {QueryRunner} queryRunner
*/
async up(queryRunner) {
// Fail fast instead of head-of-queue-blocking every "user_data" write if the ALTER is
// contended at deploy time: the timeout aborts with a clear lock_timeout error instead of
// hanging app boot.
await queryRunner.query(`SET LOCAL lock_timeout = '5s'`);
await queryRunner.query(`ALTER TABLE "user_data" ALTER COLUMN "tin" TYPE character varying(1024)`);
}

/**
* @param {QueryRunner} queryRunner
*/
async down(queryRunner) {
// Fails loud if any row already stores more than 256 characters — intentional; do not
// silently truncate a multi-tax-residence snapshot.
await queryRunner.query(`SET LOCAL lock_timeout = '5s'`);
await queryRunner.query(`ALTER TABLE "user_data" ALTER COLUMN "tin" TYPE character varying(256)`);
}
}
27 changes: 27 additions & 0 deletions migration/1784029705806-EnablePhilippinesOnboarding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Enable individual DFX onboarding for the Philippines.
//
// The Philippines is no longer subject to FATF increased monitoring and its country row is already
// FATF-enabled. The remaining dfxEnable=false flag keeps it out of the DFX KYC country list and also
// makes bankAllowed false in the public country DTO. Keep organization onboarding and all unrelated
// country controls unchanged. This regulatory allow-list change is intentionally one-way: reverting an
// application deployment must not silently block the country again. A future restriction requires its
// own explicit migration and compliance review.

/**
* @typedef {import('typeorm').MigrationInterface} MigrationInterface
* @typedef {import('typeorm').QueryRunner} QueryRunner
*/

module.exports = class EnablePhilippinesOnboarding1784029705806 {
name = 'EnablePhilippinesOnboarding1784029705806';

async up(queryRunner) {
await queryRunner.query(
`UPDATE "country" SET "dfxEnable" = true, "updated" = NOW() WHERE "symbol" = 'PH' AND "dfxEnable" = false`,
);
}

async down(_queryRunner) {
// Intentionally empty: country restrictions are compliance data and must be changed explicitly.
}
};
7 changes: 7 additions & 0 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,13 @@ export class Configuration {
],
};

payout = {
// Cap on auto-retries for a payout order that fails provably before the on-chain send call
// (gas estimation, nonce fetch, gasPrice RPC). Beyond this, a permanently failing pre-broadcast
// step (e.g. gas-estimation revert) escalates to PAYOUT_UNCERTAIN instead of retrying forever.
maxPreBroadcastRetries: +(process.env.PAYOUT_MAX_PRE_BROADCAST_RETRIES ?? 3),
};

blockchain = {
default: {
user: process.env.NODE_USER,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Wallet } from '@arkade-os/sdk';
import { TxBroadcastError } from 'src/integration/blockchain/shared/errors/tx-broadcast.error';
import { ArkadeClient } from '../arkade-client';

// Mock the SDK to provide InMemory repositories
Expand Down Expand Up @@ -105,6 +106,100 @@ describe('ArkadeClient', () => {
});
});

// --- SEND TRANSACTION - BROADCAST BOUNDARY --- //
// wallet.sendBitcoin signs AND broadcasts atomically inside the SDK, so it must never be retried by
// this.call()'s reconnect-and-retry (which would resend an already-broadcast tx). Any failure at or
// after that call is wrapped into a TxBroadcastError; wallet resolution failures never reach
// sendBitcoin and stay plain errors.

describe('sendTransaction - broadcast boundary', () => {
it('wraps a sendBitcoin failure into a TxBroadcastError, keeping message and cause', async () => {
const cause = new Error('unexpected rejection');
mockWallet.sendBitcoin.mockRejectedValue(cause);

let error: unknown;
try {
await client.sendTransaction('ark1destination', 0.5);
} catch (e) {
error = e;
}

expect(error).toBeInstanceOf(TxBroadcastError);
expect((error as TxBroadcastError).message).toBe('unexpected rejection');
expect((error as TxBroadcastError).cause).toBe(cause);
});

it('wraps a non-Error rejection using String(e)', async () => {
mockWallet.sendBitcoin.mockRejectedValue('plain string rejection');

let error: unknown;
try {
await client.sendTransaction('ark1destination', 0.5);
} catch (e) {
error = e;
}

expect(error).toBeInstanceOf(TxBroadcastError);
expect((error as TxBroadcastError).message).toBe('plain string rejection');
});

it('never resends when sendBitcoin fails with a connection-classified error (fail-closed, no double-send), but resets the cache so the next call reconnects', async () => {
mockWallet.sendBitcoin.mockRejectedValue(new Error('connection lost'));

await expect(client.sendTransaction('ark1destination', 0.5)).rejects.toBeInstanceOf(TxBroadcastError);
expect(mockWallet.sendBitcoin).toHaveBeenCalledTimes(1); // never resent within this call

const freshWallet = { ...mockWallet, sendBitcoin: jest.fn().mockResolvedValue('tx-fresh') };
jest.spyOn(Wallet, 'create').mockResolvedValue(freshWallet as any);

const result = await client.sendTransaction('ark1destination', 0.5);

expect(result).toEqual({ txid: 'tx-fresh', fee: 0 }); // next call picked up the fresh wallet
expect(freshWallet.sendBitcoin).toHaveBeenCalledTimes(1);
});

it('does not reset the cache when sendBitcoin fails with an unrelated error', async () => {
mockWallet.sendBitcoin.mockRejectedValue(new Error('insufficient funds'));

await expect(client.sendTransaction('ark1destination', 0.5)).rejects.toBeInstanceOf(TxBroadcastError);

mockWallet.sendBitcoin.mockResolvedValue('tx-retry');
await client.sendTransaction('ark1destination', 0.5);

// still the same wallet instance from Wallet.create - no reinitialization happened
expect(Wallet.create).toHaveBeenCalledTimes(1);
});

it('propagates a wallet resolution failure as a plain (non-TxBroadcastError) error and resets the cache, never reaching sendBitcoin', async () => {
jest.spyOn(Wallet, 'create').mockRejectedValue(new Error('server down'));
const freshClient = new ArkadeClient();

let error: unknown;
try {
await freshClient.sendTransaction('ark1destination', 0.5);
} catch (e) {
error = e;
}

expect(error).toBeInstanceOf(Error);
expect(error).not.toBeInstanceOf(TxBroadcastError);
expect((error as Error).message).toBe('server down');
expect(mockWallet.sendBitcoin).not.toHaveBeenCalled();

// reset proof: the next attempt re-initializes instead of reusing the permanently-rejected AsyncField
jest.spyOn(Wallet, 'create').mockResolvedValue(mockWallet as any);
const result = await freshClient.sendTransaction('ark1destination', 0.5);
expect(result).toEqual({ txid: 'tx-abc123', fee: 0 });
});

it('wraps an empty txid from sendBitcoin into a TxBroadcastError (fail-closed silent failure)', async () => {
mockWallet.sendBitcoin.mockResolvedValue('');

await expect(client.sendTransaction('ark1destination', 0.5)).rejects.toBeInstanceOf(TxBroadcastError);
expect(mockWallet.sendBitcoin).toHaveBeenCalledTimes(1);
});
});

// --- GET TRANSACTION --- //

describe('getTransaction', () => {
Expand Down
36 changes: 33 additions & 3 deletions src/integration/blockchain/arkade/arkade-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { DfxLogger } from 'src/shared/services/dfx-logger';
import { AsyncField } from 'src/shared/utils/async-field';
import { BlockchainTokenBalance } from '../shared/dto/blockchain-token-balance.dto';
import { SignedTransactionResponse } from '../shared/dto/signed-transaction-reponse.dto';
import { TxBroadcastError } from '../shared/errors/tx-broadcast.error';
import { BlockchainClient } from '../shared/util/blockchain-client';

export interface ArkadeTransaction {
Expand Down Expand Up @@ -52,16 +53,45 @@ export class ArkadeClient extends BlockchainClient {
// --- TRANSACTION METHODS --- //

async sendTransaction(to: string, amount: number): Promise<{ txid: string; fee: number }> {
return this.call(async (wallet) => {
const amountSats = Math.round(amount * 1e8);
// Pre-broadcast: resolving the wallet handle never calls sendBitcoin, so a failure here is
// provably pre-broadcast. Reset the cache so the next retry gets a fresh connection attempt
// instead of repeating a possibly permanently-rejected cached promise.
let wallet: Wallet;
try {
wallet = await this.wallet;
} catch (e) {
this.wallet.reset();
this.cachedAddress.reset();
throw e;
}

const amountSats = Math.round(amount * 1e8);

// Broadcast boundary: wallet.sendBitcoin signs AND broadcasts atomically inside the SDK - there
// is no separate pre-broadcast step to isolate, so any failure from here on is ambiguous (the
// tx may already be on-chain). Deliberately NOT routed through this.call()'s reconnect-and-retry:
// that wrapper re-invokes the whole operation on a connection-classified error, which for a
// broadcast would risk sending twice. Reset the cache on such an error so only the *next* call
// reconnects, without resending this one.
try {
const txid = await wallet.sendBitcoin({
address: to,
amount: amountSats,
});

// An empty txid from the SDK is an ambiguous silent failure - fail-closed rather than
// returning it and letting the empty id roll the order back for re-broadcast.
if (!txid) throw new TxBroadcastError('Arkade broadcast returned an empty txid');

return { txid, fee: 0 };
});
} catch (e) {
if (e instanceof TxBroadcastError) throw e;
if (e?.message?.includes('disconnected') || e?.message?.includes('connection')) {
this.wallet.reset();
this.cachedAddress.reset();
}
throw new TxBroadcastError(e instanceof Error ? e.message : String(e), { cause: e });
}
}

async getTransaction(txId: string): Promise<ArkadeTransaction> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { HttpService } from 'src/shared/services/http.service';
import { TxBroadcastError } from '../../../shared/errors/tx-broadcast.error';
import { BitcoinClient } from '../bitcoin-client';

// Mock Config and GetConfig
Expand Down Expand Up @@ -285,6 +286,48 @@ describe('BitcoinClient', () => {

expect(result).toBe('newtxid123');
});

// Bitcoin Core's `send` RPC builds, signs and broadcasts atomically in one node call - a
// failure here (including an HTTP-level timeout) is ambiguous, so it must surface as
// TxBroadcastError rather than a plain Error, mirroring the Cardano/Solana broadcast boundary.
it('should wrap a failure of the underlying `send` RPC call into a TxBroadcastError', async () => {
// BitcoinRpcClient.call() itself wraps any transport-level rejection into a plain Error
// prefixed with "Bitcoin RPC <method> failed: ..." before it reaches sendMany - that
// intermediate Error is what our TxBroadcastError boundary observes and preserves as cause.
const payload = [{ addressTo: 'bc1qaddr1', amount: 0.1 }];

mockRpcPost.mockImplementationOnce(() => Promise.resolve({ result: null, error: null, id: 'test' })); // walletpassphrase
mockRpcPost.mockImplementationOnce(() => Promise.reject(new Error('Invalid amount')));

let error: unknown;
try {
await client.sendMany(payload, 10);
} catch (e) {
error = e;
}

expect(error).toBeInstanceOf(TxBroadcastError);
expect((error as TxBroadcastError).message).toBe('Bitcoin RPC send failed: Invalid amount');
expect((error as TxBroadcastError).cause).toBeInstanceOf(Error);
expect(((error as TxBroadcastError).cause as Error).message).toBe('Bitcoin RPC send failed: Invalid amount');
});

it('should wrap a non-Error rejection of the `send` RPC call into a TxBroadcastError via String(e)', async () => {
const payload = [{ addressTo: 'bc1qaddr1', amount: 0.1 }];

mockRpcPost.mockImplementationOnce(() => Promise.resolve({ result: null, error: null, id: 'test' })); // walletpassphrase
mockRpcPost.mockImplementationOnce(() => Promise.reject('node unreachable'));

let error: unknown;
try {
await client.sendMany(payload, 10);
} catch (e) {
error = e;
}

expect(error).toBeInstanceOf(TxBroadcastError);
expect((error as TxBroadcastError).message).toBe('Bitcoin RPC send failed: node unreachable');
});
});

// --- testMempoolAccept() Tests --- //
Expand Down
Loading
Loading