feat(withdraw): Binance travel-rule (localentity) withdrawal support - #51
Conversation
Binance rejects the standard capital/withdraw/apply endpoint with error -4104 for jurisdictions that require travel-rule metadata (e.g. Australia under AUSTRAC from 2026-07-01). Add opt-in support for the localentity/withdraw/apply endpoint, which carries a per-address questionnaire of beneficiary answers. - Register the localentity endpoint at runtime via ccxt defineRestApi (no ccxt fork change; binance sign() already signs sapi POSTs) - Add a per-exchange travelRule policy section (enabled flag + static questionnaire keyed by destination address), validated against the Australia questionnaire schema at policy-load time - Route withdrawals to the localentity endpoint when enabled; fail closed when enabled but no questionnaire is configured for the address Bumps version to 0.2.15.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds Binance travel-rule support for withdrawals: new types and validation, policy loading for travel-rule rules, endpoint registration, withdrawal routing logic, a policy example, tests, and a version bump. ChangesTravel-Rule Withdrawal Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WithdrawHandler
participant resolveTravelRuleDecision
participant BinanceExchange
Client->>WithdrawHandler: withdraw request
WithdrawHandler->>resolveTravelRuleDecision: resolveTravelRuleDecision(policy, exchange, address)
resolveTravelRuleDecision-->>WithdrawHandler: standard | localentity | denied
alt denied
WithdrawHandler-->>Client: FAILED_PRECONDITION travel_rule_denied
else localentity
WithdrawHandler->>BinanceExchange: withdrawViaLocalEntity(...)
BinanceExchange-->>WithdrawHandler: transaction response
WithdrawHandler-->>Client: withdrawal response
else standard
WithdrawHandler->>BinanceExchange: broker.withdraw(...)
BinanceExchange-->>WithdrawHandler: transaction response
WithdrawHandler-->>Client: withdrawal response
end
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/handlers/execute-action/withdraw.ts`:
- Around line 105-111: The local-entity path in withdraw logic is dropping
exchange-specific withdrawal params while the standard path preserves them. In
the `withdraw` handler, either validate and reject non-empty
`transferValue.params` before calling `withdrawViaLocalEntity`, or update
`withdrawViaLocalEntity` to accept and forward the supported params consistently
with the other withdrawal branch. Use the `withdraw`, `withdrawViaLocalEntity`,
and `transferValue.params` symbols to locate the affected flow.
In `@src/helpers/index.ts`:
- Around line 85-97: The travel-rule entry schema in travelRuleEntrySchema
currently allows any exchange value, which lets non-Binance entries reach
withdrawViaLocalEntity and fail later. Tighten the validation in
src/helpers/index.ts so exchange is restricted to BINANCE only, or add an
equivalent guard in the resolver path that uses travel-rule entries before
routing. Use the existing travelRuleEntrySchema and withdrawViaLocalEntity
symbols to locate and update the validation flow.
In `@src/helpers/travel-rule.ts`:
- Around line 133-178: The localentity withdrawal path in withdrawViaLocalEntity
is dropping extra withdrawal parameters that handleWithdraw already forwards in
the standard path. Add a params field to LocalEntityWithdrawArgs and thread
transferValue.params from the withdraw action into withdrawViaLocalEntity, then
merge those params into the request built in withdrawViaLocalEntity so Binance
memo/tag or wallet-specific options are preserved alongside the existing
coin/address/amount/network/questionnaire fields.
- Around line 91-103: The exchange lookup in travelRule logic can be bypassed by
duplicate entries and can incorrectly apply Binance-only handling to non-Binance
exchanges. Update the resolver in the travel-rule helper to select an enabled
Binance rule only, and avoid falling back to standard or invoking the
localentity path for any non-Binance exchange. Use the existing travel-rule
resolution flow and identifiers like the exchange matching block and the
Binance-only helper entry point to tighten the selection logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: baaff3e6-f656-413f-b955-38549161b353
📒 Files selected for processing (8)
package.jsonpolicy/policy.example.jsonsrc/handlers/execute-action/withdraw.tssrc/helpers/broker.tssrc/helpers/index.tssrc/helpers/travel-rule.tssrc/types.tstest/travel-rule.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-04-14T06:47:01.283Z
Learnt from: csmithington
Repo: usherlabs/cex-broker PR: 38
File: src/client.dev.ts:0-0
Timestamp: 2026-04-14T06:47:01.283Z
Learning: In this codebase, gRPC action constants such as `FetchTicker`, `FetchFees`, and `FetchAccountId` should be sourced from `src/helpers/constants.ts` and imported from there (e.g., used by both `src/client.dev.ts` and `src/server.ts`). Do not import or reference generated proto TypeScript artifacts (for example `./proto/cex_broker/Action`), since those generated files are git-ignored and won’t be available/committed consistently.
Applied to files:
src/helpers/broker.tssrc/handlers/execute-action/withdraw.tssrc/types.tssrc/helpers/index.tssrc/helpers/travel-rule.ts
🪛 ast-grep (0.44.0)
test/travel-rule.test.ts
[warning] 261-261: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(tempPath, JSON.stringify(policy))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🔇 Additional comments (8)
package.json (1)
3-3: LGTM!test/travel-rule.test.ts (2)
258-264: False positive: static analysis path-traversal flag.
tempPathis derived fromos.tmpdir(),Date.now(), andMath.random()— not attacker-controlled input — so the CWE-22 flag onfs.writeFileSyncdoesn't apply here.
1-297: LGTM!policy/policy.example.json (1)
47-64: LGTM!src/types.ts (1)
29-76: LGTM!src/helpers/index.ts (1)
17-17: LGTM!Also applies to: 33-39
src/helpers/broker.ts (1)
7-7: LGTM!Also applies to: 55-56
src/handlers/execute-action/withdraw.ts (1)
2-6: LGTM!Also applies to: 87-100
Addresses review feedback on #51: - travelRuleEntrySchema now only accepts BINANCE (the sole exchange with a localentity endpoint), so a misconfigured non-Binance entry fails at policy load instead of throwing a cryptic "endpoint not registered" error on the first withdrawal. - withdrawViaLocalEntity now forwards caller params (memo/tag, withdrawOrderId, etc.), matching the standard withdraw path. Fixed fields (coin/amount/network/questionnaire) still take precedence on collision.
|
@coderabbitai not chasing the docstring-coverage threshold on this PR. The exported functions ( The 4 actionable inline findings (Binance-only |
|
Thanks for confirming the fixes in 54be075 for the Binance-only ✅ Action performedReview finished.
|
Resolve conflicts combining the ClickHouse research/archive stack (this PR) with the travel-rule deposit reconciler landed on develop (#51-#54): - src/index.ts: keep both CEXBroker fields (brokerArchiver + depositReconciler); both are referenced by the auto-merged body. - .env.sample: keep develop's OTel legacy host/port + travel-rule docs (env vars still consumed by src/helpers/otel.ts) and append this PR's ClickHouse block.
1 participant
Problem
Binance rejects the standard
POST /sapi/v1/capital/withdraw/applyendpoint with error -4104 ("withdrawals are not permitted due to travel rule restrictions") for accounts in jurisdictions that require travel-rule metadata — e.g. Australia under AUSTRAC from 2026-07-01. Once an account's jurisdiction requires it, the standard endpoint stops accepting withdrawals entirely; Binance only acceptsPOST /sapi/v1/localentity/withdraw/apply, which carries an extra requiredquestionnairefield (a JSON string of beneficiary answers).ccxt has no wrapper for the
localentityendpoint.Change
Opt-in support for the travel-rule withdraw endpoint:
travel-rule.ts): registerslocalentity/withdraw/applyon the Binance instance via ccxt's owndefineRestApi. No ccxt fork change — binance'ssign()already signs allsapiprivate POSTs, and the path falls into theurlencodebranch (which URL-encodes the questionnaire JSON, as the spec requires).types.ts,index.ts): new optionaltravelRulesection — a per-exchangeenabledflag plus staticquestionnaireanswers keyed by destination address. The Australia questionnaire is validated at policy-load time (australiaQuestionnaireSchema), so a malformed questionnaire fails startup, not a live withdrawal. Includes an optionaldescriptionfield so operators can document why an entry exists.withdraw.ts): when travel rule is enabled for the exchange, resolve the questionnaire by destination address (case-insensitive) and route to the localentity endpoint. Fails closed — if enabled but no questionnaire is configured for the address, returnsFAILED_PRECONDITIONrather than falling back to the standard endpoint (which would just reproduce -4104).Non-Binance exchanges and accounts with
enabled: falsekeep using the standard endpoint unchanged.Tests
New
test/travel-rule.test.ts(19 tests): AU questionnaire validation (self-owned, individual, corporate, VASP branches + conditional-required/forbidden rules), decision resolution (standard / localentity / fail-closed), endpoint registration (Binance vs no-op), the localentity request shape (incl. questionnaire JSON serialization), andloadPolicyacceptance/rejection.Full suite: 291 pass / 0 fail.
tscandbiomeclean.Notes
travelRulesection, since olderloadPolicyrejects the unknown key.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
0.2.15.