Open
Conversation
Contributor
Summary by CodeRabbit
WalkthroughAdds a pluggable confirmation strategy (blockhash vs transactionStatus), dispatches send/confirm flows accordingly (including batched send+confirm and polling-based transaction-status confirmation), switches config uploads to use transactionStatus, refactors uploader to batch processing with stricter retries, and updates CLI progress/tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant SendConfirm as umiSendAllTransactionsAndConfirm
participant Batch as sendAndConfirmInBatches
participant RPC as RPC/Blockchain
participant Poller as confirmAllByTransactionStatus
Client->>SendConfirm: send(transactions, sendOptions{confirmationStrategy: transactionStatus})
SendConfirm->>Batch: route to batched sender
loop per batch
Batch->>RPC: send batch (commitment: processed)
RPC-->>Batch: signatures[]
Batch->>Poller: start polling signatures[]
loop poll rounds (up to MAX_POLL_ROUNDS)
Poller->>RPC: getSignatureStatuses(signatures[])
alt status observed
RPC-->>Poller: statuses (confirmed/unconfirmed)
Poller->>SendConfirm: onProgress/update per signature
else continue polling
end
end
Poller-->>Batch: confirmation results (confirmed or timed out)
Batch->>Client: progress "Processed X/Y..." and partial results
end
SendConfirm-->>Client: aggregated results with confirmations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/umi/confirmAllTransactions.ts`:
- Around line 75-91: The loop in confirmAllTransactions.ts that sequentially
awaits umi.rpc.getTransaction for each pending entry should be parallelized to
avoid slow rounds: gather the pending entries (index, signature) into an array,
call Promise.allSettled (or Promise.all) on mapped calls to
umi.rpc.getTransaction, then iterate the settled results to update
results[index], delete from pending, and call onProgress(index, results[index])
for those that resolved; for rejected or failed RPCs simply leave the entry in
pending to retry next round. Locate the code block in confirmAllTransactions
(the for (const [index, signature] of pending) { ... } section) and replace it
with a batched/parallelized approach that preserves existing error handling (set
confirmed false and error when txResult.meta.err, confirmed true when ok) and
does not treat RPC errors as fatal.
- Around line 72-73: The loop currently sleeps before the first poll, causing an
unnecessary initial delay; change the polling order in confirmAllTransactions
(the for loop using MAX_POLL_ROUNDS, POLL_INTERVAL and pending) to poll
immediately then sleep between subsequent rounds. Concretely, call the poll
logic at the top of the loop, and only await new Promise(resolve =>
setTimeout(resolve, POLL_INTERVAL)) at the end of the iteration (or guard the
sleep with if (round < MAX_POLL_ROUNDS - 1 && pending.size > 0)) so the first
round runs without the initial delay.
- Around line 88-90: The empty catch in confirmAllTransactions.ts swallows RPC
exceptions; change the catch block to capture the error (e.g., catch (err)) and
log it (using the existing logger or console.debug) with context like
"confirmAllTransactions RPC error" and the error object so transient failures
are visible during debugging; optionally gate the debug log behind a
non-production or verbose flag so production noise is avoided.
In `@src/lib/umi/sendAllTransactionsAndConfirm.ts`:
- Around line 75-99: The local variable processedCount in
sendAllTransactionsAndConfirm is unused; remove its declaration and the
processedCount += batch.length update (or alternatively wire it into the
batchLabel/spinner logic if you intended to track progress). Specifically,
delete the let processedCount = 0 line and the processedCount += batch.length
statement inside the for-loop (or replace usages of transactions.length in
batchLabel/spinner with processedCount where appropriate) so there are no dead
variables left in the sendAllTransactionsAndConfirm function.
In `@src/lib/umi/sendOptions.ts`:
- Line 12: The field declaration confirmationStrategy?: ConfirmationStrategy |
undefined is redundant because the `?:` already allows undefined; remove the `|
undefined` so the property reads confirmationStrategy?: ConfirmationStrategy in
the sendOptions type/interface to clean up the style (look for the
confirmationStrategy field in src/lib/umi/sendOptions.ts).
In `@src/lib/uploader/uploadFiles.ts`:
- Line 45: Replace the manual split('/') filename extraction with the repo's
file helper: change the logic that computes firstFile from
filePaths[batchStart].split('/') to use the utility in src/lib/file.ts (or at
minimum path.basename) so both occurrences (the one at firstFile and the similar
one at line 62) use the same helper; update references in uploadFiles.ts (the
firstFile variable calculation and the similar code at the second spot) to call
the shared filename function (or path.basename) instead of split('/') to ensure
consistent filename normalization.
- Around line 57-67: The progress callback currently emits a raw globalIndex
from uploadFiles.ts (in the loop that maps batch results) which breaks semantics
and UI (renders as percent) and hides progress until a batch finishes; change
the onProgress call to provide an explicit object like { completed: globalIndex
+ 1, total: filePaths.length } (or equivalent completed count and total) instead
of a single number, update the uploadResults mapping code in uploadFiles.ts to
use the new callback signature, and apply corresponding updates to the two
callers referenced (the renderer in src/commands/toolbox/storage/upload.ts and
the caller in src/lib/cm/uploadItems.ts) so they accept and render the {
completed, total } shape; keep the same call site timing (after each per-file
assignment) so progress remains incremental despite awaiting
umi.uploader.upload(batchFiles).
- Around line 26-33: The batching loop still holds the entire files array in
memory (so peak memory stays high); change the loop to slice from the original
filePaths per batch instead of using a pre-materialized files array—inside the
for loop compute batchFiles = filePaths.slice(batchStart, batchEnd) and then
map/transform those paths into the upload items and fill uploadResults
(UploadFileResult[]) as you process each batch, keeping BATCH_SIZE memory bounds
and avoiding full-materialization of files before the loop.
- Around line 38-53: The current batch retry loop around umi.uploader.upload can
re-submit already-persisted files on partial failures; change uploadFiles.ts so
upload retries are per-file (not per-batch): call umi.uploader.upload for each
filePath (or use a provider method that returns per-file results) and maintain a
map of filePath -> uri/failed state, retry only files marked failed up to
MAX_RETRIES, using retryCount per file; use the existing symbols
batchFiles/batchUris/filePaths/batchStart/MAX_RETRIES to locate the loop and
replace the batch retry logic with per-file retry/backoff and update the final
URI mapping to come from the per-file results.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10babcf5-eceb-4549-a651-b5067b91b28f
📒 Files selected for processing (7)
src/lib/cm/insertItems.tssrc/lib/umi/confirmAllTransactions.tssrc/lib/umi/sendAllTransactionsAndConfirm.tssrc/lib/umi/sendOptions.tssrc/lib/uploader/uploadFiles.tstest/commands/cm/cm.full.test.tstest/commands/cm/cm.insert.test.ts
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)
src/lib/umi/confirmAllTransactions.ts (1)
22-26:⚠️ Potential issue | 🔴 CriticalBug: Missing
confirmations.push()for transactions without signatures.When a transaction has no signature, the function calls
onProgressbut doesn't push a result to theconfirmationsarray. This causesconfirmations.lengthto be less thantransactions.length, breaking the index correspondence expected by callers (e.g.,sendAllThenConfirmAllat line 60 doesconfirmations[index]).🐛 Proposed fix
for (const transaction of transactions) { if (!transaction?.signature) { - onProgress && onProgress(index, { confirmed: false, error: transaction?.err || 'transaction has no signature' }) + const result = { confirmed: false, error: transaction?.err || 'transaction has no signature' } + confirmations.push(result) + onProgress && onProgress(index, result) index++ continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/umi/confirmAllTransactions.ts` around lines 22 - 26, In confirmAllTransactions, when a transaction lacks a signature the code calls onProgress but fails to push a corresponding result into the confirmations array, breaking the index alignment used by callers like sendAllThenConfirmAll; update the branch that checks transaction?.signature to push a confirmation object (e.g., { confirmed: false, error: transaction?.err || 'transaction has no signature' }) into confirmations at the current index before incrementing index and continuing so confirmations.length always matches transactions.length.
♻️ Duplicate comments (2)
src/lib/uploader/uploadFiles.ts (2)
35-50:⚠️ Potential issue | 🟠 MajorBatch-level retries can re-upload already persisted files after partial failures.
Retrying the whole batch on Line 37 after an error can duplicate uploads and spend extra funds if the provider persists a subset before throwing. Prefer retrying only failed items (or idempotency checks) instead of resubmitting the full batch payload.
In the currently used `@metaplex-foundation/umi-uploader-irys` version for metaplex-foundation/cli, does `uploader.upload(files)` guarantee all-or-nothing atomicity, or can a subset of files be persisted before an error is thrown?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/uploader/uploadFiles.ts` around lines 35 - 50, The current batch retry loop around umi.uploader.upload(batchFiles) can re-submit already-persisted files; instead change the retry strategy so you only retry failed items: call umi.uploader.upload on each file (or a per-file upload API) or, if umi.uploader.upload returns a partial-success result, inspect its response to determine which files succeeded and only requeue the failed ones for subsequent retries; update the logic around batchFiles, batchUris, retryCount and MAX_RETRIES (and the error path that throws using batchStart / firstFile) to perform per-file retries with exponential backoff and deduplication (or include idempotency keys) until all items in the original batch succeed or MAX_RETRIES is exceeded.
1-2: 🛠️ Refactor suggestion | 🟠 MajorUse
src/lib/file.tshelpers for file operations in this module.Line 25 (
fs.readFileSync) and Line 27/42/59 (path.basename) bypass the repository’s file utility requirement forsrc/**/*.ts. Please switch these calls to the helper(s) fromsrc/lib/file.tsto keep path normalization and file I/O behavior consistent across commands.As per coding guidelines,
src/**/*.ts: Use utilities in src/lib/file.ts for file handling operations.Also applies to: 24-29, 42-42, 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/uploader/uploadFiles.ts` around lines 1 - 2, In uploadFiles.ts replace direct node fs/path usage with the repository file helpers: import the file I/O and path utilities from src/lib/file.ts and use them instead of fs.readFileSync and path.basename; specifically, swap the fs.readFileSync call and the three path.basename usages in this module to the corresponding read (async or sync) and basename/normalize helpers exported by src/lib/file.ts so path normalization and I/O follow repository conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/umi/sendAllTransactionsAndConfirm.ts`:
- Around line 82-88: The batch-level progress callbacks in
sendAllTransactionsAndConfirm are not forwarded to per-batch operations; update
sendAllTransactionsAndConfirm to pass sendOptions.onSendProgress into
umiSendAllTransactions (e.g., umiSendAllTransactions(umi, batch, { ...,
onSendProgress })) and sendOptions.onConfirmProgress into confirmAllTransactions
(e.g., confirmAllTransactions(umi, sent, { ..., onConfirmProgress })) so any
per-transaction progress callbacks provided on sendOptions are invoked; ensure
you merge these keys with the existing sendOptions and keep the batch-level
spinner behavior unchanged.
---
Outside diff comments:
In `@src/lib/umi/confirmAllTransactions.ts`:
- Around line 22-26: In confirmAllTransactions, when a transaction lacks a
signature the code calls onProgress but fails to push a corresponding result
into the confirmations array, breaking the index alignment used by callers like
sendAllThenConfirmAll; update the branch that checks transaction?.signature to
push a confirmation object (e.g., { confirmed: false, error: transaction?.err ||
'transaction has no signature' }) into confirmations at the current index before
incrementing index and continuing so confirmations.length always matches
transactions.length.
---
Duplicate comments:
In `@src/lib/uploader/uploadFiles.ts`:
- Around line 35-50: The current batch retry loop around
umi.uploader.upload(batchFiles) can re-submit already-persisted files; instead
change the retry strategy so you only retry failed items: call
umi.uploader.upload on each file (or a per-file upload API) or, if
umi.uploader.upload returns a partial-success result, inspect its response to
determine which files succeeded and only requeue the failed ones for subsequent
retries; update the logic around batchFiles, batchUris, retryCount and
MAX_RETRIES (and the error path that throws using batchStart / firstFile) to
perform per-file retries with exponential backoff and deduplication (or include
idempotency keys) until all items in the original batch succeed or MAX_RETRIES
is exceeded.
- Around line 1-2: In uploadFiles.ts replace direct node fs/path usage with the
repository file helpers: import the file I/O and path utilities from
src/lib/file.ts and use them instead of fs.readFileSync and path.basename;
specifically, swap the fs.readFileSync call and the three path.basename usages
in this module to the corresponding read (async or sync) and basename/normalize
helpers exported by src/lib/file.ts so path normalization and I/O follow
repository conventions.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ffdb952b-a9bf-4828-b27f-92f9f1e454a4
📒 Files selected for processing (5)
src/commands/toolbox/storage/upload.tssrc/lib/umi/confirmAllTransactions.tssrc/lib/umi/sendAllTransactionsAndConfirm.tssrc/lib/umi/sendOptions.tssrc/lib/uploader/uploadFiles.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/umi/confirmAllTransactions.ts (1)
18-25:⚠️ Potential issue | 🟠 MajorKeep blockhash confirmations index-aligned with
transactions.This branch reports the failure via
onProgress, but it never writes a placeholder intoconfirmations.src/lib/umi/sendAllTransactionsAndConfirm.ts:57-62andsrc/lib/umi/sendAllTransactionsAndConfirm.ts:90-95later pair confirmation results back to the original transaction list by index, so the first missing signature shifts every subsequent result onto the wrong transaction.♻️ Preserve one result per input transaction
- let confirmations: UmiTransactionConfirmationResult[] = [] + const confirmations: UmiTransactionConfirmationResult[] = [] @@ for (const transaction of transactions) { if (!transaction?.signature) { - onProgress && onProgress(index, { confirmed: false, error: transaction?.err || 'transaction has no signature' }) + const res = { confirmed: false, error: transaction?.err || 'transaction has no signature' } + confirmations.push(res) + onProgress?.(index, res) index++ continue } @@ - onProgress && onProgress(index, res) + onProgress?.(index, res) index++ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/umi/confirmAllTransactions.ts` around lines 18 - 25, The loop in confirmAllTransactions builds confirmations but skips pushing a placeholder when transaction has no signature, causing index misalignment; update the for-loop in confirmAllTransactions (the block handling !transaction?.signature) to push a corresponding UmiTransactionConfirmationResult placeholder into the confirmations array (e.g., { confirmed: false, error: transaction?.err || 'transaction has no signature' }) at the same index, while still calling onProgress(index, ...) and continuing, so confirmations remains one-to-one with transactions.
♻️ Duplicate comments (1)
src/lib/uploader/uploadFiles.ts (1)
29-32: 🛠️ Refactor suggestion | 🟠 MajorUse
src/lib/file.tshelpers for file reads/name extraction.The changed code performs file operations with direct
fs/pathcalls (Line 29, Line 31, Line 42, Line 51, Line 56). Please switch these to the repository file utilities for consistency and centralized file-path handling behavior.As per coding guidelines,
src/**/*.ts: "Use utilities in src/lib/file.ts for file handling operations".Also applies to: 42-43, 51-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/uploader/uploadFiles.ts` around lines 29 - 32, The code reads files and extracts names using direct fs/path calls (fs.readFileSync, path.basename) around createGenericFile and mime.getType; replace those with the centralized helpers exported from src/lib/file.ts (use the file read helper to get the buffer and the filename extraction helper to get the base name) so the uploadFiles.ts logic uses those utilities instead of fs/path directly, and ensure you pass the resulting buffer and filename into createGenericFile and preserve the content-type tag derived from mime.getType.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/umi/confirmAllTransactions.ts`:
- Around line 71-93: Replace the per-signature umi.rpc.getTransaction polling
with batched signature-status polling using umi.rpc.getSignatureStatuses (pass
the array of pending signatures and the intended commitment) inside the
confirmAllTransactions loop: gather the pending signatures from the pending Map,
call umi.rpc.getSignatureStatuses(signatures, { searchTransactionHistory: true,
commitment: intendedCommitment }), iterate the returned statuses to set
results[index] = { confirmed: boolean, error: status.err ?? null } and delete
resolved indexes from pending, call onProgress for each resolved index, and keep
the existing retry/wait logic (POLL_INTERVAL / MAX_POLL_ROUNDS) while catching
RPC errors around the batched call; ensure you preserve use of symbols pending,
results, onProgress, POLL_INTERVAL and MAX_POLL_ROUNDS and thread the desired
commitment through the RPC call.
In `@src/lib/uploader/uploadFiles.ts`:
- Around line 49-53: The current throw in uploadFiles.ts (inside the retry/throw
block that checks retryCount > MAX_RETRIES and constructs a new Error) aborts
the whole directory upload; change this to make the behavior configurable by
adding a failFast flag to the uploadFiles function (or its options) so that when
failFast is true you keep the existing throw (using the retryCount > MAX_RETRIES
branch), and when failFast is false you instead return/record a failed upload
result as an empty URI (preserve the index mapping like uri || '') and continue
processing other files; update callers (e.g., the toolbox directory upload path)
to pass failFast: false and any CM path callers to pass failFast: true, and
ensure the error message still includes error instanceof Error ? error.message :
String(error) when failFast is true.
---
Outside diff comments:
In `@src/lib/umi/confirmAllTransactions.ts`:
- Around line 18-25: The loop in confirmAllTransactions builds confirmations but
skips pushing a placeholder when transaction has no signature, causing index
misalignment; update the for-loop in confirmAllTransactions (the block handling
!transaction?.signature) to push a corresponding
UmiTransactionConfirmationResult placeholder into the confirmations array (e.g.,
{ confirmed: false, error: transaction?.err || 'transaction has no signature' })
at the same index, while still calling onProgress(index, ...) and continuing, so
confirmations remains one-to-one with transactions.
---
Duplicate comments:
In `@src/lib/uploader/uploadFiles.ts`:
- Around line 29-32: The code reads files and extracts names using direct
fs/path calls (fs.readFileSync, path.basename) around createGenericFile and
mime.getType; replace those with the centralized helpers exported from
src/lib/file.ts (use the file read helper to get the buffer and the filename
extraction helper to get the base name) so the uploadFiles.ts logic uses those
utilities instead of fs/path directly, and ensure you pass the resulting buffer
and filename into createGenericFile and preserve the content-type tag derived
from mime.getType.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c18df0f0-7bca-4aef-ac77-a2623cc1d3c7
📒 Files selected for processing (2)
src/lib/umi/confirmAllTransactions.tssrc/lib/uploader/uploadFiles.ts
Replace per-signature getTransaction calls with a single batched getSignatureStatuses call per poll round. This is lighter on the RPC and eliminates rate limit retries on large inserts.
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)
src/lib/umi/confirmAllTransactions.ts (1)
22-26:⚠️ Potential issue | 🔴 CriticalMissing result entry for transactions without signatures causes array length mismatch.
When a transaction lacks a signature,
onProgressis called but no entry is pushed toconfirmations. This creates an array shorter than the input, breaking index alignment with the caller's transaction array. TheconfirmAllByTransactionStatusstrategy correctly pre-allocates results for all indices—this strategy should do the same.🐛 Proposed fix to ensure consistent array length
for (const transaction of transactions) { if (!transaction?.signature) { - onProgress && onProgress(index, { confirmed: false, error: transaction?.err || 'transaction has no signature' }) + const failedResult = { confirmed: false, error: transaction?.err || 'transaction has no signature' } + confirmations.push(failedResult) + onProgress && onProgress(index, failedResult) index++ continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/umi/confirmAllTransactions.ts` around lines 22 - 26, The loop in confirmAllTransactions.ts currently calls onProgress for transactions without a signature but does not push a corresponding entry into the confirmations array, causing length/ordering mismatches; update the branch that checks (!transaction?.signature) to push a result object into confirmations (e.g., { signature: null, confirmed: false, error: transaction?.err || 'transaction has no signature' }) before incrementing index and continuing so confirmAllTransactions (like confirmAllByTransactionStatus) always produces a result slot per input transaction and preserves index alignment for onProgress and callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/umi/confirmAllTransactions.ts`:
- Around line 76-79: The RPC call to umi.rpc.getSignatureStatuses is not
forwarding the caller's commitment level from sendOptions, causing potential
mismatches; update the call inside confirmAllTransactions (where signatures are
queried) to include the commitment from sendOptions (e.g.,
sendOptions?.commitment) in the options object along with
searchTransactionHistory: true so the RPC uses the same commitment as the
original sendOptions; ensure you reference umi.rpc.getSignatureStatuses, the
signatures variable, and sendOptions when making this change.
---
Outside diff comments:
In `@src/lib/umi/confirmAllTransactions.ts`:
- Around line 22-26: The loop in confirmAllTransactions.ts currently calls
onProgress for transactions without a signature but does not push a
corresponding entry into the confirmations array, causing length/ordering
mismatches; update the branch that checks (!transaction?.signature) to push a
result object into confirmations (e.g., { signature: null, confirmed: false,
error: transaction?.err || 'transaction has no signature' }) before incrementing
index and continuing so confirmAllTransactions (like
confirmAllByTransactionStatus) always produces a result slot per input
transaction and preserves index alignment for onProgress and callers.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c5cee805-387f-4477-9548-fc9143708095
📒 Files selected for processing (1)
src/lib/umi/confirmAllTransactions.ts
Contributor
Author
|
Thanks for the review @tonyboylehub , please have another look |
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
2 participants
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.
This PR fixes two things:
uploadFiles was uploading files individually in a sequential loop, each with its own price check and retry logic. This caused issues with too many concurrent upload. Refactored to upload in batches of 50, letting the Irys uploader handle concurrency internally. Also removed the manual balance check/fund logic (the uploader handles this) and simplified the retry logic to operate per-batch rather than per-file.
The candy machine insertItems was using the default blockhash confirmation strategy, which sends all transactions at once then confirms all at once. For large CM uploads this could overwhelm the RPC. Changed to transactionStatus strategy, which sends and confirms in batches of 10, spreading RPC load and using polling-based confirmation that's more reliable for bulk operations.
To test try to upload at least 1k assets, this was the number where i could reliably reproduce the issues.