[hippocampusSK]

Bitget Wallet users can now connect Sui, TON and XRP, on top of Bitcoin, Solana, Tron and the EVM chains.
Connecting is more reliable: chains are asked one at a time with a short pause, and a chain the wallet cannot offer no longer breaks the rest.
Solana, Tron and XRP transactions are now checked against what was requested before they are sent, which also fixes Tron sends that were being rejected.
Cosmos, NEAR and Starknet are left out because the wallet does not expose them yet; they are parked in a follow-up.

Linear: API-3829

[hippocampusSK]

/dominion review

[ai-reviewer-towan]

PR review · published

Outcome: failed. 6 findings: 2 high, 1 medium, 3 low.

Open the published review for the TL;DR, findings and fixes.

Gate	Model	Status
logic	codex-cli gpt-6.1-sol	❌ failed
adversarial	codex-cli gpt-6.1-sol	❌ failed
code-style-complexity	claude-code claude-opus-5-5	✅ passed

Run details

• Run: 8177052d-bd37-447d-8ad5-8074114cc3a9
• Head: c45ee9dea927
• Base: 3c61e954240c
• Check: review-runner

[ai-reviewer-towan]

TL;DR

This PR adds Sui, TON and XRP to Bitget Wallet, connects chains one at a time, and checks Solana, Tron and XRP transactions before sending. The biggest problem: XRP's integrity check can be bypassed through shared nested fields, and a sandbox probe confirmed it. Real SDK, codec, signing and broadcast behavior was not verified, and the bun.lock contents were not inspected.

Review failed. At least one gate reported a blocking finding. 6 findings: 2 high, 1 medium, 3 low.

High

XRP integrity check misses in-place changes to nested fields

packages/wallet-extensions/src/bitget/helpers.ts:238 · logic

The shallow copy shares nested transaction fields with the provider. If the provider changes Memos[0].Memo.MemoData or an issued-currency Amount.value in place before it returns the signed blob, transaction changes too. expected is built after the await, so the comparison accepts the altered values. The modified blob is then returned for broadcast, which bypasses the requested-transaction integrity check.

Fix: Take a deep snapshot before calling the provider. Pass a separate deep copy to signTransaction. Compare the decoded result against the untouched snapshot plus the allowed autofill fields. Add tests for in-place mutation of nested fields.

Probe confirms nested XRP memo and amount changes pass the check

packages/wallet-extensions/src/bitget/helpers.ts:238 · adversarial

The provider gets only a shallow copy, and the expected transaction is built after awaiting it. Nested fields stay shared with the caller and with the comparison baseline. The connector probe changed Memos[0].Memo.MemoData and Amount.value in place. Both altered blobs were accepted, and both original requests were modified. Scalar changes and independently cloned changes were correctly rejected. This bypasses the new XRP request-integrity safeguard. The probe isolates the aliasing only, not real XRPL encoding or broadcast.

Fix: Deep-clone the full expected request before calling the provider, and pass a separate deep clone to the provider. Compare the decoded result against the untouched snapshot, keeping only the intended allowances for wallet-filled fields. Add tests that mutate nested memo and amount fields in place and require both rejection and unchanged caller input.

Medium

Any non-200 Tron response is treated as user rejection and stops connect

packages/wallet-extensions/src/bitget/helpers.ts:136 · logic

Every non-200 Tron response is classified as a user rejection, including unsupported-method and disconnected-chain responses. The new sequential connect loop rethrows this classification immediately. So connecting [Tron, Ethereum] stops without trying Ethereum when Tron returns a failure that is not a cancellation. That contradicts the intended chain-skipping behavior.

Fix: Classify only the real user-rejection response as wallet_connection_rejected_by_user. Use a non-rejection error for other Tron failures so the loop can skip Tron and continue. Add connection tests for both cancellation and non-cancellation responses.

Low

Solana signing repeats the same array check and casts twice

packages/wallet-extensions/src/bitget/helpers.ts:96 · code-style-complexity

Solana signTransaction checks Array.isArray(response) three times: to read bytes (line 96), in the guard (line 97), and in a nested ternary (lines 99-103) that also branches on transaction type. It casts bytes as Uint8Array twice because the narrowing is lost between steps. The two response shapes, Phantom-style and Wallet Standard, are hard to follow, and the casts hide whether bytes was actually checked.

