[hippocampusSK]

Users can now connect Sui with OKX Wallet and swap from it.
Builds on the Solana, Starknet and Cardano support, so it should be merged after that one.
Still needs to be tried by hand with the OKX extension before it's ready for review.

https://linear.app/swapkit/issue/API-3994/okx-sui

[hippocampusSK]

/dominion review

[ai-reviewer-towan]

PR review · published

Outcome: failed. 6 findings: 3 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: 9fbafd7b-116d-44ee-8b6a-86c36c50d7c2
• Head: 1d122807a31f
• Base: 5a19bdd97666
• Check: review-runner

[ai-reviewer-towan]

TL;DR

This PR adds Sui connection and signing to OKX Wallet. The biggest problem: if OKX exposes no Sui signing feature, a mixed-chain connect now fails completely instead of skipping Sui, and the new test's global module mock can break other suites. Adversarial sandbox probes passed, but nobody tested against the real OKX extension, the real SDK/toolbox integration or transaction broadcasting.

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

Medium

Missing Sui signing feature aborts the whole mixed-chain connection

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

OKX may advertise Sui mainnet but expose neither signing feature. The new test models exactly this case. The error thrown at this line then aborts the entire mixed-chain connection, because connectOkx only skips wallet_okx_not_found and wallet_missing_params. A request for [Cardano, Sui, Solana] now rejects and never attempts Solana, even when the other chains are available. Before this change, Sui was filtered out.

Fix: Classify missing Sui signing capability as a skippable unavailable-provider error. Add a mixed-chain regression test. Keep user rejection and mainnet-validation failures fatal.

Unrestored global module mock breaks other Wallet Standard test suites

packages/wallet-extensions/src/okx/__tests__/okx.test.ts:26 · logic

This process-wide Bun module mock is never restored. It replaces getWallets() with an object that has no register(). phantom-bitcoin.test.ts and ledger-wallet-provider.test.ts register real Wallet Standard fixtures. If they run after this suite in the same process, they fail with a TypeError and their existing coverage never runs.

Fix: Register and unregister the Sui fixture through the real Wallet Standard registry, as the existing tests do. Alternatively, restore the original module exports after this suite.

Sui connector is inline in helpers.ts instead of in its own module

packages/wallet-extensions/src/okx/helpers.ts:210 · code-style-complexity

The Sui branch puts about 50 lines of wallet-standard setup inline in the getWalletMethods match. That setup finds the wallet, picks a feature, connects, filters accounts and wraps the signer. Its types and the SUI_MAINNET constant (lines 17-27) also sit at the top of helpers.ts, which otherwise holds window-injected providers. TON, the other non-trivial OKX connector, lives in ./ton.ts and is wired as .with(Chain.Ton, () => getOkxTonWallet()). Keeping Sui inline makes the chain dispatch harder to read and mixes wallet-standard details into the shared helper file.

Fix: Move the branch body, the SuiAccount, SuiSignInput and OkxSuiFeatures types, and SUI_MAINNET into packages/wallet-extensions/src/okx/sui.ts as export async function getOkxSuiWallet(). Then reduce the match arm to .with(Chain.Sui, () => getOkxSuiWallet()), matching how TON is wired.

Low

Untested legacy sui:signTransactionBlock branch adds confusing indirection

packages/wallet-extensions/src/okx/helpers.ts:224 · code-style-complexity

signWithWallet comes from a ternary whose else branch uses legacyFeature && (async ...). That makes its type function | undefined and requires a separate !signWithWallet check. This is the only use of the legacy sui:signTransactionBlock path. It adds a second type and a result-renaming adapter. No test covers it, since all three new tests use sui:signTransaction. The PR also does not say that OKX exposes only the legacy feature. The result is an untested branch that is hard to follow.

Fix: If OKX exposes sui:signTransaction, remove the legacy branch and the sui:signTransactionBlock type. Check that feature directly, e.g. const signFeature = features["sui:signTransaction"]; if (!signFeature) throw ..., and call signFeature.signTransaction({ account, chain: SUI_MAINNET, transaction }). If the legacy path is needed, write it as a plain if/else that assigns the function, and add a test that exposes only sui:signTransactionBlock.

signTransaction needlessly destructures and rebuilds the signer result

packages/wallet-extensions/src/okx/helpers.ts:250 · code-style-complexity

signTransaction takes { bytes, signature } from signWithWallet and immediately rebuilds the same object. signWithWallet already returns exactly { bytes, signature } in both branches. The extra step does nothing and makes the flow longer to read.

Fix: Return the call directly: signTransaction = (txBytes: Uint8Array) => { const serialized = base64.encode(txBytes); return signWithWallet(account, { serialize: () => serialized, toJSON: () => Promise.resolve(serialized) }); }.

Rejection tests mutate shared suiWallet and may never restore it

packages/wallet-extensions/src/okx/__tests__/okx.test.ts:148 · code-style-complexity

The two rejection tests change the shared module-level suiWallet. They restore it only after the await expect(...) line. If an assertion fails, the restore never runs and the mocked wallet stays broken, so one failure cascades into later tests. The third test also rebuilds the restored features by hand from destructured pieces, which makes the setup harder to follow.

Fix: Build a fresh wallet fixture per test with a small factory such as makeSuiWallet(overrides), and have the mock.module getWallets mock return the current fixture. Alternatively, put the restore in afterEach or try/finally so the shared state always resets.

Review coverage and raw evidence

Reviewed head 1d122807a31fd76b0f0b65910d140c89046c8512 against base 5a19bdd97666870b01d022b5f9dcd5783a2d9c75. Run 9fbafd7b-116d-44ee-8b6a-86c36c50d7c2.

logic — failed (2 findings)

Inspected all changed files, connection/error callers, and related Wallet Standard tests. No existing assertions were removed, but found mixed-chain fallback and test-isolation regressions. Real-extension and legacy-signing compatibility remain unverified; no code was executed.

adversarial — passed (0 findings)

Inspected all changed files and surrounding loader/connector workflow; no tests or assertions were removed. Supplied probes meaningfully exercised signing boundaries, rejection handling and later-provider continuation. Existing tests rejected both realistic mutations. No product defect was demonstrated. Actual OKX extension compatibility, real SDK/toolbox integration, transaction construction/broadcasting and retained non-Sui suites remain unverified.

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

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

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

code-style-complexity — failed (4 findings)

I reviewed the whole patch. That covers the changeset, the OKX Sui connector in helpers.ts, the Sui entries added to index.ts and the new Sui tests. I also compared it with the other OKX connectors in helpers.ts, the separate TON module (ton.ts) and the existing Sui ledger integration. No tests or fallbacks were deleted or weakened. The Sui connector works, but it is the only large standard-wallet connector written inline, and its fallback signer and wrapper add indirection. The @​swapkit/toolboxes Sui signer type is not in the checkout, so I could not check whether the signer needs both getAddress and toSuiAddress.

Original specialist findings

F1 · medium · logic · packages/wallet-extensions/src/okx/helpers.ts:238

Issue: When OKX advertises Sui mainnet but exposes neither signing feature—the condition explicitly modeled by the new test—this error aborts the entire mixed-chain connection. connectOkx only skips wallet_okx_not_found and wallet_missing_params, so requesting [Cardano, Sui, Solana] now rejects and never attempts Solana, even when the other chains are available. Previously Sui was filtered out.

Solution: Classify missing Sui signing capability as a skippable unavailable-provider error, and add a mixed-chain regression test. Keep user rejection and mainnet-validation failures fatal.

F2 · medium · logic · packages/wallet-extensions/src/okx/__tests__/okx.test.ts:26

Issue: This process-wide Bun module mock is never restored and replaces getWallets() with an object lacking register(). Existing phantom-bitcoin.test.ts and ledger-wallet-provider.test.ts register real Wallet Standard fixtures, so running them after this suite in the same process fails with a TypeError instead of exercising their existing coverage.

Solution: Register and unregister the Sui fixture through the real Wallet Standard registry, as the existing tests do, or restore the original module exports after this suite.

F3 · medium · code-style-complexity · packages/wallet-extensions/src/okx/helpers.ts:210

Issue: The Sui branch puts about 50 lines of wallet-standard setup inline in the getWalletMethods match: finding the wallet, choosing a feature, connecting, filtering accounts and wrapping the signer. Its types and the SUI_MAINNET constant (lines 17-27) also sit at the top of helpers.ts, which otherwise holds window-injected providers. TON, the other non-trivial OKX connector, has its own module (.with(Chain.Ton, () => getOkxTonWallet()) with ./ton.ts). Keeping Sui inline makes the chain dispatch harder to read and mixes wallet-standard details into the shared helper file.

