chore: apply prettier formatting#135
Conversation
|
@cybermax4200 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughAdded a spec/design/requirements/tasks suite for an Changes
Sequence DiagramsequenceDiagram
participant Test as Smoke Test
participant Index as Index (re-exports)
participant Encoding as Encoding Module
participant Hashing as Hashing Module
participant Keys as Keys Module
participant Stellar as `@stellar/stellar-sdk`
Test->>Index: import * as Crypto
activate Index
Index->>Encoding: re-export encoding funcs
Index->>Hashing: re-export hash funcs
Index->>Keys: re-export key funcs
deactivate Index
Test->>Test: assert EXPECTED_EXPORTS present & exact
Test->>Stellar: derive deterministic keypair
Stellar-->>Test: keypair
Test->>Index: call verify/sign or exported async func
Index->>Hashing: perform hash/hmac (if needed)
Hashing-->>Index: digest
Index-->>Test: return result
Test->>Test: assert no console output
Test->>Encoding: call fromHex(invalid)
Encoding-->>Test: throw TypeError (message checked for no secret leakage)
Test->>Test: assert CRYPTO_VERSION is non-empty
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 unit tests (beta)
Comment |
|
Please Merge the PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui-kit/eslint.config.cjs (1)
12-21:⚠️ Potential issue | 🔴 Critical
parserOptionsis undefined and will cause ESLint config to fail immediately.Line 14 and Line 51 reference
parserOptions, but this variable is never declared in the file. Additionally, lines 15–21 defineglobalstwice in the same object; the second definition (line 19) overwrites the first (lines 15–18), dropping theglobals.nodeglobals that were merged in.Problematic code snippet
languageOptions: { parser: tsparser, parserOptions, globals: { ...globals.browser, ...globals.node, }, globals: { ...globals.browser, },Also applies to line 51 (test files block).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-kit/eslint.config.cjs` around lines 12 - 21, The ESLint config references an undeclared identifier parserOptions and mistakenly defines globals twice in languageOptions; fix by declaring a parserOptions constant (e.g., the desired parserOptions object) and use it as languageOptions.parserOptions instead of the bare identifier, and remove the duplicate globals key by merging both sets (spread ...globals.browser and ...globals.node once) so node globals aren't overwritten; apply the same fixes to the test-files languageOptions block (look for languageOptions, parserOptions, globals, and tsparser in the file).
🧹 Nitpick comments (2)
.kiro/specs/crypto-utilities-package/design.md (1)
139-156: Consider documenting the seed size gap betweenSeedResultandderiveKeyPair.The design documents
SeedResult.seedas a 64-byte BIP39 seed (line 151), which is correct. However, there's no mention thatderiveKeyPairfromkeys.tsrequires exactly 32 bytes. Consumers may assume these APIs work together seamlessly.Adding a note about the derivation workflow (e.g., "use first 32 bytes" or "apply SLIP-0010 derivation") would prevent confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.kiro/specs/crypto-utilities-package/design.md around lines 139 - 156, The docs define SeedResult.seed as a 64-byte BIP39 seed but don't state that deriveKeyPair (in keys.ts) expects a 32-byte input, which can mislead consumers; update the design.md to explicitly document the derivation workflow by noting the mismatch and the expected approach (e.g., "deriveKeyPair consumes the first 32 bytes of the 64-byte BIP39 seed" or alternatively "apply SLIP-0010/ED25519 key derivation from the 64-byte seed before calling deriveKeyPair"), and reference the SeedResult.seed and deriveKeyPair symbols so readers know how to convert the 64-byte seed into the 32-byte input required by deriveKeyPair.packages/crypto/src/__tests__/smoke.test.ts (1)
97-109: Secret material test does not exercise the secret with CryptoAPI.The test creates
secretandsecretHexbut only callsCryptoAPI.fromHex('not-valid-hex!')— an unrelated invalid input. The assertion that the error message doesn't containsecretHexis trivially true since the invalid input'not-valid-hex!'has no relation tosecretHex.A more meaningful test would pass
secretHex(or a corrupted version) to an encoding function and verify the error doesn't leak it.♻️ Proposed improvement
it('does not include secret bytes in error messages from encoding functions', () => { const secret = new Uint8Array(32).fill(0xab); const secretHex = Buffer.from(secret).toString('hex'); + // Corrupt the secret hex to make it invalid (odd length) + const corruptedSecretHex = secretHex + 'z'; - expect(() => CryptoAPI.fromHex('not-valid-hex!')).toThrow(); + expect(() => CryptoAPI.fromHex(corruptedSecretHex)).toThrow(); try { - CryptoAPI.fromHex('not-valid-hex!'); + CryptoAPI.fromHex(corruptedSecretHex); } catch (e) { expect((e as Error).message).not.toContain(secretHex); + expect((e as Error).message).not.toContain(corruptedSecretHex); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crypto/src/__tests__/smoke.test.ts` around lines 97 - 109, The test creates secret/secretHex but never uses them, so the leak check is vacuous; update the test ("does not include secret bytes in error messages from encoding functions") to invoke the encoding API with the secret (or a slightly corrupted secret) so any thrown error could potentially include it—specifically call CryptoAPI.fromHex(secretHex) (or a mutated variant) inside the expect/toThrow or try/catch and then assert that (e as Error).message does not contain secretHex; keep the existing variables secret and secretHex and the test name, but replace the unrelated 'not-valid-hex!' input with secretHex or a corrupted secret to properly exercise CryptoAPI.fromHex for leakage checking.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.kiro/specs/crypto-utilities-package/design.md:
- Around line 94-101: The design lists signing.ts exporting signMessage but the
implementation currently exports signTransaction; reconcile by adding or
renaming so the API matches the design: update signing.ts to export a
signMessage function (and optionally export signTransaction as an alias) and
update index.ts to re-export signMessage (or change the design doc if you prefer
signTransaction), ensuring both the module export names (signing.ts:
signMessage, signTransaction) and the public re-exports in
index.ts/CRYPTO_VERSION reflect the chosen name consistently.
In `@packages/crypto/src/__tests__/smoke.test.ts`:
- Around line 3-23: The smoke test's EXPECTED_EXPORTS array lists many symbols
(e.g., CRYPTO_VERSION, verifySignature, signMessage, sha256, sha512, hmac,
deriveKeyPair, publicKeyFromSecret, generateMnemonic, mnemonicToSeed, toHex,
fromHex, toBase64, fromBase64) that don't match what index.ts actually exports,
causing CryptoAPI[symbol] to be undefined and actualKeys to differ; fix by
either (A) updating index.ts to re-export all the listed symbols so the public
API matches EXPECTED_EXPORTS (ensure functions like verifySignature,
signMessage, sha256, deriveKeyPair, generateMnemonic, toHex, etc. are exported
from the barrel) or (B) adjust EXPECTED_EXPORTS to exactly reflect the current
exports in index.ts so CryptoAPI lookups and actualKeys comparison succeed.
In `@packages/crypto/src/encoding.ts`:
- Around line 19-27: The round-trip check in fromBase64 rejects valid unpadded
base64 because it compares the re-encoded string (which always includes padding)
to the original b64; change the validation to normalize both sides by stripping
trailing '=' padding before comparison (i.e., compare decoded.toString('base64')
with b64 after removing any trailing '='), so fromBase64 accepts valid unpadded
inputs while still catching malformed data.
In `@packages/crypto/src/hashing.ts`:
- Around line 12-23: The package barrel is missing exports for the hashing
functions: add named exports for sha256, sha512, and hmac to the crypto
package's public index (export the symbols sha256, sha512, hmac from
packages/crypto/src/index.ts) so the smoke-test EXPECTED_EXPORTS array can
access CryptoAPI['sha256'], CryptoAPI['sha512'], and CryptoAPI['hmac']; ensure
you import them from their defining module (the file that contains the
sha256/sha512/hmac implementations) and re-export them from the barrel.
In `@packages/crypto/src/keys.ts`:
- Around line 12-20: deriveKeyPair currently only accepts a 32-byte seed and
fails when fed the 64-byte BIP39 output from mnemonicToSeed; update
deriveKeyPair(seed: Uint8Array) so it accepts both forms by: if seed.length ===
64, derive a 32-byte Ed25519 seed by taking seed.slice(0, 32) (or clearly
document/rename if you prefer to only accept raw ed25519 seeds), preserve the
existing behavior for seed.length === 32, and throw an error for any other
length; reference deriveKeyPair and mnemonicToSeed so reviewers can find and
validate the change.
In `@packages/crypto/src/mnemonic.ts`:
- Around line 3-13: The generated BIP39 seed is 64 bytes but deriveKeyPair
requires a 32-byte seed, so add a small helper and surface a 32-byte seed:
create a new function mnemonicToSeed32(mnemonic: string): Promise<Uint8Array>
that calls bip39.mnemonicToSeed(mnemonic) and returns the first 32 bytes as a
Uint8Array, update the SeedResult interface to include seed32: Uint8Array, and
modify generateMnemonic() to fill both seed (64-byte) and seed32 (32-byte) by
calling mnemonicToSeed32; reference generateMnemonic, mnemonicToSeed, and
deriveKeyPair so callers can pass seed32 into deriveKeyPair.
---
Outside diff comments:
In `@packages/ui-kit/eslint.config.cjs`:
- Around line 12-21: The ESLint config references an undeclared identifier
parserOptions and mistakenly defines globals twice in languageOptions; fix by
declaring a parserOptions constant (e.g., the desired parserOptions object) and
use it as languageOptions.parserOptions instead of the bare identifier, and
remove the duplicate globals key by merging both sets (spread ...globals.browser
and ...globals.node once) so node globals aren't overwritten; apply the same
fixes to the test-files languageOptions block (look for languageOptions,
parserOptions, globals, and tsparser in the file).
---
Nitpick comments:
In @.kiro/specs/crypto-utilities-package/design.md:
- Around line 139-156: The docs define SeedResult.seed as a 64-byte BIP39 seed
but don't state that deriveKeyPair (in keys.ts) expects a 32-byte input, which
can mislead consumers; update the design.md to explicitly document the
derivation workflow by noting the mismatch and the expected approach (e.g.,
"deriveKeyPair consumes the first 32 bytes of the 64-byte BIP39 seed" or
alternatively "apply SLIP-0010/ED25519 key derivation from the 64-byte seed
before calling deriveKeyPair"), and reference the SeedResult.seed and
deriveKeyPair symbols so readers know how to convert the 64-byte seed into the
32-byte input required by deriveKeyPair.
In `@packages/crypto/src/__tests__/smoke.test.ts`:
- Around line 97-109: The test creates secret/secretHex but never uses them, so
the leak check is vacuous; update the test ("does not include secret bytes in
error messages from encoding functions") to invoke the encoding API with the
secret (or a slightly corrupted secret) so any thrown error could potentially
include it—specifically call CryptoAPI.fromHex(secretHex) (or a mutated variant)
inside the expect/toThrow or try/catch and then assert that (e as Error).message
does not contain secretHex; keep the existing variables secret and secretHex and
the test name, but replace the unrelated 'not-valid-hex!' input with secretHex
or a corrupted secret to properly exercise CryptoAPI.fromHex for leakage
checking.
🪄 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: a05686be-0892-4383-b4c6-480c61e9f29c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.kiro/specs/crypto-utilities-package/.config.kiro.kiro/specs/crypto-utilities-package/design.md.kiro/specs/crypto-utilities-package/requirements.md.kiro/specs/crypto-utilities-package/tasks.mdpackages/account-abstraction/eslint.config.cjspackages/account-abstraction/src/account-contract.tspackages/core-sdk/eslint.config.cjspackages/core-sdk/src/account-transaction-builder.tspackages/crypto/eslint.config.cjspackages/crypto/package.jsonpackages/crypto/src/__tests__/smoke.test.tspackages/crypto/src/encoding.tspackages/crypto/src/hashing.tspackages/crypto/src/index.tspackages/crypto/src/keys.tspackages/crypto/src/mnemonic.tspackages/types/eslint.config.cjspackages/ui-kit/eslint.config.cjs
💤 Files with no reviewable changes (1)
- packages/core-sdk/src/account-transaction-builder.ts
| | Submodule | Public Exports | | ||
| | ------------- | -------------------------------------------- | | ||
| | `signing.ts` | `verifySignature`, `signMessage` | | ||
| | `hashing.ts` | `sha256`, `sha512`, `hmac` | | ||
| | `keys.ts` | `deriveKeyPair`, `publicKeyFromSecret` | | ||
| | `mnemonic.ts` | `generateMnemonic`, `mnemonicToSeed` | | ||
| | `encoding.ts` | `toHex`, `fromHex`, `toBase64`, `fromBase64` | | ||
| | `index.ts` | `CRYPTO_VERSION` + all of the above | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what signing.ts actually exports
echo "=== Exports from signing.ts ==="
rg -n "^export (async )?function" packages/crypto/src/signing.tsRepository: ancore-org/ancore
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Check what index.ts exports related to signing
echo "=== Exports from index.ts related to signing ==="
rg -n "export.*sign" packages/crypto/src/index.tsRepository: ancore-org/ancore
Length of output: 173
Design specifies signMessage but implementation exports signTransaction.
Line 96 in the Expected Submodules table lists signMessage as a public export from signing.ts, but the actual implementation exports signTransaction instead. This discrepancy should be reconciled — either:
- Update
signing.tsto exportsignMessage(or both) - Update the design doc to reflect
signTransaction
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.kiro/specs/crypto-utilities-package/design.md around lines 94 - 101, The
design lists signing.ts exporting signMessage but the implementation currently
exports signTransaction; reconcile by adding or renaming so the API matches the
design: update signing.ts to export a signMessage function (and optionally
export signTransaction as an alias) and update index.ts to re-export signMessage
(or change the design doc if you prefer signTransaction), ensuring both the
module export names (signing.ts: signMessage, signTransaction) and the public
re-exports in index.ts/CRYPTO_VERSION reflect the chosen name consistently.
| const EXPECTED_EXPORTS = [ | ||
| 'CRYPTO_VERSION', | ||
| // signing | ||
| 'verifySignature', | ||
| 'signMessage', | ||
| // hashing | ||
| 'sha256', | ||
| 'sha512', | ||
| 'hmac', | ||
| // keys | ||
| 'deriveKeyPair', | ||
| 'publicKeyFromSecret', | ||
| // mnemonic | ||
| 'generateMnemonic', | ||
| 'mnemonicToSeed', | ||
| // encoding | ||
| 'toHex', | ||
| 'fromHex', | ||
| 'toBase64', | ||
| 'fromBase64', | ||
| ] as const; |
There was a problem hiding this comment.
Test will fail: EXPECTED_EXPORTS does not match actual barrel exports.
The EXPECTED_EXPORTS list expects 15 symbols, but index.ts currently exports only 6 different symbols (see review comment on index.ts). This test will fail at both:
- Line 47:
CryptoAPI[symbol]will beundefinedfor missing symbols - Line 55:
actualKeyswon't matchexpectedKeys
Either update index.ts to export all expected symbols, or update EXPECTED_EXPORTS to reflect the actual public API surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/crypto/src/__tests__/smoke.test.ts` around lines 3 - 23, The smoke
test's EXPECTED_EXPORTS array lists many symbols (e.g., CRYPTO_VERSION,
verifySignature, signMessage, sha256, sha512, hmac, deriveKeyPair,
publicKeyFromSecret, generateMnemonic, mnemonicToSeed, toHex, fromHex, toBase64,
fromBase64) that don't match what index.ts actually exports, causing
CryptoAPI[symbol] to be undefined and actualKeys to differ; fix by either (A)
updating index.ts to re-export all the listed symbols so the public API matches
EXPECTED_EXPORTS (ensure functions like verifySignature, signMessage, sha256,
deriveKeyPair, generateMnemonic, toHex, etc. are exported from the barrel) or
(B) adjust EXPECTED_EXPORTS to exactly reflect the current exports in index.ts
so CryptoAPI lookups and actualKeys comparison succeed.
| export function sha256(input: HashInput): Uint8Array { | ||
| return nobleSha256(toBytes(input)); | ||
| } | ||
|
|
||
| /** Returns SHA-512 digest as Uint8Array */ | ||
| export function sha512(input: HashInput): Uint8Array { | ||
| return nobleSha512(toBytes(input)); | ||
| } | ||
|
|
||
| /** Returns HMAC-SHA256 as Uint8Array */ | ||
| export function hmac(key: HashInput, message: HashInput): Uint8Array { | ||
| return nobleHmac(nobleSha256, toBytes(key), toBytes(message)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
nl -ba packages/crypto/src/index.ts | sed -n '1,30p'
echo '---'
nl -ba packages/crypto/src/__tests__/smoke.test.ts | sed -n '1,40p'Repository: ancore-org/ancore
Length of output: 163
🏁 Script executed:
cat -n packages/crypto/src/index.ts
echo '---'
cat -n packages/crypto/src/__tests__/smoke.test.tsRepository: ancore-org/ancore
Length of output: 4960
Add hashing exports to the package barrel.
The hashing functions (sha256, sha512, hmac) are declared in the smoke test's EXPECTED_EXPORTS array but are not exported from packages/crypto/src/index.ts. The smoke test will fail at the "exports every symbol in the public API" and "has no undeclared exports" assertions because CryptoAPI['sha256'] etc. will be undefined.
Barrel fix
+export { sha256, sha512, hmac } from './hashing';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/crypto/src/hashing.ts` around lines 12 - 23, The package barrel is
missing exports for the hashing functions: add named exports for sha256, sha512,
and hmac to the crypto package's public index (export the symbols sha256,
sha512, hmac from packages/crypto/src/index.ts) so the smoke-test
EXPECTED_EXPORTS array can access CryptoAPI['sha256'], CryptoAPI['sha512'], and
CryptoAPI['hmac']; ensure you import them from their defining module (the file
that contains the sha256/sha512/hmac implementations) and re-export them from
the barrel.
- fromBase64: normalize padding before round-trip check - deriveKeyPair: accept 64-byte BIP39 seeds (uses first 32 bytes) - smoke test: use actual secret bytes in error message leak check
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/crypto/tsconfig.json (1)
7-9: Align TS module settings with base config and dual CJS/ESM build strategyThe
packages/crypto/tsconfig.jsonoverrides the base config's"module": "ESNext"and"moduleResolution": "bundler"with"module": "CommonJS"and"moduleResolution": "node". Since this package builds both CJS and ESM via tsup, aligning with the base settings reduces config drift and lets tsup handle format conversion cleanly.♻️ Suggested alignment
- "module": "CommonJS", + "module": "ESNext", "lib": ["ES2022", "DOM"], - "moduleResolution": "node", + "moduleResolution": "bundler",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crypto/tsconfig.json` around lines 7 - 9, The package tsconfig overrides "module" and "moduleResolution" causing drift; change the settings in packages/crypto's tsconfig to match the base config by setting "module": "ESNext" and "moduleResolution": "bundler" so tsup can produce both CJS and ESM outputs consistently; update the "module" and "moduleResolution" entries in the tsconfig JSON (referencing the "module" and "moduleResolution" keys) and remove any conflicting values so the base config behavior is preserved and tsup handles format conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/account-abstraction/tsconfig.json`:
- Around line 5-6: The tsconfig.json in the account-abstraction package sets
"composite": false but is referenced by packages/core-sdk, so change the
TypeScript project config to enable project references by setting "composite":
true (and keep "declarationDir": "./dist" or adjust as needed); update the
tsconfig's "composite" property to true so tsc --build can include this package
in the dependency graph and produce .d.ts outputs for the referencing project.
In `@packages/crypto/tsconfig.json`:
- Line 11: The tsconfig for this referenced project sets "composite": false
which breaks TypeScript project references; update the project's tsconfig.json
to set "composite": true and also enable "declaration": true (composite mode
requires declarations), and verify any other composite-related settings (like
"declarationMap" if needed) are adjusted so tsc --build and the root project
reference succeed.
In `@packages/types/tsconfig.json`:
- Around line 5-6: The package tsconfig currently sets "composite": false which
breaks the TypeScript build graph for referenced projects (used by root tsconfig
and packages like core-sdk, crypto, stellar, account-abstraction); update the
tsconfig in the types package to set "composite": true so it can participate in
tsc --build, ensure "declaration" and "declarationDir" are compatible with
composite builds (keep "declarationDir": "./dist" or adjust as needed) and
verify the referenced projects' tsconfigs expect a composite project (identify
the tsconfig JSON that contains the "composite" key and flip it to true).
---
Nitpick comments:
In `@packages/crypto/tsconfig.json`:
- Around line 7-9: The package tsconfig overrides "module" and
"moduleResolution" causing drift; change the settings in packages/crypto's
tsconfig to match the base config by setting "module": "ESNext" and
"moduleResolution": "bundler" so tsup can produce both CJS and ESM outputs
consistently; update the "module" and "moduleResolution" entries in the tsconfig
JSON (referencing the "module" and "moduleResolution" keys) and remove any
conflicting values so the base config behavior is preserved and tsup handles
format conversion.
🪄 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: cb84d7a7-98d5-43e7-89e3-2f097cb6c082
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/account-abstraction/tsconfig.jsonpackages/crypto/package.jsonpackages/crypto/src/__tests__/smoke.test.tspackages/crypto/src/encoding.tspackages/crypto/src/keys.tspackages/crypto/tsconfig.jsonpackages/stellar/tsconfig.jsonpackages/types/tsconfig.json
💤 Files with no reviewable changes (1)
- packages/stellar/tsconfig.json
✅ Files skipped from review due to trivial changes (2)
- packages/crypto/package.json
- packages/crypto/src/tests/smoke.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/crypto/src/encoding.ts
- packages/crypto/src/keys.ts
| "composite": false, | ||
| "declarationDir": "./dist" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the account-abstraction tsconfig.json
cat -n packages/account-abstraction/tsconfig.jsonRepository: ancore-org/ancore
Length of output: 363
🏁 Script executed:
# Then check the core-sdk tsconfig.json
cat -n packages/core-sdk/tsconfig.jsonRepository: ancore-org/ancore
Length of output: 481
🏁 Script executed:
# Search for any references to account-abstraction in all tsconfig.json files
rg "account-abstraction" --type json -A 2 -B 2Repository: ancore-org/ancore
Length of output: 2358
composite: false incompatible with project reference from core-sdk
Line 5 sets "composite": false, but packages/core-sdk/tsconfig.json references this package (line 9). When a package is referenced via the references array, it must have "composite": true for tsc --build to properly manage the compilation dependency graph. This will cause build failures or incorrect incremental compilation behavior.
Suggested fix
"compilerOptions": {
"outDir": "./dist",
- "composite": false,
+ "composite": true,
"declarationDir": "./dist"
},📝 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.
| "composite": false, | |
| "declarationDir": "./dist" | |
| "composite": true, | |
| "declarationDir": "./dist" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/account-abstraction/tsconfig.json` around lines 5 - 6, The
tsconfig.json in the account-abstraction package sets "composite": false but is
referenced by packages/core-sdk, so change the TypeScript project config to
enable project references by setting "composite": true (and keep
"declarationDir": "./dist" or adjust as needed); update the tsconfig's
"composite" property to true so tsc --build can include this package in the
dependency graph and produce .d.ts outputs for the referencing project.
| "lib": ["ES2022", "DOM"], | ||
| "moduleResolution": "node", | ||
| "types": ["node", "jest"], | ||
| "composite": false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -maxdepth 2 -name "tsconfig.json" -type f | head -20Repository: ancore-org/ancore
Length of output: 76
🏁 Script executed:
cat -n tsconfig.jsonRepository: ancore-org/ancore
Length of output: 360
🏁 Script executed:
cat -n packages/crypto/tsconfig.jsonRepository: ancore-org/ancore
Length of output: 537
🌐 Web query:
TypeScript composite project references incompatible composite false
💡 Result:
With TypeScript project references, the referenced project must be a composite project. If you set "composite": false (or omit it) in the referenced project’s tsconfig, TypeScript will report it as incompatible (often as TS6306: Referenced project must have setting "composite": true). TypeScript requires this so it can reliably locate the referenced project’s build outputs (notably .d.ts). [1]
Fix
In every referenced project (the dependency), enable composite (which also forces declaration: true):
// packages/lib/tsconfig.json
{
"compilerOptions": {
"composite": true,
"declaration": true,
"outDir": "dist"
}
}Then in the consuming project:
// packages/app/tsconfig.json
{
"references": [{ "path": "../lib" }]
}Build with build mode so references are built in the right order:
tsc -bTypeScript’s handbook explicitly states that referenced projects must have "composite": true, and that enabling composite requires declaration output. [1] (This is also the design described when project references were introduced.) [2]
Sources: TypeScript Handbook: Project References [1]; TypeScript blog (TypeScript 3.0 announcement: project references + composite) [2].
composite: false is incompatible with root project references
Line 11 has "composite": false, but the root tsconfig.json references this project. TypeScript project references require "composite": true in referenced projects and will fail with tsc --build (error TS6306: "Referenced project must have setting 'composite': true").
Change to "composite": true and ensure declaration: true is set (composite mode requires it).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/crypto/tsconfig.json` at line 11, The tsconfig for this referenced
project sets "composite": false which breaks TypeScript project references;
update the project's tsconfig.json to set "composite": true and also enable
"declaration": true (composite mode requires declarations), and verify any other
composite-related settings (like "declarationMap" if needed) are adjusted so tsc
--build and the root project reference succeed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/crypto/src/key-derivation.ts (1)
57-75: Consider caching the seed to avoid repeated PBKDF2 derivation.Each loop iteration calls
deriveKeypairFromMnemonic, which re-derives the seed via PBKDF2 (2048 iterations). Since the seed depends only on the mnemonic (not the index), computing it once and reusing it would significantly improve performance when deriving multiple keypairs.♻️ Proposed optimization
export function deriveMultipleKeypairsFromMnemonic( mnemonic: string, count: number, startIndex: number = 0 ): Keypair[] { + if (!validateMnemonic(mnemonic)) { + throw new Error('Invalid mnemonic phrase'); + } + if (count <= 0 || !Number.isInteger(count)) { throw new Error('Count must be a positive integer'); } if (startIndex < 0 || !Number.isInteger(startIndex)) { throw new Error('Start index must be a non-negative integer'); } + const seed = mnemonicToSeedSync(mnemonic); + const seedHex = Array.from(seed) + .map((b) => b.toString(16).padStart(2, '0')) + .join(''); + const keypairs: Keypair[] = []; for (let i = 0; i < count; i++) { - keypairs.push(deriveKeypairFromMnemonic(mnemonic, startIndex + i)); + const path = `m/44'/148'/${startIndex + i}'`; + const derivedKey = ed25519HdKey.derivePath(path, seedHex); + keypairs.push(Keypair.fromRawEd25519Seed(derivedKey.key)); } return keypairs; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/crypto/src/key-derivation.ts` around lines 57 - 75, The loop in deriveMultipleKeypairsFromMnemonic repeatedly calls deriveKeypairFromMnemonic which re-runs the PBKDF2 seed derivation each time; compute the seed once and reuse it. Modify or add a helper (e.g., deriveSeedFromMnemonic) to run the PBKDF2 once outside the loop, then either add an overload/optional parameter to deriveKeypairFromMnemonic to accept the precomputed seed or create a new deriveKeypairFromSeed(mnemonicSeed, index) and call that inside deriveMultipleKeypairsFromMnemonic so the PBKDF2 step is removed from the per-index loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/crypto/src/key-derivation.ts`:
- Around line 57-75: The loop in deriveMultipleKeypairsFromMnemonic repeatedly
calls deriveKeypairFromMnemonic which re-runs the PBKDF2 seed derivation each
time; compute the seed once and reuse it. Modify or add a helper (e.g.,
deriveSeedFromMnemonic) to run the PBKDF2 once outside the loop, then either add
an overload/optional parameter to deriveKeypairFromMnemonic to accept the
precomputed seed or create a new deriveKeypairFromSeed(mnemonicSeed, index) and
call that inside deriveMultipleKeypairsFromMnemonic so the PBKDF2 step is
removed from the per-index loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a125f06-03e9-447c-8943-0977cec62b20
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/crypto/eslint.config.cjspackages/crypto/src/key-derivation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/crypto/eslint.config.cjs
|
All CodeRabbit feedback has been addressed. You now merge the PR |
1 participant
feat(crypto): implement @ancore/crypto package integration layer
Description
Wires together all cryptographic submodules and exposes a clean, stable public API from a single barrel export in
packages/crypto/src/index.ts.Type of Change
Changes
Core implementation:
index.ts— barrel re-export for all 14 public symbols +CRYPTO_VERSIONsigning.ts— addedsignMessagealongside existingverifySignaturehashing.ts—sha256,sha512,hmacvia@noble/hasheskeys.ts—deriveKeyPair,publicKeyFromSecretvia Stellar SDKmnemonic.ts—generateMnemonic,mnemonicToSeedviabip39encoding.ts—toHex,fromHex,toBase64,fromBase64smoke.test.ts— verifies all exports defined, no console leaks, idempotent resolution, safe error messagesPre-existing fixes (unblocked the build/lint pipeline):
composite: trueincore-sdktsconfig breaking DTS buildnetworkPassphrasefield inaccount-transaction-builder.tsanytypes inaccount-contract.tsui-kiteslint configeslint --fixfrom lint-staged (no root eslint config)Testing
Security Considerations
No secret material is logged or exposed. Error messages use safe placeholders only (
"invalid key","invalid mnemonic phrase").Checklist
Closes #51
Summary by CodeRabbit
New Features
Tests
Documentation
Chores