Skip to content

feat(scripts): recover stray SUI from the stSUI CollectionFeeCap - #44

Merged
harshaalphafi merged 2 commits into
mainfrom
bugfix/lst-receive-stray
Aug 12, 2026
Merged

feat(scripts): recover stray SUI from the stSUI CollectionFeeCap#44
harshaalphafi merged 2 commits into
mainfrom
bugfix/lst-receive-stray

Conversation

@Zorag44

@Zorag44 Zorag44 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Add scripts/receiveStray.ts to pull the 1513.878 SUI mistakenly sent to CollectionFeeCap 0x019466989a. Guards on receive_stray existing, the cap owner signing, and the coin still being unclaimed, so it no-ops until the liquid_staking upgrade lands.

Port scripts/utils.ts to SuiGrpcClient: public fullnodes no longer serve JSON-RPC, and 2.22.0 dropped SuiClient/fromB64.

Fix scripts/test.ts against current src signatures — explicit lstInfo and lstCoinType params, setValidators/updateFee renames, required typeName on event queries.

Add scripts/receiveStray.ts to pull the 1513.878 SUI mistakenly sent to
CollectionFeeCap 0x019466989a. Guards on receive_stray existing, the cap
owner signing, and the coin still being unclaimed, so it no-ops until the
liquid_staking upgrade lands.

Port scripts/utils.ts to SuiGrpcClient: public fullnodes no longer serve
JSON-RPC, and 2.22.0 dropped SuiClient/fromB64.

Fix scripts/test.ts against current src signatures — explicit lstInfo and
lstCoinType params, setValidators/updateFee renames, required typeName on
event queries.
@Zorag44
Zorag44 requested review from jangid and rg-alpha August 7, 2026 20:38
@jangid

jangid commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

operate.md review — re-run against head ee21dda (was aa012d3). The two items that gated approval are fixed and verified in the current diff; approving. Updated status of every prior finding, plus one new item.

Resolved

  1. RECIPIENT validationscripts/receiveStray.ts now routes the env value through requireAddress(), which rejects anything isValidSuiAddress() doesn't accept (so the left-padding case is refused rather than silently accepted), exits 1 on failure, and returns the normalized form that the recipient: summary line prints. Fix matches the finding exactly.
  2. Execution failure no longer exits 0executeTransactionBlock returns boolean, checks res.$kind === "FailedTransaction" explicitly (the MoveAbort case that does not throw) and catches thrown errors; dryRunTransactionBlock does the same for simulateTransaction; main() exits 1 on !ok. Both the abort path and the throw path now fail loudly.

Still open

  1. 🟡 Nothing compiles scripts/, unchanged this run. tsconfig.scripts.json still has "include": ["src/**/*", "scripts/migration-snapshot.ts"], lint is eslint 'src/**/*.ts', and no CI job typechecks the directory — so all three changed files remain outside every gate, including future @mysten/sui bumps. Adding scripts/**/* there plus a tsc --noEmit -p tsconfig.scripts.json step is the whole fix.
  2. 🟢 The bare catch around getMoveFunction still conflates "not published yet" with any transient RPC error — fails safe, misdiagnoses.
  3. 🟢 CAP still duplicates getConf().COLLECTION_FEE_CAP_ID; NETWORK=testnet still resolves production ids.
  4. 🟢 Dry run still needs PK_B64, because dryRunTransactionBlock calls getExecStuff() before using the sender argument.

New

  1. 🟡 dotenv is imported but is not a dependency, so the documented invocation cannot start. scripts/utils.ts:5 does import * as dotenv from "dotenv", but dotenv appears in neither package.json nor package-lock.json (zero occurrences) — I confirmed a clean tsc over the three files reports exactly one error, TS2307: Cannot find module 'dotenv', and nothing else. npx tsx scripts/receiveStray.ts therefore dies with ERR_MODULE_NOT_FOUND before any of the new logic runs. Pre-existing in utils.ts and loud rather than dangerous, but this PR's deliverable is a script an operator runs by hand under time pressure, so it should be a working one: add dotenv to devDependencies (it also unblocks item 3, since a scripts typecheck would have caught this).

Verified, no action

  • receive_stray<P, T: key + store>(self: &mut CollectionFeeCap<P>, receiving: Receiving<T>): T on alpha@bugfix/lst-receive-stray matches the PTB built here: typeArguments are [P, T], the script derives P from cap.type and passes the coin's own type as T, and the function returns the object rather than transferring it — so deciding the destination client-side is correct.
  • scripts/test.ts's updated call signatures typecheck against src/ on this branch, so the API migration in that file is consistent even though CI never checks it.
  • fetchTotalStakerss() is now the enabled top-level call where apr() used to be — matches the file's uncomment-to-run convention, noting it only so it isn't an accidental leftover.

@jangid

jangid commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

@Zorag44 — two fixes requested before this can be approved (details in the operate.md comment above): (1) validate RECIPIENT as a full normalized 66-char address and print the normalized destination — right now a truncated paste is silently zero-padded into a valid dead address and --execute would send the 1513.878 SUI there irrecoverably; (2) make the execute path detect FailedTransaction/thrown errors and exit non-zero — today a failed recovery exits 0. Both are small; there's time since the script no-ops until alpha#987 deploys.

Reject addresses that BCS would silently zero-pad into a valid dead address,
and print the normalized destination instead of the raw env value.

Return status from execute/dry-run so a MoveAbort — which does not throw, it
returns $kind: FailedTransaction — exits non-zero instead of 0.

@jangid jangid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operate.md: clean — CI green; the two items that gated the earlier review (RECIPIENT validation, non-zero exit on tx failure) are verified fixed in ee21dda. Non-blocking follow-ups in the review comment: dotenv is imported by scripts/utils.ts but is not a declared dependency, so the documented invocation cannot start until it is added; and scripts/ is still outside every CI gate. Approvals: 2/2 — threshold met, merge is a separate call.

@harshaalphafi
harshaalphafi merged commit 8d83fb5 into main Aug 12, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants