Skip to content

chore: rm decompressInterface. house keeping#2297

Open
SwenSchaeferjohann wants to merge 13 commits intomainfrom
swen/hardening-ts
Open

chore: rm decompressInterface. house keeping#2297
SwenSchaeferjohann wants to merge 13 commits intomainfrom
swen/hardening-ts

Conversation

@SwenSchaeferjohann
Copy link
Contributor

  • rm decompressInterface

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 18, 2026

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (59)
  • js/compressed-token/CHANGELOG.md is excluded by none and included by none
  • js/compressed-token/package.json is excluded by none and included by none
  • js/compressed-token/src/constants.ts is excluded by none and included by none
  • js/compressed-token/src/index.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/create-associated-ctoken.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/decompress-interface.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/decompress-mint.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/get-or-create-ata-interface.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/index.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/load-ata.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/mint-to-compressed.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/transfer-interface.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/unwrap.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/update-metadata.ts is excluded by none and included by none
  • js/compressed-token/src/v3/actions/update-mint.ts is excluded by none and included by none
  • js/compressed-token/src/v3/assert-v2-only.ts is excluded by none and included by none
  • js/compressed-token/src/v3/ata-utils.ts is excluded by none and included by none
  • js/compressed-token/src/v3/derivation.ts is excluded by none and included by none
  • js/compressed-token/src/v3/errors.ts is excluded by none and included by none
  • js/compressed-token/src/v3/get-account-interface.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/create-associated-ctoken.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/create-decompress-interface-instruction.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/create-load-accounts-params.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/create-mint.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/decompress-mint.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/freeze-thaw.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/index.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/load-ata.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/mint-to-compressed.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/transfer-interface.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/unwrap.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/update-metadata.ts is excluded by none and included by none
  • js/compressed-token/src/v3/instructions/update-mint.ts is excluded by none and included by none
  • js/compressed-token/src/v3/layout/index.ts is excluded by none and included by none
  • js/compressed-token/src/v3/layout/layout-mint-action.ts is excluded by none and included by none
  • js/compressed-token/src/v3/layout/layout-mint.ts is excluded by none and included by none
  • js/compressed-token/src/v3/layout/layout-transfer2.ts is excluded by none and included by none
  • js/compressed-token/src/v3/layout/serde.ts is excluded by none and included by none
  • js/compressed-token/src/v3/unified/index.ts is excluded by none and included by none
  • js/compressed-token/src/v3/utils/estimate-tx-size.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/compressible-load.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/decompress2.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/freeze-thaw-ctoken.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/get-account-interface.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/get-mint-interface.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/load-ata-freeze.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/load-ata-spl-t22.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/load-ata-standard.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/multi-cold-inputs-batching.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/payment-flows.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/transfer-interface.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/unwrap.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/v3-interface-migration.test.ts is excluded by none and included by none
  • js/compressed-token/tests/e2e/wrap.test.ts is excluded by none and included by none
  • js/compressed-token/tests/unit/decompress-interface-version.test.ts is excluded by none and included by none
  • js/compressed-token/tests/unit/delegate-merge-semantics.test.ts is excluded by none and included by none
  • js/compressed-token/tests/unit/estimate-tx-size.test.ts is excluded by none and included by none
  • js/compressed-token/tests/unit/layout-serde.test.ts is excluded by none and included by none
  • js/compressed-token/tests/unit/load-transfer-cu.test.ts is excluded by none and included by none

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch swen/hardening-ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@SwenSchaeferjohann SwenSchaeferjohann marked this pull request as ready for review February 19, 2026 00:54
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
* For load: use this owner for getAtaInterface; only sources the delegate
* can use are included. For transfer: see TransferOptions.owner.
*/
owner?: PublicKey;
Copy link
Contributor

@ananas-block ananas-block Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment and naming are not completely clear.
I think in the spl program this field would be called authority (either owner or delegate).
Do you need the owner to derive the ata pubkey?

Comment on lines 164 to 169
/**
* ATA owner when the signer is the delegate (not the owner).
* Required when transferring as delegate: pass the owner so the SDK
* can derive the source ATA and validate the signer is the account delegate.
*/
owner?: PublicKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@ananas-block
Copy link
Contributor

BUG: parseCompressedOnlyFromTlv dead code causes TLV misparse for unknown discriminators

create-decompress-interface-instruction.ts:62-69

