Skip to content

fix(sdk): drop dead @mysten/walrus import from /manual (WALM-145) - #897

Open
HoangDucBach wants to merge 3 commits into
devfrom
wyner/walm-145-bug-manual-entry-pulls-mystenwalrus-wasm-via-dead-import
Open

HoangDucBach wants to merge 3 commits into
devfrom
wyner/walm-145-bug-manual-entry-pulls-mystenwalrus-wasm-via-dead-import

Conversation

@HoangDucBach

@HoangDucBach HoangDucBach commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Ticket

WALM-145 — https://linear.app/mysten-labs/issue/WALM-145
GH #330 — #330

What changed?

  • Removed unused getWalrusClient(), _walrusClient, and walrusUpload() from MemWalManual.
  • No remaining import("@mysten/walrus") in the SDK. Blob download stays on HTTP fetch to the aggregator.
  • Docs no longer claim /manual uploads/downloads via @mysten/walrus.

Why is this needed?

The live remember path uploads through the relayer. The live recall path downloads over HTTP. getWalrusClient() still did await import("@mysten/walrus") with no call sites, so Next.js 15 Turbopack put Walrus WASM in the /manual module graph and hit /ROOT/ resolution errors.

Test plan

  • SDK tests, including packages/sdk/test/no-walrus-wasm.test.mjs
  • Import @mysten-incubation/memwal/manual under Turbopack/Vite without @mysten/walrus
  • recallManual still downloads blobs over HTTP

getWalrusClient had no call sites but the static specifier still pulled WASM into Turbopack. Download stays on HTTP fetch.

@ducnmm ducnmm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

WALM-145 is correctly implemented: getWalrusClient(), _walrusClient, and unused walrusUpload() are gone, SDK source has no @mysten/walrus specifier, and recallManual still downloads through HTTP walrusDownload. Version dump is right — unpublished 0.1.7 on dev vs 0.1.6 on main, so no bump; the new Fixed bullet sits under existing ## 0.1.7 in both changelogs.

Issue counts by severity

  • bugs: 0
  • suggestions: 2
  • nits: 1

Issues outside the diff

  • [nit] packages/sdk/src/manual.ts:23 — The file header now says remember uploads through the relayer, but the @example still says remember is “all client-side: embed → SEAL encrypt → Walrus upload → register”. That contradicts rememberManual. Change the example to the hybrid flow (embed → SEAL encrypt → relayer upload).

Comment thread packages/sdk/src/manual-entry.ts
Comment thread packages/sdk/src/manual.ts
/manual no longer uses @mysten/walrus. Stop advertising it and remove the silent walrusEpochs / walrusPublisherUrl config.
ducnmm
ducnmm previously requested changes Sep 11, 2026

@ducnmm ducnmm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Summary

Follow-up e6031c98 closed the prior comments: optional @mysten/walrus peer and install docs, dead walrusEpochs / walrusPublisherUrl, and the @example upload wording. WALM-145 itself is still correct.

Issue counts by severity

  • bugs: 1
  • suggestions: 0
  • nits: 0

CI JS jobs fail pnpm install --frozen-lockfile because pnpm-lock.yaml still lists @mysten/walrus under packages/sdk.

Comment thread packages/sdk/package.json
CI frozen-lockfile failed after the optional peer was removed from package.json.
@ducnmm
ducnmm dismissed their stale review September 11, 2026 01:22

Lockfile updated on 5354052 — packages/sdk no longer lists @mysten/walrus. No remaining issues.

@HoangDucBach
HoangDucBach requested a review from ducnmm September 11, 2026 01:55

@ducnmm ducnmm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, cleans up dead @mysten/walrus imports and adds verifying unit tests.

This branch has not been deployed

No deployments
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.

2 participants