Solution: Move the branch body, the SuiAccount, SuiSignInput and OkxSuiFeatures types, and SUI_MAINNET into packages/wallet-extensions/src/okx/sui.ts as export async function getOkxSuiWallet(). Then reduce the match arm to .with(Chain.Sui, () => getOkxSuiWallet()), the same way TON is wired.

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

Issue: signWithWallet is built from a ternary whose else branch uses legacyFeature && (async ...). That gives it the type function | undefined and requires a separate !signWithWallet check. This is the only use of the legacy sui:signTransactionBlock path. It adds a second type and a result-renaming adapter, but no test covers it (all three new tests use sui:signTransaction) and the PR does not say OKX only exposes the legacy feature. The result is an untested branch that is hard to follow.

Solution: If OKX exposes sui:signTransaction, remove the legacy branch and the sui:signTransactionBlock type. Then check that feature directly, for example const signFeature = features["sui:signTransaction"]; if (!signFeature) throw ..., and call signFeature.signTransaction({ account, chain: SUI_MAINNET, transaction }). If the legacy path is needed, write it as a plain if/else that assigns the function, and add a test that only exposes sui:signTransactionBlock.

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

Issue: signTransaction takes { bytes, signature } from signWithWallet and immediately rebuilds the same object. signWithWallet already returns exactly { bytes, signature } in both branches, so the destructure-and-rebuild step has no effect and makes the flow longer to read.

Solution: Return the call directly: signTransaction = (txBytes: Uint8Array) => { const serialized = base64.encode(txBytes); return signWithWallet(account, { serialize: () => serialized, toJSON: () => Promise.resolve(serialized) }); }.

F6 · low · code-style-complexity · packages/wallet-extensions/src/okx/__tests__/okx.test.ts:148

Issue: The two rejection tests change the shared module-level suiWallet and restore it only after the await expect(...) line. If an assertion fails, the restore never runs and the mocked wallet stays broken for later tests, so one failure causes others. The third test also builds the restored features by hand from destructured pieces, which makes the setup harder to follow.

Solution: Build a fresh wallet fixture per test. Use a small factory such as makeSuiWallet(overrides) and let the mock.module getWallets mock return the current fixture. Alternatively, put the restore in afterEach or try/finally so the shared state is always reset.

[hippocampusSK]

/dominion review

[ai-reviewer-towan]

PR review · published

Outcome: inconclusive. 2 findings: 1 medium, 1 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	⚠️ error
code-style-complexity	claude-code claude-opus-5-5	✅ passed

Run details

• Run: 53cc953a-bf04-47d2-be29-33110465995d
• Head: e7926826f622
• Base: 15a8aad354f5
• Check: review-runner

[ai-reviewer-towan]

TL;DR

This PR adds Sui connect and swap support to OKX Wallet. The main problem: an OKX wallet without Sui signing now aborts multi-chain connection instead of skipping Sui. The adversarial lane errored, so its evidence is missing. No real OKX-extension signing or @swapkit/toolboxes/sui behavior was verified.

This review is inconclusive. Some evidence is missing or failed, so it does not establish that the change is safe. 2 findings: 1 medium, 1 low.

• adversarial did not complete: assess adversarial experiments: adversarial experiments did not establish meaningful behavior and test-quality evidence

Medium

Missing Sui signing aborts multi-chain OKX connection

packages/wallet-extensions/src/okx/sui.ts:29 · logic

A registered OKX wallet that lacks sui:signTransaction now aborts multi-chain connection. Before this change, unsupported Sui was filtered out and the other chains still connected. For example, connectOkx([Chain.Cardano, Chain.Sui, Chain.Solana]) throws at line 29 after connecting Cardano and never tries Solana. The enclosing loop only skips wallet_okx_not_found and wallet_missing_params.

Fix: Treat the missing signing capability as a skippable unavailable-chain error, and keep user rejection fatal. Add a multi-chain regression test that checks available chains still connect. Requesting only an unavailable Sui should still fail.

Low

Redundant features check in Sui connect guard

packages/wallet-extensions/src/okx/sui.ts:22 · code-style-complexity

The check if (!(features && connect)) tests features for no reason. connect is read as features?.["standard:connect"], so it is always undefined when features is undefined. Testing both suggests two conditions when there is only one. It also differs from the Phantom Wallet Standard lookup, which tests only the features it reads.

Fix: Change the check to if (!connect). Once connect is defined, TypeScript already knows features is defined, so the later features["sui:signTransaction"] access needs no other change.