Fix: Convert the response to a transaction in a single branch: start with let signed: BitgetSolanaTransaction = response as BitgetSolanaTransaction, then inside if (Array.isArray(response)) read response[0]?.signedTransaction, throw transactionFailedError(...) if it is missing, and deserialize with VersionedTransaction.deserialize or SolanaTransaction.from depending on "version" in transaction. You can also move this into a small toSignedTransaction(response, transaction) helper. Either way removes the nested ternary and the casts.

Tron rejection skips the new connectionRejectedError helper

packages/wallet-extensions/src/bitget/helpers.ts:136 · code-style-complexity

This PR adds a connectionRejectedError(chain) helper (line 35), and Bitcoin uses it. The Tron path still builds new SwapKitError("wallet_connection_rejected_by_user", { message }) by hand. That error has no chain or wallet info, so the same failure carries different details on different chains. It is also the one rejection path that isUserRejection in index.ts relies on to stop the connect loop.

Fix: Throw through the shared helper on the Tron path, for example throw connectionRejectedError(Chain.Tron). If the wallet's message is still useful, add an optional message parameter to connectionRejectedError and pass response.message. Do not build the error inline.

EVM chain check is duplicated across index.ts and helpers.ts

packages/wallet-extensions/src/bitget/index.ts:82 · code-style-complexity

index.ts adds a private isEvmChain, while helpers.ts line 263 repeats the same check inline as EVMChains.includes(chain as EVMChain). The Bitget module now has two copies of a rule that controls both dispatch and connect order. The copies can drift apart, for example if one is later narrowed to a Bitget-specific EVM subset.

Fix: Define isEvmChain once in bitget/helpers.ts and export it. Use it in getWalletMethods (if (isEvmChain(chain)) return getEvmWalletMethods(chain as EVMChain)), import it into index.ts and delete the local copy.

Review coverage and raw evidence

Reviewed head c45ee9dea92729d17ea90443fdb6760339efd48a against base 3c61e954240c075cbd6c1d24ca63e0829d3e08d9. Run 8177052d-bd37-447d-8ad5-8074114cc3a9.

logic — failed (2 findings)

Reviewed all changed files, relevant callers, signing guards, connection error handling, exports/types, dependencies and test preservation. Cosmos removal is intentional; no tests were deleted or weakened. Graph context was available but bounded; Argus coverage was partial, omitting 172 reference files. External SDK/provider runtime behavior remains unverified; no repository code was executed.

adversarial — failed (1 findings)

Inspected all changed files, preservation changes, connector workflow and related SDK/helper callers. Supplied behavior probes meaningfully exercised failure isolation and cancellation; both baseline-controlled Tron mutations were caught. The XRP probe demonstrated an integrity bypass. No tests were deleted; Cosmos removal is explicitly intentional. Graph context is available but incomplete; Argus coverage is partial. External dependencies were doubled: real codec/signature/broadcast compatibility, SDK integration, Solana signing and successful TON transfers remain unverified.

Experiment probe-1 (behavior): passed, exit 0, 1960 ms.

Experiment probe-2 (behavior): violated, exit 1, 1725 ms.

Experiment probe-3 (test-quality): passed, exit 0, 1495 ms.

code-style-complexity — passed (3 findings)

I reviewed the Bitget changes for structure and repository style, reading the full head versions of bitget/index.ts, bitget/helpers.ts and bitget/types.ts and the types.ts hunks in the local patch. The move from one large ts-pattern match to a function per chain dispatched from getWalletMethods follows the existing onekey/helpers.ts pattern and is easier to follow. Moving the Bitget provider types out of the global window declaration into bitget/types.ts also tightens the code.

The connect loop in index.ts is more involved than before: it sorts EVM chains last, pauses between chains and collects failures. Each step has a stated reason, and none of it can be removed without changing behaviour.

I found three low-severity issues, all inside the changed helpers:

