fix: add delegate to packed accounts in decompress, chunk proofs#2284
Open
SwenSchaeferjohann wants to merge 19 commits intomainfrom
Open
fix: add delegate to packed accounts in decompress, chunk proofs#2284SwenSchaeferjohann wants to merge 19 commits intomainfrom
SwenSchaeferjohann wants to merge 19 commits intomainfrom
Conversation
…on-aware proof chunking
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (30)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReworks instruction-level c-token flows in Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant InstrCreator
participant BatchAggregator
participant RPC
rect rgba(100,150,200,0.5)
Note over Client,RPC: Batch-Based Transfer Flow
Client->>InstrCreator: createTransferInterfaceInstructions(...)
InstrCreator-->>Client: [batch1, batch2, ..., transferBatch]
Client->>BatchAggregator: sliceLast(batches)
BatchAggregator-->>Client: [loadBatches, transferBatch]
par Execute loads
Client->>RPC: sendAndConfirmTransaction(loadBatch1)
Client->>RPC: sendAndConfirmTransaction(loadBatch2)
end
Client->>RPC: sendAndConfirmTransaction(transferBatch)
RPC-->>Client: confirmed
end
sequenceDiagram
participant Client
participant MintService
participant InstrCreator
participant RPC
rect rgba(100,200,100,0.5)
Note over Client,RPC: Parallel Unwrap Flow
Client->>InstrCreator: createLoadAtaInstructions(...)
Client->>MintService: getMint(mintAddr)
MintService-->>Client: decimals
Client->>InstrCreator: createUnwrapInstruction(decimals)
par Parallel load RPCs
Client->>RPC: sendAndConfirmTransaction(loadBatch1)
Client->>RPC: sendAndConfirmTransaction(loadBatch2)
end
Client->>RPC: sendAndConfirmTransaction(unwrapTransaction)
RPC-->>Client: unwrap confirmed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
SwenSchaeferjohann
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ctoken_for_payments.md (1)
268-311:⚠️ Potential issue | 🟡 MinorFix the misleading "parallel" comment and surface the new decimals requirement.
The example has three issues worth addressing:
Misleading parallel execution: The comment says "in parallel, if any" but the code uses sequential
for-loop withawaiton each iteration. Either execute truly in parallel usingPromise.all()or correct the comment to reflect sequential execution.New
decimalsparameter is required: ThecreateUnwrapInstructionnow requires the mint's decimals (fortransfer_checkedvalidation). The example correctly fetches this viagetMint, but this change should be called out since it differs from previous API versions.Missing error handling: In production, both the load batches loop and the unwrap transaction should be wrapped in try-catch blocks with appropriate failure handling (retries, user notifications, etc.).
Example: Fix parallel execution or comment
For truly parallel execution:
-// Send load batches first (in parallel, if any), then unwrap tx -for (const batch of loadBatches) { - await sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [ - payer, - owner, - ]); -} +// Send load batches in parallel (if any), then unwrap tx +if (loadBatches.length > 0) { + await Promise.all( + loadBatches.map(batch => + sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [payer, owner]) + ) + ); +}Or, for sequential (fix comment):
-// Send load batches first (in parallel, if any), then unwrap tx +// Send load batches first (if any), then unwrap tx for (const batch of loadBatches) {
🤖 Fix all issues with AI agents
In `@ctoken_for_payments.md`:
- Around line 144-159: Add a cross-reference to the official
js/compressed-token/docs/payment-integration.md and a brief note next to the
createTransferInterfaceInstructions example: mention the hot/cold sender pattern
and the rule "For a hot sender or <=8 cold inputs, the result is a
single-element array", and instruct readers to run loadBatches (the rest
returned by sliceLast) in parallel and then submit transferBatch sequentially;
reference the symbols createTransferInterfaceInstructions, sliceLast,
loadBatches, and transferBatch and add a link to the payment-integration.md
which contains the full usage example and the table describing contents of each
transaction.
Comment on lines
144
to
159
| createTransferInterfaceInstructions, | ||
| sliceLast, | ||
| } from "@lightprotocol/compressed-token/unified"; | ||
|
|
||
| const sourceAta = getAssociatedTokenAddressInterface(mint, owner.publicKey); | ||
| const destinationAta = getAssociatedTokenAddressInterface(mint, recipient); | ||
|
|
||
| const tx = new Transaction().add( | ||
| ...(await createLoadAtaInstructions( | ||
| rpc, | ||
| sourceAta, | ||
| owner.publicKey, | ||
| mint, | ||
| payer.publicKey | ||
| )), | ||
| createTransferInterfaceInstruction( | ||
| sourceAta, | ||
| destinationAta, | ||
| owner.publicKey, | ||
| amount | ||
| ) | ||
| const batches = await createTransferInterfaceInstructions( | ||
| rpc, | ||
| payer.publicKey, | ||
| mint, | ||
| amount, | ||
| owner.publicKey, | ||
| recipient | ||
| ); | ||
| const { rest: loadBatches, last: transferBatch } = sliceLast(batches); | ||
| // For hot sender: one batch. For cold: run loadBatches in parallel, then transferBatch | ||
| const tx = new Transaction().add(...transferBatch); | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its location
find . -name "ctoken_for_payments.md" -type fRepository: Lightprotocol/light-protocol
Length of output: 96
🏁 Script executed:
# Search for createTransferInterfaceInstructions export (plural)
rg -n "export.*createTransferInterfaceInstructions" --type=ts --type=jsRepository: Lightprotocol/light-protocol
Length of output: 298
🏁 Script executed:
# Search for sliceLast export
rg -n "export.*sliceLast" --type=ts --type=jsRepository: Lightprotocol/light-protocol
Length of output: 324
🏁 Script executed:
# Check for old singular API references
rg -n "createTransferInterfaceInstruction[^s]" --type=ts --type=js -g '!*.md'Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Find CLAUDE.md files for compressed token documentation guidance
find . -path "*/compressed-token/*" -name "CLAUDE.md" -o -path "*/compressed-token/*" -name "*.md" | head -20Repository: Lightprotocol/light-protocol
Length of output: 1176
🏁 Script executed:
# Look at the unified module structure
find . -path "*/compressed-token/unified*" -type f | grep -E "\.(ts|js|md)$"Repository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Get the full path and first 200 lines of ctoken_for_payments.md
find . -name "ctoken_for_payments.md" -exec wc -l {} \;Repository: Lightprotocol/light-protocol
Length of output: 100
🏁 Script executed:
# Check the directory containing ctoken_for_payments.md
find . -name "ctoken_for_payments.md" -exec dirname {} \;Repository: Lightprotocol/light-protocol
Length of output: 73
🏁 Script executed:
# Read the relevant section of the file (around lines 144-159)
sed -n '130,170p' ./ctoken_for_payments.md | cat -nRepository: Lightprotocol/light-protocol
Length of output: 1345
🏁 Script executed:
# Check for hot/cold sender pattern documentation elsewhere
rg -n "hot\s+sender|cold\s+sender" --type=md --type=tsRepository: Lightprotocol/light-protocol
Length of output: 1159
🏁 Script executed:
# Look for CLAUDE.md guidance in js/compressed-token
find ./js/compressed-token -name "CLAUDE.md" -o -name "*.md" | grep -i claudeRepository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check docs directory in js/compressed-token for standards
ls -la ./js/compressed-token/docs/ 2>/dev/null || echo "No docs dir"Repository: Lightprotocol/light-protocol
Length of output: 321
🏁 Script executed:
# Read payment-integration.md to see existing hot/cold sender documentation
cat ./js/compressed-token/docs/payment-integration.mdRepository: Lightprotocol/light-protocol
Length of output: 3120
🏁 Script executed:
# Check the JSDoc for createTransferInterfaceInstructions in transfer-interface.ts
sed -n '200,225p' ./js/compressed-token/src/v3/actions/transfer-interface.ts | cat -nRepository: Lightprotocol/light-protocol
Length of output: 1335
🏁 Script executed:
# Check if there are any references from ctoken_for_payments.md to payment-integration.md
rg -n "payment-integration" ./ctoken_for_payments.mdRepository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Check if ctoken_for_payments.md should be elsewhere or if this is the intended location
cat -n ./ctoken_for_payments.md | head -30Repository: Lightprotocol/light-protocol
Length of output: 865
🏁 Script executed:
# Check for any references to ctoken_for_payments.md in docs or READMEs
rg -n "ctoken_for_payments" --type=md --type=tsRepository: Lightprotocol/light-protocol
Length of output: 54
🏁 Script executed:
# Understand the file structure - is ctoken_for_payments.md a payment flow guide?
head -50 ./ctoken_for_payments.mdRepository: Lightprotocol/light-protocol
Length of output: 993
Reference the official payment-integration.md documentation for batching details.
The hot/cold sender pattern and batching guidance already exist in the official js/compressed-token/docs/payment-integration.md. Consider adding a link or cross-reference to that documentation, which explains:
- "For a hot sender or <=8 cold inputs, the result is a single-element array"
- Complete usage showing parallel load transactions followed by sequential transfer
- A table describing what each transaction contains
The APIs (createTransferInterfaceInstructions, sliceLast) are correct and properly exported from the unified module. The code example is accurate; it just needs to point readers to the comprehensive documentation that already covers the concepts in detail.
🤖 Prompt for AI Agents
In `@ctoken_for_payments.md` around lines 144 - 159, Add a cross-reference to the
official js/compressed-token/docs/payment-integration.md and a brief note next
to the createTransferInterfaceInstructions example: mention the hot/cold sender
pattern and the rule "For a hot sender or <=8 cold inputs, the result is a
single-element array", and instruct readers to run loadBatches (the rest
returned by sliceLast) in parallel and then submit transferBatch sequentially;
reference the symbols createTransferInterfaceInstructions, sliceLast,
loadBatches, and transferBatch and add a link to the payment-integration.md
which contains the full usage example and the table describing contents of each
transaction.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@ctoken_for_payments.md`:
- Around line 312-318: The comment says "Send load batches first (in
parallel...)" but the for-loop over loadBatches runs sequentially; update to
actually send in parallel by mapping loadBatches to an array of
sendAndConfirmTransaction promises using sendAndConfirmTransaction(rpc, new
Transaction().add(...batch), [payer, owner]) and await Promise.all(...) (or,
alternatively, change the comment to remove "in parallel" if sequential behavior
is intended). Ensure you reference loadBatches, sendAndConfirmTransaction, rpc,
Transaction, payer, and owner when making the change so the behavior and intent
remain clear.
Comment on lines
+312
to
+318
| // Send load batches first (in parallel, if any), then unwrap tx | ||
| for (const batch of loadBatches) { | ||
| await sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [ | ||
| payer, | ||
| owner, | ||
| ]); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fix mismatch: comment says parallel, code runs sequentially.
The text says “Send load batches first (in parallel, if any)” but the loop sends them sequentially. Either update the comment or send in parallel to match the guidance.
♻️ Suggested fix (parallelize loads)
-// Send load batches first (in parallel, if any), then unwrap tx
-for (const batch of loadBatches) {
- await sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [
- payer,
- owner,
- ]);
-}
+// Send load batches first (in parallel, if any), then unwrap tx
+await Promise.all(
+ loadBatches.map((batch) =>
+ sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [
+ payer,
+ owner,
+ ])
+ )
+);📝 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
| // Send load batches first (in parallel, if any), then unwrap tx | |
| for (const batch of loadBatches) { | |
| await sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [ | |
| payer, | |
| owner, | |
| ]); | |
| } | |
| // Send load batches first (in parallel, if any), then unwrap tx | |
| await Promise.all( | |
| loadBatches.map((batch) => | |
| sendAndConfirmTransaction(rpc, new Transaction().add(...batch), [ | |
| payer, | |
| owner, | |
| ]) | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In `@ctoken_for_payments.md` around lines 312 - 318, The comment says "Send load
batches first (in parallel...)" but the for-loop over loadBatches runs
sequentially; update to actually send in parallel by mapping loadBatches to an
array of sendAndConfirmTransaction promises using sendAndConfirmTransaction(rpc,
new Transaction().add(...batch), [payer, owner]) and await Promise.all(...) (or,
alternatively, change the comment to remove "in parallel" if sequential behavior
is intended). Ensure you reference loadBatches, sendAndConfirmTransaction, rpc,
Transaction, payer, and owner when making the change so the behavior and intent
remain clear.
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 by CodeRabbit
Documentation
Refactor
Breaking Change