Skip to content

fix: pieceStatus confuses piece ownership. Closes #296#655

Merged
hugomrdias merged 23 commits intomasterfrom
fix/piece-status-data-set
Mar 12, 2026
Merged

fix: pieceStatus confuses piece ownership. Closes #296#655
hugomrdias merged 23 commits intomasterfrom
fix/piece-status-data-set

Conversation

@juliangruber
Copy link
Copy Markdown
Member

@juliangruber juliangruber commented Mar 5, 2026

Fixes that pieceStatus() can say a piece was found when it was on the SP but not in the right data set.

Closes #296.

TODO:

  • Refactor to read more from the contract and less from SPs
  • Use contract state where possible
  • Add test

@github-project-automation github-project-automation bot moved this to 📌 Triage in FOC Mar 5, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 5, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
synapse-dev b11ef4f Commit Preview URL

Branch Preview URL
Mar 12 2026, 06:25 AM

@rjan90 rjan90 moved this from 📌 Triage to ⌨️ In Progress in FOC Mar 5, 2026
@rjan90 rjan90 added this to the M4.1: mainnet ready milestone Mar 5, 2026
@juliangruber juliangruber marked this pull request as ready for review March 6, 2026 16:43
Co-authored-by: Rod Vagg <rod@vagg.org>
@BigLep
Copy link
Copy Markdown
Contributor

BigLep commented Mar 10, 2026

What are the next steps here for landing?

@juliangruber
Copy link
Copy Markdown
Member Author

What are the next steps here for landing?

I need to remove SP calls, so that this just checks contract state, then request review again

@juliangruber juliangruber requested a review from rvagg March 10, 2026 07:52
@rjan90 rjan90 changed the title Fix pieceStatus confuses piece ownership. Closes #296 fix: pieceStatus confuses piece ownership. Closes #296 Mar 10, 2026
@juliangruber juliangruber requested a review from rvagg March 11, 2026 13:35
@BigLep
Copy link
Copy Markdown
Contributor

BigLep commented Mar 11, 2026

2026-03-11 standup: @hugomrdias said he'll look today.

// Process proof timing data if we have data set data and PDP config
if (pdpConfig != null) {
// Check if this PieceCID is in the data set
const pieceData = activePieces.pieces.find((piece) => piece.cid.equals(parsedPieceCID))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need to find again? already done in line 1131

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

good call, nice simplification

// Check if this PieceCID is in the data set
const pieceData = activePieces.pieces.find((piece) => piece.cid.equals(parsedPieceCID))

if (pieceData != null) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should always be != null here if we take my previous comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@github-project-automation github-project-automation bot moved this from ⌨️ In Progress to ✔️ Approved by reviewer in FOC Mar 12, 2026
@hugomrdias hugomrdias merged commit be76ad1 into master Mar 12, 2026
14 checks passed
@hugomrdias hugomrdias deleted the fix/piece-status-data-set branch March 12, 2026 14:16
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FOC Mar 12, 2026
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.

Bug: pieceStatus confuses piece ownership

5 participants