• Solana signing: the response handling uses a nested ternary and checks the same condition three times.
• Tron connect rejection: the error is built by hand instead of with the connectionRejectedError helper this PR adds. Bitcoin uses the helper, so the two errors carry different details.
• Duplicated EVM check: index.ts adds its own EVM-chain test, while helpers.ts repeats the same check inline.

Coverage limits:

• bun.lock is listed as a diff-only source. I did not inspect lockfile contents, the full dependency graph or package.json beyond the patch headers.
• The .dominion-review.context.json graph context was not consulted. I traced callers by ordinary search: getWalletMethods is the only caller of the per-chain functions, and other wallets use the same EVM-dispatch pattern.
• The Argus pass reports partial coverage (coverage_incomplete; 3 unsupported files: the changeset, bun.lock and package.json). Its three behaviour-preservation candidates use placeholder line-1 anchors and fall outside this structural review. The one visible removal, Cosmos, is intentional and stated in the PR description, so I did not report it as a regression.

Original specialist findings

F1 · high · logic · packages/wallet-extensions/src/bitget/helpers.ts:238

Issue: The shallow copy shares nested transaction fields with the provider. If the provider changes Memos[0].Memo.MemoData or an issued-currency Amount.value in place before returning its signed blob, it also changes transaction. Because expected is constructed after the await, the comparison accepts the altered values and returns the modified blob for broadcast, bypassing the requested-transaction integrity check.

Solution: Capture a deep snapshot before calling the provider, pass a separate deep copy to signTransaction, and compare the decoded result against the untouched snapshot plus permitted autofill fields. Add tests for in-place mutation of nested fields.

F2 · high · adversarial · packages/wallet-extensions/src/bitget/helpers.ts:238

Issue: The provider receives only a shallow copy, and the expected transaction is constructed after awaiting it. Nested fields therefore remain shared with the caller and comparison baseline. The supplied connector probe mutated Memos[0].Memo.MemoData and Amount.value in place: both altered blobs were accepted and both original requests were modified. Scalar and independently cloned changes were correctly rejected. This bypasses the new XRP request-integrity safeguard; the observation isolates aliasing rather than real XRPL encoding or broadcast.

Solution: Deep-clone the complete expected request before invoking the provider, and pass a separate deep clone to the provider. Compare the decoded result against the untouched snapshot, retaining only the intended wallet-filled-field allowances. Add in-place nested memo and amount mutation tests that require rejection and unchanged caller input.

F3 · medium · logic · packages/wallet-extensions/src/bitget/helpers.ts:136

Issue: Every non-200 Tron response is classified as user rejection, including unsupported-method or disconnected-chain responses. The new sequential connection loop immediately rethrows this classification. Consequently, connecting [Tron, Ethereum] stops without attempting Ethereum when Tron returns such a non-cancellation failure, contrary to the intended chain-skipping behavior.

Solution: Classify only the actual user-rejection response as wallet_connection_rejected_by_user. Represent other Tron failures with a non-rejection error so the loop can skip Tron and continue. Cover both cancellation and non-cancellation responses in connection tests.

F4 · low · code-style-complexity · packages/wallet-extensions/src/bitget/helpers.ts:96

Issue: Solana signTransaction checks Array.isArray(response) three times: once to read bytes (line 96), once in the guard (line 97), and again in a nested ternary (lines 99-103) that also branches on the transaction type. It also casts bytes as Uint8Array twice because the narrowing is lost between these steps. The two possible response shapes (Phantom-style or Wallet Standard) are hard to follow, and the casts hide whether bytes really was checked.

Solution: Turn the response into a transaction in one branch. For example, write let signed: BitgetSolanaTransaction = response as BitgetSolanaTransaction; if (Array.isArray(response)) { const bytes = response[0]?.signedTransaction; if (!bytes) throw transactionFailedError(...); signed = "version" in transaction ? VersionedTransaction.deserialize(bytes) : SolanaTransaction.from(bytes); }. You could also move this into a small toSignedTransaction(response, transaction) helper. Either way the nested ternary and the casts go away.

F5 · low · code-style-complexity · packages/wallet-extensions/src/bitget/helpers.ts:136

Issue: This PR adds a connectionRejectedError(chain) helper (line 35), and Bitcoin uses it. The Tron rejection still builds new SwapKitError("wallet_connection_rejected_by_user", { message }) by hand. That error has no chain or wallet info, so the same failure carries different details depending on the chain. It is also the one rejection path that isUserRejection in index.ts depends on to stop the connect loop.

Solution: Throw the error through the shared helper on the Tron path, for example throw connectionRejectedError(Chain.Tron). If the wallet's message is still useful, add an optional message parameter to connectionRejectedError and pass response.message to it. Building the error inline here should not be needed.

F6 · low · code-style-complexity · packages/wallet-extensions/src/bitget/index.ts:82

Issue: index.ts adds a private isEvmChain, while helpers.ts line 263 repeats the same EVM check inline as EVMChains.includes(chain as EVMChain). As a result, the Bitget module has two copies of one rule that decides both dispatch and connect order. They can drift apart, for example if one copy is later changed to a Bitget-specific EVM subset.

Solution: Define isEvmChain once in bitget/helpers.ts and export it. Use it in getWalletMethods (if (isEvmChain(chain)) return getEvmWalletMethods(chain as EVMChain)), import it into index.ts and delete the local copy.

[hippocampusSK]

/dominion review

[ai-reviewer-towan]

PR review · published

Outcome: failed. 5 findings: 2 medium, 3 low.

Open the published review for the TL;DR, findings and fixes.

Gate	Model	Status
logic	codex-cli gpt-6.1-sol	❌ failed
adversarial	codex-cli gpt-6.1-sol	✅ passed
code-style-complexity	claude-code claude-opus-5-5	❌ failed

Run details

• Run: 597baf6f-2efa-4a27-b7c7-9c469afae337
• Head: 443b455bf56d
• Base: 3c61e954240c
• Check: review-runner

[ai-reviewer-towan]

TL;DR

This PR adds Sui, TON and XRP to Bitget Wallet, connects chains one at a time, and checks Solana, Tron and XRP transactions before sending. The biggest problem: the XRP check accepts wallet-added flags such as tfPartialPayment when the request has no Flags. Adversarial sandbox probes and mutation tests passed with mocked dependencies. Real Solana and XRP integration, broadcasting and successful TON transactions were not verified.

Review failed. At least one gate reported a blocking finding. 5 findings: 2 medium, 3 low.

Medium

XRP integrity check accepts wallet-added flags like partial payment

packages/wallet-extensions/src/bitget/helpers.ts:251 · logic

When a request omits Flags, this loop accepts any flags the wallet adds. For example, the wallet can add tfPartialPayment (0x00020000) to an issued-currency Payment. That passes the comparison but allows delivery of less than the requested Amount. The new transaction-integrity check therefore accepts a payment whose meaning has changed.

Fix: Remove Flags from the unrestricted autofill list. Treat omitted flags as zero. Allow only nonsemantic defaults you have explicitly identified, such as the canonical-signature bit. Add a test that rejects a partial-payment flag added by the wallet.

connectBitget duplicates OneKey's connect loop

packages/wallet-extensions/src/bitget/index.ts:15 · code-style-complexity

connectBitget copies OneKey's connect loop (onekey/index.ts lines 16-33) almost line for line. Both have the failedChains list, the try/catch that rethrows when isUserRejection matches, the check that throws the first error if every chain failed, and the console.warn loop. The only real difference is Bitget's pause between chains. This PR already moved isUserRejection into a shared helper but left the code around it duplicated. Any future fix to error collection or partial-failure handling must be made in both files and kept in sync by hand.

Fix: Move the loop into a shared helper next to isUserRejection, for example connectChainsInOrder({ chains, connectChain, beforeChain?, label }). It runs connectChain for each chain, rethrows user rejections, throws if every chain failed, and warns about skipped chains. OneKey passes only connectChain. Bitget also passes beforeChain: (chain, previous) => previous && !(isEvmChain(previous) && isEvmChain(chain)) ? sleep(CHAIN_PAUSE_MS) : undefined. That leaves only the EVM-last sort and the pause rule in connectBitget.

Low

signPsbt type drops the toSignInputs option

packages/wallet-extensions/src/bitget/types.ts:8 · logic

