feat: fund vault#13
Conversation
WalkthroughAdds a new TypeScript CLI script, ChangesSui RFQ Vault Funding Script
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: The temp keystore import redacts secrets in error output, but this should be double-checked: any raw stdout/stderr paths that bypass the Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
🧹 Nitpick comments (1)
scripts/sui/fundRfqVault.ts (1)
20-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded per-run constants require code edits for every funding operation.
FUND_AMOUNT,FUND_SOURCE_COIN_ID,RFQ_VAULT_PACKAGE,RFQ_VAULT_OBJECTare hardcoded values that must be hand-edited for every run (per the comment above them). This works for a one-off manual script but is easy to fumble (stale coin ID, wrong vault object) with no compile-time or runtime cross-check against the actual on-chain state.Also note Lines 31-33's
||fallbacks are effectively dead code today, since the importedSUI_RFQ_VAULT_*constants are already non-empty string literals.Consider sourcing the per-run values (amount, source coin) from CLI args or env vars instead of code edits, to reduce the chance of an accidental mainnet mistake.
🤖 Prompt for 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. In `@scripts/sui/fundRfqVault.ts` around lines 20 - 33, The manual funding constants in fundRfqVault are hardcoded, forcing code edits for each run and increasing the risk of stale coin/vault values; update the script so per-run inputs like FUND_AMOUNT and FUND_SOURCE_COIN_ID (and, if possible, RFQ_VAULT_PACKAGE and RFQ_VAULT_OBJECT) come from CLI args or env/config instead of literals. Also remove or replace the dead `||` fallbacks for FUND_GAS_BUDGET, FUND_NETWORK_ALIAS, and FUND_RPC_URL, since the SUI_RFQ_VAULT_* values are already fixed strings, and keep the existing fundRfqVault flow aligned with those symbols.
🤖 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 `@scripts/sui/fundRfqVault.ts`:
- Around line 113-142: The runSui helper currently calls spawnSync without any
timeout, so any hung sui invocation can block the script indefinitely. Update
runSui to pass a timeout option (and handle the timeout case explicitly in the
existing result.error/result.status flow), and keep the behavior consistent for
all callers such as the env setup, keytool import, split-coin, and vault call
steps.
- Around line 46-62: The deployer secret handling in resolveDeployerConfig() and
the subsequent sui keytool import path should not pass the raw secret as a
positional argv argument. Update the import flow to use a non-argv mechanism
such as stdin or a temp file/input channel, and ensure the code that consumes
deployer.secret no longer embeds it in process arguments. If that cannot be
changed, restrict the script to an isolated single-tenant environment and make
the unsafe path explicit.
- Around line 219-226: The mnemonic import flow in keytool import currently
depends on --alias, which can be lost on older Sui releases and break the later
switch --address DEPLOYER_ALIAS step. Update the fundRfqVault import path to
either pin a known-safe Sui CLI version or explicitly set/verify the alias after
import in the keytool import/deployer handling logic so DEPLOYER_ALIAS is
guaranteed to exist.
---
Nitpick comments:
In `@scripts/sui/fundRfqVault.ts`:
- Around line 20-33: The manual funding constants in fundRfqVault are hardcoded,
forcing code edits for each run and increasing the risk of stale coin/vault
values; update the script so per-run inputs like FUND_AMOUNT and
FUND_SOURCE_COIN_ID (and, if possible, RFQ_VAULT_PACKAGE and RFQ_VAULT_OBJECT)
come from CLI args or env/config instead of literals. Also remove or replace the
dead `||` fallbacks for FUND_GAS_BUDGET, FUND_NETWORK_ALIAS, and FUND_RPC_URL,
since the SUI_RFQ_VAULT_* values are already fixed strings, and keep the
existing fundRfqVault flow aligned with those symbols.
🪄 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: ee7d0089-247e-4da5-8144-0b546deef4f7
📒 Files selected for processing (2)
package.jsonscripts/sui/fundRfqVault.ts
| function resolveDeployerConfig(): { secret: string; scheme: string } { | ||
| const secret = process.env.SUI_DEPLOYER?.trim(); | ||
| if (!secret) { | ||
| throw new Error('SUI_DEPLOYER is required'); | ||
| } | ||
|
|
||
| const scheme = ( | ||
| process.env.SUI_DEPLOYER_SCHEME?.trim() || 'ed25519' | ||
| ).toLowerCase(); | ||
| if (!DEPLOYER_SCHEMES.has(scheme)) { | ||
| throw new Error( | ||
| `SUI_DEPLOYER_SCHEME must be one of ${Array.from(DEPLOYER_SCHEMES).join(', ')}`, | ||
| ); | ||
| } | ||
|
|
||
| return { secret, scheme }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files scripts/sui/fundRfqVault.ts scripts/sui | sed -n '1,120p'
printf '\n--- outline ---\n'
ast-grep outline scripts/sui/fundRfqVault.ts --view expanded
printf '\n--- line count ---\n'
wc -l scripts/sui/fundRfqVault.ts
printf '\n--- relevant slices ---\n'
sed -n '1,220p' scripts/sui/fundRfqVault.ts | cat -n
printf '\n--- deployment config refs ---\n'
rg -n "GAS_BUDGET|NETWORK_ALIAS|RPC_URL|DEPLOYER_SCHEMES|SUI_DEPLOYER|SUI_DEPLOYER_SCHEME|keytool import|switch --address|spawnSync|timeout" scripts/sui -n -A 3 -B 3Repository: SocketDotTech/openrouter
Length of output: 21147
🌐 Web query:
Sui CLI keytool import mnemonic alias ignored documented bug
💡 Result:
The issue where the --alias flag was ignored during the sui keytool import command is a documented bug that has been addressed in previous updates to the Sui CLI [1][2]. Previously, users encountered a bug where providing the --alias flag when importing a key via a mnemonic phrase failed to correctly assign or record the specified alias [1][2]. This behavior was identified and resolved through pull requests in the official Sui repository, ensuring that the alias provided during the import process is now correctly stored [1][2]. If you are encountering this issue, ensure your Sui CLI version is up to date. You can specify an alias during import using the following syntax: sui keytool import '<MNEMONIC_PHRASE>' <KEY_SCHEME> --alias <YOUR_ALIAS> If an alias is not provided, the CLI will automatically generate one for the imported key [3]. Additionally, if you need to change an existing alias after an import, you can use the command: sui keytool update-alias <OLD_ALIAS> <NEW_ALIAS> [4]
Citations:
- 1: [CLI] fix command
sui keytool importusing--aliasis useless when inputting mnemonic phrase MystenLabs/sui#17935 - 2: [CLI] fix
sui keytool import--alias command and default alias does not work MystenLabs/sui#20111 - 3: https://github.com/MystenLabs/sui/blob/4af8007c/crates/sui/src/keytool.rs
- 4: https://github.com/MystenLabs/sui/blob/main/docs/content/getting-started/onboarding/get-address.mdx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- import/switch section ---'
sed -n '217,255p' scripts/sui/fundRfqVault.ts | cat -n
printf '\n%s\n' '--- version pin/search ---'
rg -n "sui cli|Sui CLI|keytool import|update-alias|version" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'
printf '\n%s\n' '--- package metadata ---'
git ls-files package.json pnpm-lock.yaml yarn.lock package-lock.json | sed -n '1,120p'Repository: SocketDotTech/openrouter
Length of output: 50381
Do not pass the deployer secret on argv. deployer.secret is sent to sui keytool import as a positional argument, so the raw key/mnemonic is visible in ps and /proc/<pid>/cmdline for the lifetime of the process. This should use a non-argv import path or be restricted to a fully isolated single-tenant host.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for 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.
In `@scripts/sui/fundRfqVault.ts` around lines 46 - 62, The deployer secret
handling in resolveDeployerConfig() and the subsequent sui keytool import path
should not pass the raw secret as a positional argv argument. Update the import
flow to use a non-argv mechanism such as stdin or a temp file/input channel, and
ensure the code that consumes deployer.secret no longer embeds it in process
arguments. If that cannot be changed, restrict the script to an isolated
single-tenant environment and make the unsafe path explicit.
| function runSui( | ||
| args: string[], | ||
| params: { | ||
| cwd?: string; | ||
| env?: NodeJS.ProcessEnv; | ||
| label: string; | ||
| redactions?: readonly string[]; | ||
| }, | ||
| ): string { | ||
| console.log(`[sui] ${params.label}`); | ||
| const result = spawnSync('sui', args, { | ||
| cwd: params.cwd ?? REPO_ROOT, | ||
| env: params.env, | ||
| encoding: 'utf8', | ||
| }); | ||
|
|
||
| if (result.error) { | ||
| throw result.error; | ||
| } | ||
|
|
||
| if (result.status !== 0) { | ||
| const output = redact( | ||
| [result.stdout, result.stderr].filter(Boolean).join('\n'), | ||
| params.redactions ?? [], | ||
| ); | ||
| throw new Error(`${params.label} failed\n${output}`); | ||
| } | ||
|
|
||
| return result.stdout.trim(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No timeout on spawnSync calls.
Every sui invocation (env setup, keytool import, split-coin, vault call) runs via spawnSync with no timeout option. If any call hangs (RPC stall, network partition, or an unexpected interactive prompt), the script blocks indefinitely with no way to recover other than killing the process.
Proposed fix: add a timeout
const result = spawnSync('sui', args, {
cwd: params.cwd ?? REPO_ROOT,
env: params.env,
encoding: 'utf8',
+ timeout: 120_000,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function runSui( | |
| args: string[], | |
| params: { | |
| cwd?: string; | |
| env?: NodeJS.ProcessEnv; | |
| label: string; | |
| redactions?: readonly string[]; | |
| }, | |
| ): string { | |
| console.log(`[sui] ${params.label}`); | |
| const result = spawnSync('sui', args, { | |
| cwd: params.cwd ?? REPO_ROOT, | |
| env: params.env, | |
| encoding: 'utf8', | |
| }); | |
| if (result.error) { | |
| throw result.error; | |
| } | |
| if (result.status !== 0) { | |
| const output = redact( | |
| [result.stdout, result.stderr].filter(Boolean).join('\n'), | |
| params.redactions ?? [], | |
| ); | |
| throw new Error(`${params.label} failed\n${output}`); | |
| } | |
| return result.stdout.trim(); | |
| } | |
| function runSui( | |
| args: string[], | |
| params: { | |
| cwd?: string; | |
| env?: NodeJS.ProcessEnv; | |
| label: string; | |
| redactions?: readonly string[]; | |
| }, | |
| ): string { | |
| console.log(`[sui] ${params.label}`); | |
| const result = spawnSync('sui', args, { | |
| cwd: params.cwd ?? REPO_ROOT, | |
| env: params.env, | |
| encoding: 'utf8', | |
| timeout: 120_000, | |
| }); | |
| if (result.error) { | |
| throw result.error; | |
| } | |
| if (result.status !== 0) { | |
| const output = redact( | |
| [result.stdout, result.stderr].filter(Boolean).join('\n'), | |
| params.redactions ?? [], | |
| ); | |
| throw new Error(`${params.label} failed\n${output}`); | |
| } | |
| return result.stdout.trim(); | |
| } |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for 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.
In `@scripts/sui/fundRfqVault.ts` around lines 113 - 142, The runSui helper
currently calls spawnSync without any timeout, so any hung sui invocation can
block the script indefinitely. Update runSui to pass a timeout option (and
handle the timeout case explicitly in the existing result.error/result.status
flow), and keep the behavior consistent for all callers such as the env setup,
keytool import, split-coin, and vault call steps.
| 'keytool', | ||
| 'import', | ||
| '--alias', | ||
| DEPLOYER_ALIAS, | ||
| '--json', | ||
| deployer.secret, | ||
| deployer.scheme, | ||
| ], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sui --versionRepository: SocketDotTech/openrouter
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of the script.
wc -l scripts/sui/fundRfqVault.ts
sed -n '180,270p' scripts/sui/fundRfqVault.tsRepository: SocketDotTech/openrouter
Length of output: 2257
🌐 Web query:
sui keytool import mnemonic alias bug fixed upstream release notes alias not kept when importing a key via mnemonic
💡 Result:
The bug where the --alias flag in the sui keytool import command was not correctly saved when importing a key via mnemonic has been fixed [1][2]. This issue was addressed in the Sui repository through pull requests such as #17935 and #20111 [1][2]. The fix ensures that when a user provides an alias during the mnemonic import process, the alias is properly recorded in the Sui CLI Keystore rather than being ignored or replaced by a default random alias [1][2]. If you are experiencing issues with aliases not persisting, ensure you are using an updated version of the Sui CLI. You can always verify or change an alias after importing a key using the update-alias command [3][4][5]: sui keytool update-alias
Citations:
- 1: [CLI] fix command
sui keytool importusing--aliasis useless when inputting mnemonic phrase MystenLabs/sui#17935 - 2: [CLI] fix
sui keytool import--alias command and default alias does not work MystenLabs/sui#20111 - 3: https://docs.sui.io/references/cli/cheatsheet
- 4: https://docs.sui.io/getting-started/onboarding/get-address
- 5: https://docs.sui.io/references/cli/keytool
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline scripts/sui/fundRfqVault.ts --view expanded
printf '\n---\n'
rg -n "resolveDeployerConfig|DEPLOYER_ALIAS|update-alias|mnemonic|secret|scheme" scripts/sui/fundRfqVault.tsRepository: SocketDotTech/openrouter
Length of output: 2035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '40,62p' scripts/sui/fundRfqVault.tsRepository: SocketDotTech/openrouter
Length of output: 727
Don't rely on keytool import --alias for mnemonic inputs. Older sui releases can drop the alias during mnemonic import, which breaks the later switch --address DEPLOYER_ALIAS step. This should either pin a fixed CLI version or set the alias explicitly after import.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for 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.
In `@scripts/sui/fundRfqVault.ts` around lines 219 - 226, The mnemonic import flow
in keytool import currently depends on --alias, which can be lost on older Sui
releases and break the later switch --address DEPLOYER_ALIAS step. Update the
fundRfqVault import path to either pin a known-safe Sui CLI version or
explicitly set/verify the alias after import in the keytool import/deployer
handling logic so DEPLOYER_ALIAS is guaranteed to exist.
2 participants
Summary by CodeRabbit