refactor(sdk): throwing getters for signer/credentials, drop operation args#324
Draft
ghermet wants to merge 17 commits intoprereleasefrom
Draft
refactor(sdk): throwing getters for signer/credentials, drop operation args#324ghermet wants to merge 17 commits intoprereleasefrom
ghermet wants to merge 17 commits intoprereleasefrom
Conversation
Design for replacing requireSigner/requireCredentialService with throwing getters and a hasSigner peek. Async require methods stay as-is. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrap the assertNonNullable TypeError in SignerNotConfiguredError via the options.cause field, preserving the underlying assertion for diagnostics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec corrections (operation stays on SignerRequiredError as optional metadata for the wallet errors) plus a task-by-task plan covering error refactor, signer getter, credentialService getter, call site migration, API regen, and docs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
operation becomes optional on SignerRequiredError (passed via the options bag). WalletNotConnectedError and WalletAccountNotReadyError continue to forward operation. SignerNotConfiguredError no longer interpolates the operation name in its message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SignerNotConfiguredError, WalletNotConnectedError, and WalletAccountNotReadyError now extend ZamaError directly. SignerRequiredError is removed. None of these errors take an operation argument anymore — messages are generic. Drop the unused operation parameter from GenericSigner.requireWalletAccount and from the viem/ethers/base signer implementations and call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sSigner The signer field is now a public throwing getter backed by #signer; reading sdk.signer when no signer is configured throws SignerNotConfiguredError whose cause is the underlying TypeError from assertNonNullable. hasSigner provides a non-throwing peek for callers that want to test for configuration without throwing (disposal path, useWalletAccount snapshot read). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…getter The backing field is now #credentialServiceField; the throwing accessor #credentialService is a private getter that asserts non-null and rethrows the TypeError as the cause of SignerNotConfiguredError. Internal peek call sites use #credentialServiceField directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reflects the new ZamaSDK.signer getter + hasSigner peek, the removed requireSigner method, and the flattened SignerRequiredError hierarchy (SignerNotConfiguredError / WalletNotConnectedError / WalletAccountNotReadyError now extend ZamaError directly, no operation field). GenericSigner / BaseSigner.requireWalletAccount() lost its operation parameter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The base SignerRequiredError class is gone — SignerNotConfiguredError now extends ZamaError directly. Replace stale entries in error tables with SignerNotConfiguredError / SIGNER_NOT_CONFIGURED, and remove the line about the error carrying the operation name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dentialService for the field Swap the names so the backing field gets the natural #credentialService identifier and the throwing private getter becomes #credentials. Local variables follow: callers receive the service as `credentials`, and the bundle returned from credentials.allow() is named `bundle` to disambiguate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…unt with getAccount Drop the two require* methods and the operation argument they only fed to ChainMismatchError. Expose a single getAccount() that returns the wallet account after asserting signer/provider chain alignment. Group-B callers that only wanted the side-effect now await getAccount() and discard the return; group-A callers (the majority) already wanted the account address. ChainMismatchError loses its operation field too — generic message now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
More descriptive name, matches the property being returned. Updates all call sites in Token, ReadonlyToken, ZamaSDK internals, tests, and the API reports. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Public API Changes
|
Coverage Report
File Coverage |
The throwing-getter refactor has shipped (PR #324). Drop the working documents from the repo — the PR body and commit history are the durable record. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ng-getters-signer-credentials # Conflicts: # packages/sdk/src/token/token.ts
…reAccount to throwing getter Two changes on the throwing-getter branch: 1. Revert 2418fe1 ("rename getAccount to getWalletAccount on ZamaSDK"): the shorter name reads better in the call sites and matches the existing convention. Updates ZamaSDK, Token, ReadonlyToken, the three SDK test files, and the api reports. 2. Convert ViemSigner.#requireAccount() into a private throwing getter (#account), matching the pattern this branch adopted for ZamaSDK.signer and ZamaSDK.#credentials. The returned walletClient field was redundant (always this.#walletClient); call sites now grab the field directly. Uses assertNonNullable + WalletNotConnectedError({ cause }) for parity with the other throwing getters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restores the shorter `SignerRequiredError` name (last used before 5aeeaa3 flattened the signer error hierarchy). Renames the error class, the matching `ZamaErrorCode.SignerNotConfigured` enum key, and its "SIGNER_NOT_CONFIGURED" string value to keep the code aligned with the class name as the rest of the enum does. Touches the class definition, error code enum, top-level export, every JSDoc @throws reference in ZamaSDK / ReadonlyToken / config types, the EthersSigner throw site, the SDK and React-SDK optional-signer tests, the api report, and the gitbook errors / handle-errors / ZamaSDK pages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
ZamaSDK.requireSigner(op)with a throwingsignergetter (backed by#signer) plus a non-throwinghasSignerpeek. The throwing path goes throughassertNonNullableand wraps theTypeErroras thecauseofSignerNotConfiguredError.#requireCredentialService(op)into a private throwing getter#credentialsover the renamed backing field#credentialService.requireChainAlignment(op)andrequireAlignedWalletAccount(op)into a single asyncsdk.getWalletAccount()— no production code consumed the chain-id return value, and every call site already needed the chain-alignment assertion.operation: stringargument fromSignerNotConfiguredError,WalletNotConnectedError,WalletAccountNotReadyError, andChainMismatchError. Flatten the hierarchy:SignerRequiredErroris removed; the three errors now extendZamaErrordirectly.operationparameter fromGenericSigner.requireWalletAccount()and the viem/ethers/base/wagmi implementations + tests.Breaking changes
sdk.signerno longer typedGenericSigner | undefined. Reading it without a configured signer throwsSignerNotConfiguredError. Usesdk.hasSignerto peek.sdk.requireSigner(...),sdk.requireChainAlignment(...),sdk.requireAlignedWalletAccount(...)are removed. Usesdk.signerandsdk.getWalletAccount().SignerRequiredErrorremoved.error.operationno longer set onSignerNotConfiguredError,WalletNotConnectedError,WalletAccountNotReadyError,ChainMismatchError.GenericSigner.requireWalletAccount()no longer takes an operation argument.Test plan
pnpm typecheckclean across all 6 typechecking packagespnpm -w run test:run— 1580 passed / 5 skippedpnpm lint— 0 warnings, 0 errorspnpm api-report:checkcleanerror.operationare migrated🤖 Generated with Claude Code