The replacement public Window.bitkeep type removes the previously supported toSignInputs option from signPsbt. Existing TypeScript callers that pass an options literal with selected signing inputs now fail excess-property checking. Bitcoin selective signing was not retired, so this is an unintended break.

Fix: Restore the optional toSignInputs field and its previous address/public-key input union in BitgetBtcProvider.signPsbt.

Aptos branch in getWalletMethods can never run

packages/wallet-extensions/src/bitget/helpers.ts:274 · code-style-complexity

getWalletMethods has a Chain.Aptos case that calls getAptosWalletMethods (lines 166-176), and BitgetProviders declares aptos: unknown to support it. Aptos is not in supportedChains in index.ts, and the comment there says it is blocked on the toolbox. filterSupportedChains removes Aptos before getWalletMethods is called, so this code is unreachable. This refactor rewrote it as a new standalone function. That makes unused code look like a supported path and adds an unknown provider type with a cast.

Fix: Remove the Chain.Aptos case, getAptosWalletMethods and the aptos entry in BitgetProviders. Aptos then falls through to the existing wallet_chain_not_supported default case. Keep the TODO comment in index.ts so the code can be restored once the toolbox supports Aptos.

No-accounts error is written out inline instead of using a helper

packages/wallet-extensions/src/bitget/helpers.ts:84 · code-style-complexity

The file adds small helpers for most errors (notFoundError, mainnetOnlyError, connectionRejectedError, transactionFailedError). The most common one, new SwapKitError("wallet_bitkeep_no_accounts", { chain }), is still written out in full at lines 49, 84, 145, 185 and 235. Mixing the two styles makes the per-chain functions harder to scan. It also makes it easy for the wallet field and other details to drift apart between chains.

Fix: Add a noAccountsError(chain) helper next to the other error helpers. Use it at all five places, including the TON noAccountError callback.

Review coverage and raw evidence

Reviewed head 443b455bf56d5846f2faab888baa2d3f493344c0 against base 3c61e954240c075cbd6c1d24ca63e0829d3e08d9. Run 597baf6f-2efa-4a27-b7c7-9c469afae337.

logic — failed (2 findings)

Reviewed all changed paths and relevant callers, checking chain support, signing inputs, error handling and test preservation. Cosmos retirement is intentional; no tests were removed. Found two issues. Graph context had unresolved dependencies; Argus coverage was partial. External wallet/toolbox behavior was not exercised.

adversarial — passed (0 findings)

Inspected all changed sources, relevant callers/helpers, dependency hunks and test preservation. Supplied probes meaningfully exercise connection failure isolation, cancellation, Tron/Sui signing boundaries, and three independently rejected mutations after a passing baseline. No demonstrated defect. No tests were deleted or weakened; Cosmos removal is explicitly scoped. Limitations: mocked external dependencies; Solana/XRP integration, Bitcoin signing, successful TON transactions, broadcasting and pause timing remain unverified. Graph context has unresolved relationships; Argus coverage is partial (14 checks, three unsupported files, 171 omitted references). Experiments were assessed from supplied observations, not rerun.

Experiment probe-1 (behavior): passed, exit 0, 5925 ms.

Experiment probe-2 (behavior): passed, exit 0, 3599 ms.

Experiment probe-3 (test-quality): passed, exit 0, 7262 ms.

code-style-complexity — failed (3 findings)

I reviewed every changed source file in this PR for structural complexity and repository style: bitget/index.ts, bitget/helpers.ts, bitget/types.ts, helpers/userRejection.ts, onekey/index.ts, types.ts, the new bitget test, package.json, and the changeset. I compared them against the base revision using the local patch. For bun.lock I only read the changed hunks in the patch, not the full lockfile.

Overall, the move from a ts-pattern chain to one connect function per chain, the shared getProvider lookup, the new BitgetProviders types and the move of isUserRejection into a shared helper are all clear simplifications. I found three structural issues:

• Duplicated connect loop (medium): the new connect loop is a copy of the one in OneKey's onekey/index.ts.
• Unreachable Aptos branch (low): getWalletMethods and the types still carry Aptos code that can never run.
• Inconsistent error construction (low): errors are built through small helper functions in some places and written out in full in others.

