feat(sdk): add on-chain management and read boxes in one request - #24
Merged
Conversation
Expand the SDK with NFD lifecycle management, marketplace offers, property verification, and name suggestion capabilities. These features complete the core NFD owner experience beyond minting and purchasing. NfdManager additions: - Add getRenewalPrice() and renew() for NFD renewal with configurable year count (1-20) - Add listForSale() and cancelSale() for marketplace listing with optional address reservation - Add lockSegment() for toggling segment minting with USD pricing - Add lockVault() for controlling vault opt-in permissions - Add sendToVault() for opting vault into assets with optional transfer (supports both ALGO and ASAs) - Add sendFromVault() for sending assets from vault to a receiver PurchasingModule additions: - Add makeOffer() to post offers on NFDs via the instance contract's postOffer method NfdApiClient additions: - Add suggest() for name suggestion/autocomplete with buyer eligibility filtering - Add verifyRequest() and verifyConfirm() for the two-step property verification flow (twitter, github, domain, email, bluesky, avatar, banner) NfdClient additions: - Add suggest(), makeOffer(), verifyRequest(), verifyConfirm() as top-level convenience methods with signer management - verifyRequest resets signer after use; verifyConfirm does not require a signer Type and export additions: - Add SuggestOptions, ListForSaleOptions, SendToVaultOptions, SendFromVaultOptions, VerifyField, VerifyRequestResult, VerifyConfirmResult types - Add ALGORAND_ZERO_ADDRESS constant for default sale reservation Test updates: - Add client-level tests for makeOffer, suggest, verifyRequest, verifyConfirm including signer reset behavior - Add PurchasingModule tests for makeOffer with amount coercion, missing appID, and error wrapping - Add new manager.test.ts test file (untracked) Other: - Add CLAUDE.md with project documentation for Claude Code
Resolving an NFD listed every box, then fetched each value one at a time in a sequential loop. Every public entry point routes through LookupModule.resolve(), so that cost was paid on nearly every call. getAllBoxes() (src/utils/internal/boxes.ts) now reads names and values together via the algod include=values query parameter. algokit-utils does not expose it at any version (its AppManager.getBoxValues() is still one HTTP request per box), so the read goes through the raw algod client. Measured on MainNet against armyids.voi.algo (14 boxes, view: 'full'): 16 box requests and 1801ms before, 2 requests and 373ms after, with identical output. Both readers share it — LookupModule via a BaseModule wrapper and the slim NfdResolver directly — so buildNfdRecord() no longer takes a getBoxValue callback and is now synchronous. Global state and boxes are also fetched concurrently rather than in sequence. The read paginates, which it never did before. It follows nextToken, pins pages after the first to the first page's round, and throws rather than looping if a node repeats a cursor. It throws as well when a box comes back without a value, since a node that ignores include=values would otherwise yield an NFD silently missing all of its properties. LookupModule.resolveWithBoxes() returns the raw boxes alongside the NFD. NfdManager uses it to drop a duplicate read of v.caAlgo.0.as in linkAddress(), which getNfd() had already fetched, and caches the NFD and its boxes behind one invalidate() so the two cannot diverge. Also declares typescript-eslint, which both eslint configs import but nothing depended on. It resolved only under pnpm 9's hoisting, so lint fails for every package under the pinned pnpm 10. Tests: adds tests/utils/boxes.test.ts for the bulk read, pagination and cursor guard, and extends nfd-record tests with split-field ordering and zero-slot caAlgo cases. Adds linkAddress coverage to the manager tests, which it lacked entirely. BREAKING CHANGE: algosdk must now be v3.6.0 or later. The SDK reads NFD properties with the include=values box query parameter, added in algosdk v3.6.0; on an older algosdk, resolving an NFD fails at runtime rather than at install time. It also needs an algod node new enough to honour include=values, which the public MainNet and TestNet nodes already are.
The vault and sale methods were built against the ARC-56 JSON, which carries arg types but not a single assert. Reading the TealScript source turned up several rules the SDK was ignoring — one of which made sendToVault unusable. - sendToVault built its group as [vaultOptIn] first with no MBR payment, so every opt-in failed `assert(groupIndex > 0, 'must have prior txn paying MBR')`. The group is now [MBR payment] -> [vaultOptIn] -> [optional transfer], funding 0.1 ALGO per asset (VAULT_OPT_IN_MBR). - vaultSend rejects otherAssets whenever amount is non-zero, and requires amount > 0 with no other assets for ALGO. sendFromVault documented the opposite; it now validates both, and refuses to send the NFD's own ASA to anyone but the owner. - Most owner-driven writes sit behind assertOwnerCalledNotForSaleOrExpired(). sendToVault, sendFromVault, lockSegment and lockVault now check that up front, as do listForSale (expired, minting, and the zero-box requirement) and cancelSale (must be for sale). - lockSegment validates an unlock price against the registry's segmentPlatformCostInUsd, so the default of 0 no longer guarantees failure. - renew reads maxYearsAllowed from the registry rather than a hardcoded 20. Two unrelated correctness fixes in the same surface: - parseAppId used parseInt, so the all-digit NFD name '123.algo' resolved to app 123. It now requires the whole string to be numeric. - resolveVaultReceiver validates a non-name receiver as an address instead of letting ABI encoding fail inside the send, where it read as a transaction failure. CLAUDE.md tabulates the asserts each SDK method mirrors and points at the contract repo via $NFD_CONTRACTS, so the next reader does not have to re-derive them from a JSON that carries none of it. It also states what the suite cannot prove: the composer is mocked, so a green run says nothing about whether a node would accept the group — sendToVault was malformed while its tests passed. Also fixes the two ways `pnpm format` disagreed with `pnpm format:check` about scope. It was `pnpm -r format`, which skipped root-level files and resolved a different .prettierignore per package, rewriting the generated contract clients that the root run correctly excludes. Both are now `prettier .` from the root, and the package ignore lists the generated paths for direct runs.
PUBLISHING.md still documented the pre-semantic-release setup: an NPM_TOKEN secret, a "Publish Package" workflow with manual dispatch and an override tag input, and alpha/beta/next prerelease branches. None of those exist. Releases come from .releaserc.js (branches: ['main']) via release.yml, which authenticates to npm with OIDC trusted publishing and to GitHub with an App token. Documents what is actually there, plus two things that are easy to get wrong: - The Angular preset has no breakingHeaderPattern, so `feat(core)!:` does not parse as a feat and contributes no bump on its own. A major needs the BREAKING CHANGE footer. - Squash-merging makes the PR title the commit header semantic-release reads, so a PR titled without a conventional type releases nothing. Also records the plugins-only invocation for a local dry run, since the npm and github plugins abort in verifyConditions without credentials.
pbennett
changed the title
CONTRIBUTING.md described a repository that has moved on: - "early development (pre-v1.0.0)" — the package is at 1.1.0. - "We use pnpm v9" — the root packageManager field pins pnpm 10, and engines requires >=10. - Branches named feature/*, where every branch since has been <type>/* matching the commit type. - The type list omitted build and ci, and gave no hint that the type decides the version bump. - The pre-PR checklist listed pnpm format, which is the writer, not the check CI runs, and skipped build and build:examples entirely. `pnpm run ci` is the whole sequence in CI's order. - The Angular convention link pointed at the archived angular.js repo. Adds the two traps a new contributor has no way to see: the generated files under src/api and src/contracts that a regeneration will discard, and the `!` breaking-change shorthand, which the Angular preset's header pattern rejects outright — so `feat(core)!:` without the footer parses as no type at all and releases nothing. Also records what the mocked module tests can and cannot prove, and points at PUBLISHING.md for the release mechanics. CLAUDE.md listed pnpm test:watch and pnpm test:coverage under commands that "run from the repo root". Neither exists at the root — both are SDK package scripts and need --filter.
Contributor
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three substantive commits, plus a docs pass. Releases as 2.0.0 — confirmed with a semantic-release dry run.
feat(sdk): add NFD management, offers, verification, and suggestionsNfdManagergains renew, list/cancel sale, segment and vault locks, and vault transfers. AddsmakeOffer, the two-step verification exchange, and name suggestions.feat(core)!: read NFD boxes in a single algod requestResolving an NFD listed every box, then fetched each value one at a time in a sequential loop — a cost paid on nearly every public entry point.
getAllBoxes()now reads names and values together via algod'sinclude=values, going through the raw algod client because algokit-utils exposes no bulk box API at any version. Measured on MainNet againstarmyids.voi.algo(14 boxes,view: 'full'): 16 requests and 1801 ms before, 2 requests and 373 ms after, identical output.Breaking:
algosdkmust now be>= 3.6.0..include()does not exist before it, so on an older algosdk this fails at runtime rather than at install.fix(core): honor the instance contract's rules for vault and sale callsThe vault and sale methods were written against the ARC-56 JSON, which carries arg types and not a single
assert. Reading the TealScript source turned up several rules the SDK ignored — one of which madesendToVaultunusable:sendToVaultbuilt its group as[vaultOptIn]first with no MBR payment, failingassert(groupIndex > 0, 'must have prior txn paying MBR')on every opt-in. Now[MBR payment] → [vaultOptIn] → [optional transfer], funding 0.1 ALGO per asset.sendFromVaultdocumented the opposite of whatvaultSendaccepts. It now validates the amount/asset combinations and refuses to send the NFD's own ASA to anyone but the owner.assertOwnerCalledNotForSaleOrExpired()up front, andlistForSale/cancelSalecheck their own preconditions, so a violation names the cure instead of returning an opaque assert failure.lockSegmentvalidates against the registry'ssegmentPlatformCostInUsd;renewreadsmaxYearsAllowedrather than a hardcoded 20.parseAppIdusedparseInt, so the all-digit NFD name123.algoresolved to app 123.CLAUDE.md tabulates the asserts each method mirrors and points at the contract repo via
$NFD_CONTRACTS. It also records what the suite cannot prove: the composer is mocked, so a green run says nothing about whether a node accepts the group —sendToVaultwas malformed while its tests passed.Documentation
Two
docs:commits, neither of which affects the version bump.PUBLISHING.mdstill described anNPM_TOKENsecret, a manual-dispatch "Publish Package" workflow and alpha/beta/next branches, none of which exist. It now documents the real pipeline, the plugin chain, OIDC trusted publishing, and a local dry-run recipe.CONTRIBUTING.mdhad drifted the same way: "pre-v1.0.0", pnpm v9,feature/*branches, a type list missingbuildandci, a pre-PR checklist namingpnpm formatrather than theformat:checkCI runs, and a dead link to the archived angular.js repo. It also now warns about the generated files undersrc/apiandsrc/contracts, and about the!breaking-change shorthand — the Angular preset has nobreakingHeaderPattern, sofeat(core)!:without the footer parses as no type at all and releases nothing.CLAUDE.mdlistedpnpm test:watchandpnpm test:coverageas root commands; neither exists at the root.Merging
Please merge, don't squash. The commits map to separate release-note entries; squashing collapses them into one, and the squash title would become the header semantic-release parses.