Review coverage and raw evidence

Reviewed head e7926826f622e068f41bdbe5bd213b923211f42c against base 15a8aad354f57b568673aebf35989edc9123b53c. Run 53cc953a-bf04-47d2-be29-33110465995d.

logic — failed (1 findings)

Reviewed all five changed files, preservation of existing behavior/tests, connection error handling, exports and related adapters. No tests were removed. Graph context was available but limited; Argus coverage was partial, omitting 178 reference files. No code was executed; real-extension signing and external toolbox behavior remain unverified.

adversarial — error (0 findings)

assess adversarial experiments: adversarial experiments did not establish meaningful behavior and test-quality evidence

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

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

Experiment probe-3 (test-quality): violated, exit 1, 1563 ms.

code-style-complexity — passed (1 findings)

I reviewed every changed file: the changeset, okx/sui.ts, the new Sui dispatch in okx/helpers.ts, the Sui entries added in okx/index.ts and the new Sui tests. The structure is easy to follow. Sui lives in its own sui.ts module, the same way TON is split out into ton.ts. It is dispatched through the existing match chain. Its Wallet Standard lookup works the same way as Phantom's Bitcoin path, which also uses Wallet Standard. The code also keeps three separate failure checks: no OKX wallet found, no Sui signing feature, and no mainnet account. The tests cover all three. I found one small simplification and no correctness or style regressions.

Coverage limits:

• The Argus pass is partial (coverage_incomplete) and returned no candidates. It did not evaluate the changeset file and only looked at a limited set of related files.
• The graph context only suggested some related files, so I did my own source search.
• I could not find the @swapkit/toolboxes/sui source in this checkout. So I could not check the signer contract (whether it needs both getAddress and toSuiAddress) or the SuiWallet type.
• The PR body says the change has not yet been tried by hand with the OKX extension.

Original specialist findings

F1 · medium · logic · packages/wallet-extensions/src/okx/sui.ts:29

Issue: A registered OKX wallet lacking sui:signTransaction now aborts multi-chain connection instead of skipping unavailable Sui support. For example, connectOkx([Chain.Cardano, Chain.Sui, Chain.Solana]) throws here after connecting Cardano and never attempts Solana: the enclosing loop only skips wallet_okx_not_found and wallet_missing_params. Before this change, unsupported Sui was filtered out and the other chains connected.

Solution: Classify the missing signing capability as a skippable unavailable-chain error, while retaining fatal handling for user rejection. Add a multi-chain regression test verifying that available chains still connect; requesting only unavailable Sui should continue to fail.

F2 · low · code-style-complexity · packages/wallet-extensions/src/okx/sui.ts:22

Issue: The check if (!(features && connect)) tests features for nothing. connect is read as features?.["standard:connect"], so it is always undefined when features is undefined. Testing both makes it look like two conditions are being checked when there is only one. It also differs from the Phantom Wallet Standard lookup, which tests only the features it reads.

Solution: Change the check to if (!connect). Because connect is defined, TypeScript already knows features is defined at that point, so the later features["sui:signTransaction"] access needs no other changes.

[hippocampusSK]

/dominion review

[ai-reviewer-towan]

PR review · published

Outcome: passed. 2 findings: 2 low.

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

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

Run details

• Run: 928da95c-16f7-46fe-ad01-2da8c61e3685
• Head: 82d1a0fe038f
• Base: 15a8aad354f5
• Check: review-runner

[ai-reviewer-towan]

TL;DR

This PR adds Sui connection and swap support to OKX Wallet. The biggest issue is that sui.ts reports a signed-bytes mismatch as wallet_okx_chain_not_supported, so an integrity failure looks like a network mismatch. Adversarial sandbox tests with dependency doubles passed. Real OKX SDK compatibility, extension approval, broadcasting and live swaps are still unverified.

Review passed. Every selected gate completed and passed. 2 findings: 2 low.

Low

Signed-bytes mismatch throws the wrong error key

packages/wallet-extensions/src/okx/sui.ts:45 · code-style-complexity

In packages/wallet-extensions/src/okx/sui.ts line 45, the code throws wallet_okx_chain_not_supported when the wallet returns different signed bytes. The repo uses that key for 'Mainnet only' (sui.ts:36, ton.ts:13, the Cardano check in helpers.ts:177). Tampered or modified transaction bytes have nothing to do with chain support. Callers and anyone reading logs can't tell an integrity failure from a network mismatch. The test at okx.test.ts:168 then locks in the misleading key.

