Conversation
✅ Deploy Preview for sentstx ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| "target": "ESNext", | ||
| "lib": ["dom", "dom.iterable", "esnext"], | ||
| "allowJs": false, | ||
| "target": "ES2023", |
There was a problem hiding this comment.
more modern version to make it compatible with clarity-abitype
There was a problem hiding this comment.
Pull request overview
This PR integrates the clarity-abitype library to provide type-safe interactions with Clarity smart contracts, replacing manual Clarity type handling with automated type inference from contract ABIs. The integration modernizes the TypeScript configuration for Vite bundler mode and removes unused dependencies.
Changes:
- Replaces manual Clarity CV type handling with
clarity-abitype'stypedCallReadOnlyFunctionfor type-safe contract reads - Adds ABI definitions for BNS v2 and sBTC Registry contracts in
src/lib/abi.ts - Updates TypeScript configuration to modern bundler mode with stricter linting options
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Modernizes TypeScript config for Vite bundler mode, removes several compiler options |
| src/test/names.test.ts | Updates test assertions to use clarity-abitype's unwrapped response/optional types |
| src/lib/string-utils.ts | Adds 0x prefix handling to hex_to_ascii for clarity-abitype buffer format |
| src/lib/names.ts | Migrates from fetchCallReadOnlyFunction to typedCallReadOnlyFunction with ABIs |
| src/lib/abi.ts | Adds comprehensive ABI definitions for sbtcRegistry and bnsV2 contracts |
| src/components/SendManyInputContainer.tsx | Refactors recipient resolution to use typed contract calls, changes toCV type from PrincipalCV to string |
| src/components/SBTCInfo.tsx | Updates to use typedCallReadOnlyFunction with sBTC Registry ABI |
| src/components/Address.tsx | Updates name resolution to handle clarity-abitype's response type structure |
| pnpm-lock.yaml | Adds clarity-abitype@0.3.1 dependency, removes unused @Scure packages |
| package.json | Adds clarity-abitype, removes @scure/bip32 and @scure/btc-signer |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| > | ||
| | ResponseErrorCV; | ||
| }); | ||
| console.log({ result }); |
There was a problem hiding this comment.
Debug console.log statement should be removed before merging to production. This appears to be leftover debugging code from the integration work.
| console.log({ result }); |
| functionArgs: [ | ||
| `0x${Array.from(new TextEncoder().encode(name)) | ||
| .map(b => b.toString(16).padStart(2, '0')) | ||
| .join('')}`, | ||
| `0x${Array.from(new TextEncoder().encode(namespace)) | ||
| .map(b => b.toString(16).padStart(2, '0')) | ||
| .join('')}`, | ||
| ], |
There was a problem hiding this comment.
The manual hex encoding logic is duplicated and could be extracted to a reusable helper function. Consider creating a function like stringToHexBuffer in string-utils.ts to handle the conversion from string to hex-prefixed buffer format. This would improve maintainability and reduce code duplication.
| const owner = await getNameInfo(toAscii(recipient)); | ||
| if (owner?.owner) { | ||
| return owner.owner; | ||
| } | ||
| throw new Error(`No address for ${recipient}`); | ||
| } |
There was a problem hiding this comment.
The error handling may be incorrect. According to the bnsV2Abi, the get-bns-info function returns an optional<tuple> (not a response type). With clarity-abitype, this means the result will be either the tuple object or null/undefined when the name doesn't exist. The current code checks owner?.owner, which assumes the result is the tuple. However, when a name doesn't exist, owner will be null/undefined, so the check should work. But for clarity, consider explicitly checking if (owner && owner.owner) or adding a comment explaining that owner is the optional tuple value.
| }); | ||
| }; | ||
|
|
||
| fn(addr); |
There was a problem hiding this comment.
Missing error handling for the async function call. If getNameFromAddress throws an error or rejects, the promise rejection will be unhandled. Consider adding a .catch() handler to the fn(addr) call to gracefully handle errors (e.g., network failures, invalid addresses). The error handler should silently fail or log the error without crashing the component, since the component already falls back to displaying the short address when no name is found.
| if (hexString.startsWith('0x')) { | ||
| hexString = hexString.slice(2); | ||
| } |
There was a problem hiding this comment.
The new 0x prefix handling in hex_to_ascii is not covered by tests. Consider adding a test case to verify that hex strings with '0x' prefix are correctly handled, for example: expect(hex_to_ascii('0x6672696564676572')).toBe('friedger'). This will ensure the clarity-abitype integration doesn't break if the buffer format changes.
WIP