feat(sdk): align with acta-api v1.1.1 / vc-vault v0.3.0#2
Open
JosueBrenes wants to merge 1 commit into
Open
Conversation
Bring @acta-team/credentials to v1.1.2 with full surface for the new acta-api: typed errors (ActaError), CONTRACT_LIMITS, paginated reads, batch issuance, issuer-registry methods + hook, vault metadata, and React hooks for every new endpoint. Removed deprecated methods (createCredential, getDefaults, vaultMigrate, prepareStoreTx/ListVcIdsTx/GetVcTx, vaultStore) and the matching legacy types. `revokeCredentialViaApi` now requires `owner`. `listVcIds` paginates. `VaultVerifyVcResponse.status` widened to include `invalid`/`unknown`. Versioning note: strict semver would call most of these BREAKING and bump to 2.0.0. Staying on 1.1.x intentionally until the panel migrates; do not publish from this branch. See CHANGELOG.md for the full migration guide.
📝 WalkthroughWalkthroughThis PR aligns the ACTA SDK with ChangesSDK v1.1.2 alignment with acta-api v1.2.0 / vc-vault v0.3.0
Sequence Diagram(s)sequenceDiagram
participant App
participant Hook
participant Client
participant Signer
participant Server
App->>Hook: call mutation (e.g., batchIssue, authorizeIssuers)
Hook->>Client: prepare transaction
Client->>Server: POST /contracts/vault/... (prepare mode)
Server-->>Client: { xdr, network }
Client-->>Hook: TxPrepareResponse
Hook->>Signer: sign(xdr, networkPassphrase)
Signer-->>Hook: signedXdr
Hook->>Client: submit({ signedXdr })
Client->>Server: POST /contracts/vault/... (submit mode)
Server-->>Client: { tx_id }
Client-->>Hook: TxSubmitResponse
Hook->>Hook: validate responses
Hook-->>App: { txId }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/hooks/useCredential.ts (1)
152-154: ⚡ Quick winUse shared contract limit instead of hardcoded batch size.
Line 152 hardcodes
5whileCONTRACT_LIMITS.MAX_BATCH_SIZEalready exists for this exact constraint. Reusing it avoids future drift.Proposed fix
+import { CONTRACT_LIMITS } from "../utils/contract-limits"; ... - if (args.vcs.length > 5) { - throw new Error("batchIssue accepts at most 5 vcs per call"); + if (args.vcs.length > CONTRACT_LIMITS.MAX_BATCH_SIZE) { + throw new Error( + `batchIssue accepts at most ${CONTRACT_LIMITS.MAX_BATCH_SIZE} vcs per call` + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useCredential.ts` around lines 152 - 154, Replace the hardcoded limit 5 with the shared constant CONTRACT_LIMITS.MAX_BATCH_SIZE in the batch size check (the block referencing args.vcs in src/hooks/useCredential.ts), and make sure CONTRACT_LIMITS is imported where useCredential or batchIssue is defined; change the conditional to compare args.vcs.length against CONTRACT_LIMITS.MAX_BATCH_SIZE so the module uses the centralized limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useVault.ts`:
- Around line 118-123: The prepare call to client.vaultAuthorizeIssuers uses
sourcePublicKey = args.sourcePublicKey ?? args.owner which can pass a
smart-wallet owner (C...) as the sourcePublicKey and break preparation; update
the logic around sourcePublicKey and the call to vaultAuthorizeIssuers so you
first detect smart-wallet owners (e.g., an isSmartWalletOwner(args.owner) check)
and only derive sourcePublicKey from args.owner for non-smart-wallet owners,
otherwise require and use args.sourcePublicKey (or throw/return an error if
missing) before invoking client.vaultAuthorizeIssuers with owner: args.owner,
issuers: args.issuers, sourcePublicKey, contractId.
In `@src/utils/acta-error.ts`:
- Around line 133-135: The fallback classification in the ActaError constructor
incorrectly maps any HTTP response lacking the ACTA error shape to
"bad_request"; update the logic in the ActaError instantiation (where ActaError
is constructed) to inspect the numeric HTTP status and emit "network_error" (or
a server error code) for 5xx statuses and only "bad_request" for 4xx client
errors — e.g., set code to something like: if status is >=500 then
"network_error" else "bad_request", while keeping httpStatus as status ?? 0.
---
Nitpick comments:
In `@src/hooks/useCredential.ts`:
- Around line 152-154: Replace the hardcoded limit 5 with the shared constant
CONTRACT_LIMITS.MAX_BATCH_SIZE in the batch size check (the block referencing
args.vcs in src/hooks/useCredential.ts), and make sure CONTRACT_LIMITS is
imported where useCredential or batchIssue is defined; change the conditional to
compare args.vcs.length against CONTRACT_LIMITS.MAX_BATCH_SIZE so the module
uses the centralized limit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 845d5d66-878e-4351-b812-4745fc2ab5f1
📒 Files selected for processing (16)
CHANGELOG.mdpackage.jsonsrc/client.tssrc/hooks/index.tssrc/hooks/useCredential.tssrc/hooks/useIssuerRegistry.tssrc/hooks/useVault.tssrc/hooks/useVaultIssuers.tssrc/hooks/useVaultRead.tssrc/index.tssrc/types/api-responses.tssrc/types/index.tssrc/types/type.payload.tssrc/types/types.response.tssrc/utils/acta-error.tssrc/utils/contract-limits.ts
💤 Files with no reviewable changes (3)
- src/types/index.ts
- src/types/types.response.ts
- src/types/type.payload.ts
Comment on lines
+118
to
+123
| const sourcePublicKey = args.sourcePublicKey ?? args.owner; | ||
| const prepareResult = await client.vaultAuthorizeIssuers({ | ||
| owner: args.owner, | ||
| issuers: args.issuers, | ||
| sourcePublicKey, | ||
| contractId, |
There was a problem hiding this comment.
Guard authorizeIssuers for smart-wallet owners before deriving sourcePublicKey.
Line 118 can send a C... owner as sourcePublicKey when args.sourcePublicKey is omitted, which breaks prepare for smart-wallet owners.
Proposed fix
+ const isSmartAccountOwner =
+ args.owner.startsWith("C") && args.owner.length === 56;
+ if (isSmartAccountOwner && !args.sourcePublicKey) {
+ throw new Error(
+ "authorizeIssuers() requires sourcePublicKey when owner is a C... smart wallet"
+ );
+ }
const sourcePublicKey = args.sourcePublicKey ?? args.owner;
const prepareResult = await client.vaultAuthorizeIssuers({
owner: args.owner,
issuers: args.issuers,
sourcePublicKey,
contractId,
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useVault.ts` around lines 118 - 123, The prepare call to
client.vaultAuthorizeIssuers uses sourcePublicKey = args.sourcePublicKey ??
args.owner which can pass a smart-wallet owner (C...) as the sourcePublicKey and
break preparation; update the logic around sourcePublicKey and the call to
vaultAuthorizeIssuers so you first detect smart-wallet owners (e.g., an
isSmartWalletOwner(args.owner) check) and only derive sourcePublicKey from
args.owner for non-smart-wallet owners, otherwise require and use
args.sourcePublicKey (or throw/return an error if missing) before invoking
client.vaultAuthorizeIssuers with owner: args.owner, issuers: args.issuers,
sourcePublicKey, contractId.
Comment on lines
+133
to
+135
| return new ActaError({ | ||
| code: status ? "bad_request" : "network_error", | ||
| httpStatus: status ?? 0, |
There was a problem hiding this comment.
Fix fallback code classification for non-contract error payloads.
Line 134 currently maps every HTTP response without ACTA error shape to bad_request, including server failures. That breaks reliable err.code branching in consumers.
Proposed fix
- return new ActaError({
- code: status ? "bad_request" : "network_error",
+ const fallbackCode: ActaErrorCode =
+ status == null
+ ? "network_error"
+ : status >= 500
+ ? "internal_error"
+ : status === 401
+ ? "unauthorized"
+ : status === 403
+ ? "forbidden"
+ : status === 404
+ ? "not_found"
+ : status === 429
+ ? "rate_limit_exceeded"
+ : "bad_request";
+
+ return new ActaError({
+ code: fallbackCode,
httpStatus: status ?? 0,
message:
anyErr?.message || (status ? `HTTP ${status}` : "Network request failed"),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return new ActaError({ | |
| code: status ? "bad_request" : "network_error", | |
| httpStatus: status ?? 0, | |
| const fallbackCode: ActaErrorCode = | |
| status == null | |
| ? "network_error" | |
| : status >= 500 | |
| ? "internal_error" | |
| : status === 401 | |
| ? "unauthorized" | |
| : status === 403 | |
| ? "forbidden" | |
| : status === 404 | |
| ? "not_found" | |
| : status === 429 | |
| ? "rate_limit_exceeded" | |
| : "bad_request"; | |
| return new ActaError({ | |
| code: fallbackCode, | |
| httpStatus: status ?? 0, | |
| message: | |
| anyErr?.message || (status ? `HTTP ${status}` : "Network request failed"), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/acta-error.ts` around lines 133 - 135, The fallback classification
in the ActaError constructor incorrectly maps any HTTP response lacking the ACTA
error shape to "bad_request"; update the logic in the ActaError instantiation
(where ActaError is constructed) to inspect the numeric HTTP status and emit
"network_error" (or a server error code) for 5xx statuses and only "bad_request"
for 4xx client errors — e.g., set code to something like: if status is >=500
then "network_error" else "bad_request", while keeping httpStatus as status ??
0.
JosueBrenes
changed the title
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.
Summary
Bring `@acta-team/credentials` from v1.1.1 → v1.1.2 so it works against `acta-api` v1.2.0 (PR acta-api#82). New surface includes typed errors, paginated reads, batch issuance, full `vc-issuer-registry` coverage, and React hooks for every new endpoint.
Removed — BREAKING
Changed — BREAKING
Added — new client methods
Added — new React hooks
Added — DX
Migration guide
Test plan
Stats
Companion PRs
comment: release notes by coderabbit.ai -->