Fix: Throw a signing-specific error for the byte mismatch. Other wallets use keys like wallet_onekey_sign_transaction_error or wallet_ledger_signing_error. Add an OKX signing error key or use an existing generic signing key, then update the test's expectation to match.

Two separate guards repeat the missing-feature check

packages/wallet-extensions/src/okx/sui.ts:26 · code-style-complexity

In sui.ts, two guard blocks in a row (lines 20-24 and 26-30) both throw wallet_okx_not_found for a missing wallet-standard feature, and they go through an extra intermediate features variable. That adds a second branch and a temporary variable for what is really one 'OKX Sui wallet not usable' check. Other OKX branches in helpers.ts use one existence guard per provider.

Fix: Pull out connect and signFeature together from wallet?.features and use one guard: if (!(connect && signFeature)) throw new SwapKitError("wallet_okx_not_found", { chain: Chain.Sui });. This keeps the skippable error key that isSkippableChain relies on.

Review coverage and raw evidence

Reviewed head 82d1a0fe038fd3438ad28fdcc5c1c0c63f493c27 against base 15a8aad354f57b568673aebf35989edc9123b53c. Run 928da95c-16f7-46fe-ad01-2da8c61e3685.

logic — passed (0 findings)

Inspected all five changed files, related wallet loaders, signing adapters and connection error handling. Changes are additive; no existing tests or guards were removed or weakened. No concrete defect was confirmed. Graph context was available but limited; Argus coverage was partial. External toolbox internals and live OKX signing remain unverified; no tests were executed.

adversarial — passed (0 findings)

Inspected every changed file, connector workflow and related integrations. Supplied observations demonstrate meaningful byte-boundary, rejection and later-chain recovery checks; four baseline tests pass and three independent regressions are caught. No existing tests or assertions were removed or weakened. No demonstrated defect. Dependency doubles leave actual SDK compatibility, extension discovery/approval, broadcasting and swaps unverified; unrelated suites were not run. Structural graph coverage is incomplete, and Argus coverage is partial.

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

Experiment probe-2 (test-quality): passed, exit 0, 2916 ms.

code-style-complexity — passed (2 findings)

I reviewed every changed file in the patch for structural complexity and repository style: the changeset, okx.test.ts, the okx helpers.ts and index.ts wiring, and the new sui.ts. I compared the patterns with ton.ts, the Cardano and other branches in okx helpers.ts, the phantom and Ledger wallet-standard uses, and how SwapKitError keys are used across the repo. The wiring into helpers.ts and index.ts follows the existing per-chain pattern (the same shape as getOkxTonWallet), and the tests exercise the new flows. I found two small style issues in sui.ts. Coverage note: the Argus pass is only partial (coverage_incomplete, no candidates; the .md changeset was not evaluated). The review context file was not read, so the graph-context status and suggested callers/tests in .dominion-review.context.json were not checked. Callers were traced by ordinary source search. These limits do not change the findings.

Original specialist findings

F1 · low · code-style-complexity · packages/wallet-extensions/src/okx/sui.ts:45

Issue: When the wallet returns different signed bytes, the code throws wallet_okx_chain_not_supported, the key the repo uses for 'Mainnet only' (sui.ts:36, ton.ts:13, the Cardano check in helpers.ts:177). Tampered or modified transaction bytes have nothing to do with chain support. Callers and anyone reading logs can't tell an integrity failure apart from a network mismatch. The test at okx.test.ts:168 then locks in this misleading key.

Solution: Throw a signing-specific error for the byte mismatch instead. Other wallets use keys like wallet_onekey_sign_transaction_error or wallet_ledger_signing_error. Either add an OKX signing error key or use an existing generic signing key, and update the test's expectation to match.

F2 · low · code-style-complexity · packages/wallet-extensions/src/okx/sui.ts:26

Issue: Two guard blocks in a row (lines 20-24 and 26-30) both throw wallet_okx_not_found for a missing wallet-standard feature, through an extra intermediate features variable. That adds a second branch and a temporary variable for what is really one 'OKX Sui wallet not usable' check. Other OKX branches in helpers.ts use one existence guard per provider.

Solution: Pull out connect and signFeature together from wallet?.features and use a single guard: if (!(connect && signFeature)) throw new SwapKitError("wallet_okx_not_found", { chain: Chain.Sui });. This keeps the same skippable error key that isSkippableChain relies on.