Skip to content

Feat: use sdk's next version#3

Merged
silent-cipher merged 5 commits into
mainfrom
feat/next-sdk-version
Aug 28, 2025
Merged

Feat: use sdk's next version#3
silent-cipher merged 5 commits into
mainfrom
feat/next-sdk-version

Conversation

@silent-cipher
Copy link
Copy Markdown
Collaborator

Includes -

  • conversion to 'module'
  • random dataset from kaggle
  • automatic deposit and approvals to payment contracts for warm storage service
  • use of sdk's next version ( renames & commpV2 )

@FilOzzy FilOzzy added this to FOC Aug 21, 2025
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC Aug 21, 2025
@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting review in FOC Aug 22, 2025
@BigLep BigLep requested a review from Copilot August 22, 2025 03:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the codebase by upgrading to Synapse SDK's next version and converting the project to use ES modules. The changes include automatic wallet management for storage payments, integration with Kaggle datasets, and updates to deal creation workflow to accommodate SDK changes.

Key changes:

  • Conversion to ES modules with .js file extensions in imports
  • Upgrade to Synapse SDK v0.24.0-dev1 with updated API calls
  • Addition of automatic wallet funding and service approval functionality

Reviewed Changes

Copilot reviewed 38 out of 39 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
package.json Updates SDK version and enables ES module support
src/scheduler/scheduler.service.ts Adds wallet management logic for automatic deposits and approvals
src/deal/deal.service.ts Updates deal creation to use new SDK APIs and Kaggle data source
src/dataSource/dataSource.service.ts Implements Kaggle dataset fetching with fallback to local datasets
Multiple module files Converts all imports to use .js extensions for ES module compatibility
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/scheduler/scheduler.service.ts Outdated
}

return buffer;
throw new Error(`Failed to construct kaggle dataset download url for dataset ${dataset.ref}`);
Copy link

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

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

The error message should provide more context about why the URL construction failed, such as mentioning missing required fields or invalid dataset structure.

Suggested change
throw new Error(`Failed to construct kaggle dataset download url for dataset ${dataset.ref}`);
throw new Error(
`Failed to construct kaggle dataset download url. ` +
`Dataset details: ref=${JSON.stringify(dataset.ref)}, ` +
`url=${JSON.stringify(dataset.url)}, ` +
`urlNullable=${JSON.stringify(dataset.urlNullable)}, ` +
`hasUrl=${JSON.stringify(dataset.hasUrl)}. ` +
`Possible reasons: missing or invalid 'ref', 'url', or 'urlNullable' fields.`
);

Copilot uses AI. Check for mistakes.
Comment thread src/scheduler/scheduler.service.ts Outdated
@silent-cipher silent-cipher merged commit 1a98708 into main Aug 28, 2025
@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting review to 🎉 Done in FOC Aug 28, 2025
SgtPooki added a commit that referenced this pull request May 5, 2026
* docs(checks): close resolved TBDs in data-storage, events, README

Items previously marked TBD that are now implemented in code:

- data-storage.md assertions #3, #5, #6, #7 (pieceConfirmed, IPNI
  discoverability, retrievability, all-checks-gated) -> Yes.
- data-storage.md poll intervals: replace TBD_VARIABLE refs with the
  concrete sources (hardcoded POLLING_INTERVAL_MS = 2.5s for SP piece
  status, IPNI_VERIFICATION_POLLING_MS env var with 2s default for IPNI
  verification; doc previously claimed 5s).
- data-storage.md section 7 header drops TBD; intro disclaimer removed.
- data-storage.md "TBD Summary" rewritten as "Implementation History"
  with code references for inline retrieval, CID integrity, per-deal
  timeout (AbortController -> DealStatus.FAILED), gated status, status
  model, onPieceConfirmed, IPFS gateway retrieval, filecoin-pin CAR.
- events-and-metrics.md: pieceConfirmed -> Yes (pieceConfirmedOnChainMs
  histogram); ipfsRetrievalIntegrityChecked -> implemented inline via
  per-block sha256 verification in ipfs-block.strategy.ts (no discrete
  event); ipfsRetrievalFirstByte/LastByteReceived marked Partial since
  duration histograms exist but no discrete event; histogram-buckets
  TBD replaced with link to metrics-prometheus.module.ts.
- README.md: name the dataset-creation job (data-set-creation) and
  reference its config envs.

Still TBD (not changed in this commit): uploadToSpStart,
ipniVerificationStart, ipfsRetrievalStart events; jobs.md PR #263
lookahead-skip; PDP_SUBGRAPH_ENDPOINT production value.

* docs(checks): address review feedback on callback names and event states

- data-storage.md: rename Synapse callbacks to plural form
  (onPiecesAdded, onPiecesConfirmed) to match deal.service.ts.
- events-and-metrics.md: same rename in the event list. Clarify that
  dealCreated maps to DealStatus.DEAL_CREATED only after all gates pass
  (upload alone sets UPLOADED, not DEAL_CREATED).
- events-and-metrics.md: ipfsRetrievalIntegrityChecked downgraded from
  Yes to Partial since no discrete event is emitted (inline check
  only).
- events-and-metrics.md: Mermaid timeline now matches the table -
  ipfsRetrievalFirstByteReceived/LastByteReceived labelled as
  "Partial: histogram only", ipfsRetrievalIntegrityChecked labelled
  "Partial: inline check, no event".
- README.md: refer to the canonical pg-boss job type
  data_set_creation (underscore) so operators can map the doc to
  jobType values.

* docs(checks): fix unreadable Mermaid rect fill in event timeline

The 'Data Storage Only' rect used rgb(50, 50, 50), which renders as a
near-black block that hides the message labels and arrows inside it
(both on GitHub light/dark themes). Switch to a translucent
rgba(120, 120, 200, 0.15) so the highlight is visible without
obscuring content.

* docs(events): reframe Event List as timing markers, not emitted events

The 'events' in this doc are named anchors used to define metric Timer
Starts/Ends; dealbot does not necessarily emit each as a discrete
Prometheus event or log line. Add an explicit note up top so readers
don't expect every entry to map to an emitted event, and update rows
that were marked TBD/Partial purely because no discrete event is
emitted.

- uploadToSpStart -> Yes (anchor: deal.uploadStartTime in
  deal.service.ts:255).
- ipniVerificationStart -> Yes (anchor: ipniVerificationStartTime in
  ipni-verification.service.ts:63 - drives ipniVerifyMs).
- ipfsRetrievalStart -> Yes (anchor: retrieval startTime in
  retrieval-addons.service.ts:227; logs 'retrieval_started').
- ipfsRetrievalFirstByteReceived -> Yes (drives
  ipfsRetrievalFirstByteMs).
- ipfsRetrievalLastByteReceived -> Yes (drives
  ipfsRetrievalLastByteMs).
- ipfsRetrievalIntegrityChecked -> Yes (per-block sha256 in
  ipfs-block.strategy.ts; inline, no discrete event).
- Mermaid timeline: drop the (TBD) / (Partial: ...) annotations on
  these markers so the diagram and the table agree.

* docs(events): drop Implemented column from Event List

All rows are now Yes (each marker is anchored in code), so the column
adds no signal. Anchor details folded into the Source-of-truth column.
Intro note tightened.

* Update docs/checks/data-storage.md

Co-authored-by: Puspendra Mahariya <95584952+silent-cipher@users.noreply.github.com>

* Update docs/checks/README.md

Co-authored-by: Puspendra Mahariya <95584952+silent-cipher@users.noreply.github.com>

---------

Co-authored-by: Puspendra Mahariya <95584952+silent-cipher@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

5 participants