The Argus/Jev pass reported partial coverage (coverage_incomplete): 171 related files were left out, and bun.lock, package.json and the changeset were not evaluated. It flagged helpers.ts, index.ts and types.ts as possible behaviour regressions. I checked those against the source. Cosmos was removed on purpose (the PR description says so). The bitkeep window type now uses Partial<BitgetProviders>, and nothing outside the bitget folder uses the removed typings. I confirmed none of the flags as a defect within my focus.

Bitget's own getWeb3WalletMethods copy already existed before this PR, so I did not report it.

Original specialist findings

F1 · medium · logic · packages/wallet-extensions/src/bitget/helpers.ts:251

Issue: When a request omits Flags, this loop accepts any wallet-added flags. For example, adding tfPartialPayment (0x00020000) to an issued-currency Payment passes the comparison but permits delivery of less than the requested Amount. The new transaction-integrity check therefore accepts a payment with altered semantics.

Solution: Remove Flags from the unrestricted autofill list. Treat omitted flags as zero and allow only explicitly identified nonsemantic defaults, such as the canonical-signature bit. Add a test rejecting a wallet-added partial-payment flag.

F2 · medium · code-style-complexity · packages/wallet-extensions/src/bitget/index.ts:15

Issue: connectBitget copies OneKey's connect loop (onekey/index.ts lines 16-33) almost line for line: the failedChains list, the try/catch that rethrows when isUserRejection matches, the "throw the first error if every chain failed" check, and the console.warn loop. The only real difference is Bitget's pause between chains. This PR already moved isUserRejection into a shared helper so both wallets could use it, but left the code around it duplicated. As a result, any future fix to how errors are collected or how partial failures are handled has to be made in both files and kept in sync by hand.

Solution: Move the loop into a shared helper next to isUserRejection, for example connectChainsInOrder({ chains, connectChain, beforeChain?, label }). It would run connectChain for each chain, rethrow user rejections, throw if every chain failed, and warn about the ones that were skipped. OneKey would call it with only connectChain. Bitget would also pass beforeChain: (chain, previous) => previous && !(isEvmChain(previous) && isEvmChain(chain)) ? sleep(CHAIN_PAUSE_MS) : undefined, which leaves only the EVM-last sort and the pause rule in connectBitget.

F3 · low · logic · packages/wallet-extensions/src/bitget/types.ts:8

Issue: The replacement public Window.bitkeep type drops the previously supported toSignInputs option from signPsbt. Existing TypeScript callers passing an options literal with selected signing inputs now fail excess-property checking, although Bitcoin selective signing was not retired.

Solution: Restore the optional toSignInputs field and its previous address/public-key input union in BitgetBtcProvider.signPsbt.

F4 · low · code-style-complexity · packages/wallet-extensions/src/bitget/helpers.ts:274

Issue: getWalletMethods has a Chain.Aptos case that calls getAptosWalletMethods (lines 166-176), and BitgetProviders declares aptos: unknown to support it. However, Aptos is not in supportedChains in index.ts, and the comment there says it is blocked on the toolbox. filterSupportedChains therefore removes Aptos before getWalletMethods is ever called, so this code cannot run. It was rewritten as a new standalone function in this refactor, which makes unused code look like a supported path and adds an unknown provider type with a cast.

Solution: Remove the Chain.Aptos case, getAptosWalletMethods, and the aptos entry in BitgetProviders. Aptos would then fall through to the existing wallet_chain_not_supported default case. Keep the existing TODO comment in index.ts so the code can be restored when the toolbox supports it.

F5 · low · code-style-complexity · packages/wallet-extensions/src/bitget/helpers.ts:84

Issue: The file introduces small helper functions for most of its errors (notFoundError, mainnetOnlyError, connectionRejectedError, transactionFailedError). The most common one, new SwapKitError("wallet_bitkeep_no_accounts", { chain }), is still written out in full at lines 49, 84, 145, 185 and 235. Mixing the two styles makes the per-chain functions harder to scan and invites the wallet field and other details to drift apart between chains.

Solution: Add a noAccountsError(chain) helper alongside the other error helpers and use it at all five places, including the TON noAccountError callback.