Migrate Hashi SDK into the monorepo as @mysten/hashi#1140
Conversation
Copies the Hashi SDK source, tests, and committed codegen bindings from the standalone hashi-ts-sdk repo, reformatted to monorepo prettier style. Rewrites package.json to monorepo conventions (workspace peer deps, standard scripts), retargets codegen at the ../hashi sibling checkout, and wires up the release workflow, oxlint ignore, AGENTS.md, and a changeset. Also fixes latent type errors in tests surfaced by the new test:typecheck step (union narrowing in history lookups, parse-only $kind tag in a BCS enum input); the standalone repo never type-checked its test directory.
GitHub Actions
Inactive
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Runs the @mysten/hashi integration tests against a localnet, path-filtered to packages/hashi/**. Ported from hashi-ts-sdk's integration.yml with three adaptations for this monorepo: - ts-sdks has no hashi submodule, so clone MystenLabs/hashi at the bindings pin (cd2b81f) into ./hashi explicitly. - @mysten/sui is a workspace:^ dep resolving to dist/, so build it first via `pnpm turbo build --filter @mysten/hashi` (mirrors seal-ci.yml). - Use this repo's pinned action SHAs and node 24. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GitHub Actions
Inactive
|
The Turborepo CI failure is unrelated to this PR: the |
| @@ -0,0 +1,15 @@ | |||
| /************************************************************** | |||
There was a problem hiding this comment.
these generated files should be marked as generated via linguist configs so they auto-collapse in reviews
| bitcoinNetwork: BitcoinNetwork; | ||
| } | ||
|
|
||
| export interface HashiClientOptions<Name = 'HashiClient'> { |
There was a problem hiding this comment.
the default Name here seems wrong, I assume this is meant to be hashi
I think actually string is probably a better default here. If something accepts HashiClientOptions only string would allow any options object with different names set
hayes-mysten
left a comment
hayes-mysten
left a comment
There was a problem hiding this comment.
Overall this looks really good and aligns well with this repos best practices. Code looks clean and well organized.
I've gone over everything from a best practices and sdk usage perspective and it all looks pretty solid to me. I did not review it for correctness, edge cases, etc, and I did not really look at the tests or docs.
| export interface HashiClientOptions<Name = 'HashiClient'> { | ||
| name?: Name; | ||
| /** Sui network — determines Hashi object IDs and default Bitcoin network. */ | ||
| network: SuiNetwork; |
There was a problem hiding this comment.
we generally don't want to pass network separately, and instead get it from the client being extended
|
|
||
| import type { UtxoLookupResult } from './types.js'; | ||
|
|
||
| async function rpcCall( |
There was a problem hiding this comment.
I like just making these kinds of methods take a generic so you can pass in the data type rather than always casting after calling it
| guardianInfoProvider, | ||
| }: { | ||
| client: ClientWithCoreApi; | ||
| network: SuiNetwork; |
There was a problem hiding this comment.
network should always come from the client, and we should throw if its unsupported
| this.#packageId = resolvedPackageId; | ||
| this.#bitcoinNetwork = bitcoinNetwork ?? config?.bitcoinNetwork ?? 'testnet'; | ||
| this.#btcRpcUrl = btcRpcUrl; | ||
| this.#graphqlUrl = graphqlUrl ?? defaultGraphqlUrl(network); |
There was a problem hiding this comment.
why not just create a graphql client here
| }): Transaction => { | ||
| const tx = new Transaction(); | ||
| const btcType = `${this.#packageId}::btc::BTC`; | ||
| const coin = tx.add( |
There was a problem hiding this comment.
we've added tx.coin and tx.balance (tx.coin is just an alias for coinWithBalance).
You also don't need to tx.add explicitly here you can just do const coin = conWithBalance() || tx.coin(..).
| typeArguments: [`${this.#packageId}::btc::BTC`], | ||
| arguments: [balance], | ||
| }); | ||
| tx.transferObjects([coin], options.recipient); |
There was a problem hiding this comment.
I am curious about plans for if/when we start just sending users balances instead of coins
| if (sender) { | ||
| const dummyAmount = snap.bitcoinWithdrawalMinimum + 1n; | ||
| const btcType = `${this.#packageId}::btc::BTC`; | ||
| const tx = new Transaction(); |
There was a problem hiding this comment.
Are these methods just building exactly the same transactions as the client.tx transactions? This seems like we should be reusing those to get accurate gas estimates that stay in sync without manually keeping them in sync with changes
| parentId: tableId, | ||
| name: { | ||
| type: 'address', | ||
| bcs: bcs.Address.serialize(suiAddress).toBytes(), |
There was a problem hiding this comment.
we added a top level toBytes method to avoid the extra chained serialize call
| * the results. Each element is either an object or an `Error` (for | ||
| * missing / deleted objects). | ||
| */ | ||
| async #batchGetObjects(objectIds: string[], include?: { content?: boolean }) { |
There was a problem hiding this comment.
the core getObjects API already handles batching internally
| } | ||
| }`; | ||
|
|
||
| const res = await fetch(this.#graphqlUrl, { |
There was a problem hiding this comment.
this should use the graphql client, which will properly type all the graphql requests for you
There was a problem hiding this comment.
we are also a couple weeks away from having event queries work in grpc and the core APIs
|
@clud-bot review |
|
End of turn — I'll be woken when the reviewer reports back. |
![clud-bot[bot]](https://avatars.githubusercontent.com/in/2762678?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Hashi SDK monorepo migration — Approve ✅
Reviewed the non-generated surface (generated src/contracts/** and pure prettier reformatting skipped as low-risk). No blocking or major issues.
What checks out:
- Version
0.5.0+ aminorchangeset is the correct pattern — changesets own the bump, and the CHANGELOG matches sibling packages. No hand-bumped version. package.jsondeps/peer-deps cover all externalsrcimports;@mysten/suiasworkspace:^peer +@mysten/codegenworkspace devDep follows the walrus/payment-kit convention.- Both new workflows are sound:
pull_request(notpull_request_target), no secrets referenced, actions SHA-pinned, external hashi clone pinned tocd2b81f9(matches the CHANGELOG note and codegen config). OIDC trusted publishing mirrors the ~30 existingrelease-*.ymlworkflows. - The "test type fixes" are legitimate type-narrowing — the
TransactionHistoryItemunion guards and the$kindBCS tag are correct, not masking bugs.
Everything inline is non-blocking (mostly stale "submodule" wording inherited from the standalone repo). Fine to merge; worth a quick cleanup pass.
FYI, not a finding: in release-hashi.yml the if: github.ref == 'refs/heads/main' gate constrains the dispatch branch, not the ref input, so a maintainer can publish an arbitrary commit. This is byte-identical to every existing release-*.yml, so out of scope here.
|
|
||
| ### Patch Changes | ||
|
|
||
| - ff9a398: Bump hashi submodule to cd2b81f (no contract or binding changes) |
There was a problem hiding this comment.
nit (docs): "Bump hashi submodule to cd2b81f" — the monorepo has no hashi submodule; CI clones it as a separate checkout (hashi-ci.yml). Stale wording carried over from the standalone repo. Suggest "submodule" → "pin" (or "repo").
|
|
||
| ### Patch Changes | ||
|
|
||
| - 9422708: Add a package-level `README.md` so the npm landing page has a real overview (install, one quickstart snippet, link to the repo README for full docs). Also corrects stale `@mysten/hashi` references in the root README to the actual published name `@mysten/hashi`. |
There was a problem hiding this comment.
nit (docs): Self-contradictory — "corrects stale @mysten/hashi references … to the actual published name @mysten/hashi" (same name on both sides). The old name was @mysten-incubation/hashi; the first occurrence should read that.
| } | ||
|
|
||
| /** | ||
| * Invokes the `hashi-localnet` Rust CLI from the submodule. The CI workflow |
There was a problem hiding this comment.
nit (docs): "Invokes the hashi-localnet Rust CLI from the submodule" — in the monorepo it's a separate CI checkout (hashi-ci.yml), not a submodule. Suggest "from the submodule" → "from the hashi repo checkout".
| packages: [ | ||
| { | ||
| package: '@local-pkg/hashi', // TODO: update this when hashi is published on MVR. | ||
| path: '../../../hashi/packages/hashi', |
There was a problem hiding this comment.
nit (correctness): '../../../hashi/packages/hashi' resolves to a sibling MystenLabs/hashi clone outside ts-sdks. CI handles this explicitly, but local codegen silently requires the dev to have cloned hashi next to ts-sdks. Worth a one-line comment noting that prerequisite (matches the sibling-repo pattern in AGENTS.md).
|
|
||
| echo "Installing Bitcoin Core v${BITCOIN_VERSION}..." | ||
|
|
||
| wget -q https://bitcoincore.org/bin/bitcoin-core-${BITCOIN_VERSION}/bitcoin-${BITCOIN_VERSION}-x86_64-linux-gnu.tar.gz || { |
There was a problem hiding this comment.
low (security/supply-chain): the Bitcoin Core tarball is downloaded from bitcoincore.org with no SHA256SUMS/GPG verification before extraction. Blast radius is small (read-only token, no secrets, ephemeral runner), but consider fetching SHA256SUMS and verifying the hash before tar -xzf.
2 participants
Description
Migrates the Hashi TypeScript SDK (native BTC collateralization on Sui) from the standalone
hashi-ts-sdk repo into
packages/hashi, renaming itfrom
@mysten-incubation/hashi(v0.5.0 on npm) to@mysten/hashi(SEDEFI-163).to monorepo prettier style (tabs, single quotes, import sort) — most of the diff is this
mechanical reformat plus the generated
src/contracts/**bindings.@mysten/suias aworkspace:^peerdependency,
@mysten/codegenas a workspace devDependency, standard build/lint/test scripts.sui-codegen.config.ts) retargeted at the../hashisibling checkout(walrus/payment-kit pattern); bindings stay committed at the revision generated from
MystenLabs/hashi
cd2b81f.release-hashi.ymlworkflow (OIDC trusted publishing via_release-package.yml), oxlint ignore forpackages/hashi/src/contracts,AGENTS.mdsibling-repo/codegen entries, and a minor changeset — so the first publish under the new name is
0.6.0 with a changelog entry explaining the rename.
testtask; integrationtests remain env-gated (
HASHI_E2E_*, devnet/localnet) — the standalone repo's Rust/Bitcoin-Corelocalnet CI harness is intentionally not ported.
test:typecheckstep surfaced latent type errors the standalonerepo never caught (it never ran
tscover tests): unnarrowedTransactionHistoryItemunionaccess in three integration tests, and a parse-only
$kindtag in a BCS enum serialize input.README); a docs-site section can follow at graduation.
Post-merge follow-ups (outside this PR): dispatch
release-hashi.yml(dry-run first), deprecate@mysten-incubation/hashion npm, archive the hashi-ts-sdk repo.Test plan
pnpm turbo build --filter @mysten/hashi— typecheck + tsdown pass against workspace@mysten/sui2.20.x (verifies the peer-range move from^2.14.1)pnpm --filter @mysten/hashi test— test-dir typecheck + 185 unit tests passpnpm turbo lint --filter @mysten/hashi+ root oxlint — clean (0 findings on ported code)pnpm changeset status—@mysten/hashiqueued for a minor bump../hashisibling checkout (regenerates successfully;regenerated output not committed since that checkout is ahead of the pinned contract revision)
AI Assistance Notice
🤖 Generated with Claude Code