const size = SIZES[disc] ?? 0;       // replaces undefined with 0
if (size === undefined) return null;  // DEAD CODE: size is never undefined after ?? 0
offset += size;                       // offset += 0, never advances

Any TLV discriminator not in {29, 30, 31} gets size=0. The offset never advances, so all subsequent extensions in the buffer are misparsed. A CompressedOnly extension after an unknown disc will never be found.

Fix: Remove ?? 0 so SIZES[disc] returns undefined for unknown types, making the existing guard effective.

@ananas-block
Copy link
Contributor

BUG: assertNotFrozen before delegate filter blocks valid delegate transfers

transfer-interface.ts:311

assertNotFrozen(senderInterface, 'transfer');  // checks ALL sources
// ...
senderInterface = filterInterfaceForAuthority(senderInterface, sender);  // filters to delegate's sources

_anyFrozen is true if ANY source is frozen, including sources not accessible to the delegate. If the account has a frozen SPL hot source and an unfrozen CTokenCold source delegated to the sender, the entire transfer is blocked even though the delegate has valid, unfrozen sources.

Fix: For the delegate path, move assertNotFrozen to after filterInterfaceForAuthority.

@ananas-block
Copy link
Contributor

BUG: filterInterfaceForAuthority empty-case returns stale derived fields

get-account-interface.ts:992-997

The empty-case spreads ...iface and only overrides _sources: [] and parsed.amount: BigInt(0). It retains the original _anyFrozen, _hasDelegate, _needsConsolidation flags. A consumer calling assertNotFrozen on this empty interface would still see _anyFrozen: true.

Fix: Reset derived fields in the empty return: _needsConsolidation: false, _hasDelegate: false, _anyFrozen: false.

@ananas-block
Copy link
Contributor

BUG: compressionIndex hardcoded to 0 for all accounts in buildInTlv

create-decompress-interface-instruction.ts:121

Every account gets compressionIndex: 0. On-chain, two CompressedOnly accounts both claiming index 0 trigger TokenError::DuplicateCompressionIndex. Works by accident because exactly one compression is built per batch currently. Would immediately break if multi-compression decompress is added.

Fix: Document the single-compression invariant with a comment. Use the loop index if multi-compression is ever needed.

@ananas-block
Copy link
Contributor

BUG: Silent signer fallback when authority not in packed accounts

create-decompress-interface-instruction.ts:466-471

When a delegate authority is not found in packedAccountIndices, the code silently falls back to ownerIndex. The transaction is built with the owner marked as signer instead of the delegate, causing an on-chain signature verification failure with no client-side diagnostic.

Fix: Throw an explicit error when authority is not owner and not found in packed accounts.

@ananas-block
Copy link
Contributor

BUG: Inconsistent null-guards for amount in unwrap

unwrap.ts:129,147

const unwrapAmount = amount != null ? BigInt(amount.toString()) : totalBalance;   // != null catches null+undefined
// ...
amount !== undefined ? unwrapAmount : undefined,  // !== undefined does NOT catch null

The two guards diverge for the null case. Also, amount=0 passes all guards and submits a zero-amount unwrap transaction.

Fix: Unify guards to amount != null on both lines. Add if (unwrapAmount === BigInt(0)) throw new Error(...).

@ananas-block
Copy link
Contributor

CONCERN: assertNotFrozen is a breaking semantic change for unwrap

unwrap.ts:121, get-account-interface.ts:78-87

The old behavior allowed partial unwrap from unfrozen sources when some sources were frozen. The new assertNotFrozen blocks the entire operation if ANY source is frozen. Users with mixed frozen/unfrozen accounts can no longer unwrap the unfrozen portion.

If intentional, consider updating the error message to provide recovery guidance (e.g., "Thaw all frozen accounts before unwrapping").

@ananas-block
Copy link
Contributor

CONCERN: Approve-style cold source check runs after RPC work

transfer-interface.ts:350-367

The hasApproveStyleCold check runs after _buildLoadBatches which fetches validity proofs via RPC. The data needed for this check (senderInterface._sources) is available before _buildLoadBatches. Moving the check earlier avoids wasted RPC round-trips for this error case.

@ananas-block
Copy link
Contributor

CONCERN: isCold reflects only the primary source

get-account-interface.ts:915

isCold: isColdSourceType(primarySource.type) reports false if the first source is hot, even when cold secondary sources exist. Callers using isCold to decide whether to skip loading could miss cold sources.

Consider using sources.some(src => isColdSourceType(src.type)) instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants