chore: js, load, new call sigs, renames#2316
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (100)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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 |
[Warning] "c-token" in JSDoc of brand-new freeze-thaw.tsFile: New file uses "c-token" 8 times in JSDoc despite being added on the rename branch. Should say "light-token". |
[Warning] CHANGELOG incomplete -- only documents decompressInterface removalFile: Does not document the many other breaking changes:
Also uses "c-token" terminology in the new entry (line 6). |
[Warning] Test filename uses old naming: freeze-thaw-ctoken.test.tsFile: Should be |
[Warning] Misleading test name: "partial freeze" tests full freezeFile: Test named |
[Warning] Unused addressTreeInfo in 10 test casesFile:
|
[Warning] Unused imports in mint-to-light-token.test.tsFile:
|
[Warning] Magic byte offsets in tests instead of structured deserializationFiles:
Using |
[Warning] Unified index missing freeze/thaw exportsFile:
|
[Warning] Incomplete TLV extension parserFile:
const SIZES: Record<number, number | undefined> = {
29: 8,
30: 1,
31: 17,
};
const size = SIZES[disc];
if (size === undefined) return null; // silently fails for unknown extensions |
[Nit] Redundant assertNotFrozen double-callFile:
|
[Nit] Redundant spread propertyFile:
|
[Nit] Duplicate imports from same moduleFile: Two separate imports from import { getAssociatedLightTokenAddress, findMintAddress } from '../../src/v3/derivation'; |
[Nit] Unexplained 1-second sleep in testFile: Bare |
[Nit] Hardcoded program ID strings in testsFile: Uses literal |
66dfe6d to
1106d9e
Compare
test cov: offcurve, zero-amounts test cov: dupe hash failure, v1 reject at ixn boundary more test cov load, add freeze thaw, extend test cov add tests lint frozen handling more tests mark internals rm _tryfetchctokencoldbyaddress cleanups fmt
|
Issue 17/22:
|
|
Issue 18/22: Fake
if (mintInterface.mintContext?.cmintDecompressed) {
return '' as TransactionSignature;
}When the mint is already decompressed, callers get an empty string. Returning |
|
Issue 19/22:
|
|
Issue 20/22: Validity proof fetch pattern duplicated 7 times The same
Could be extracted into a shared helper. |
|
Issue 21/22: Silent
Line 352-354 in } catch {
// Ignore errors - associated token account may already exist
}Line 406-408 in } catch {
// Ignore all errors; for now there is no API-compatible way to selectively ignore...
}Both catch ALL errors (network failures, insufficient funds, etc.), not just "account already exists". |
|
Issue 22/22: Both By contrast, const authorityWritable = maxTopUp !== undefined && !feePayer;When a separate fee payer handles top-ups, |
|
Naming:
|
|
"Account not found" is an expected condition, not an exceptional one. Using exceptions for control flow forces callers into try/catch for normal cases -- see Suggest changing |
|
Return type is |
|
All three functions (get-account-interface.ts lines 424, 450, 476) use the same pattern: if (!info || !info.owner.equals(PROGRAM_ID)) {
throw new Error('Not a PROGRAM_ID account');
}This makes two semantically different cases indistinguishable to the caller:
By contrast, SPL's |
|
Silent error swallowing makes RPC failures indistinguishable from empty results Four
No logging in any of these handlers. A network timeout, RPC permission error, and a genuine "no accounts found" all produce the same result. Callers cannot retry transient failures or detect RPC issues. Suggest at minimum logging caught errors, or propagating them with a flag so callers can distinguish "confirmed empty" from "fetch failed". |
No description provided.