Skip to content

Conversation

DarkLord017
Copy link

This pull request updates how raw piece sizes are calculated and improves clarity in proof fee calculations in the PDPVerifier contract. The main changes involve switching to a more accurate method for determining piece sizes and renaming variables for better readability.

FIxes #217

Piece size calculation improvements:

  • Changed the calculation of tempRawSizes to use Cids.pieceSize(padding, height) instead of multiplying leaf counts by 32, resulting in more accurate piece size determination.

Proof fee calculation clarity:

  • Renamed variables from rawSize to proofSize in calculateProofFee, calculateProofFeeForSize, and calculateAndBurnProofFee functions to clarify that the fee is based on the proof size in bytes, not the raw size.
  • Added comments to explain that challengeRange represents a leaf count and is converted to bytes by multiplying by 32.

@DarkLord017
Copy link
Author

Also fixes #221

@rjan90 rjan90 moved this to 🔎 Awaiting review in PDP Oct 15, 2025
@rjan90 rjan90 requested a review from rvagg October 15, 2025 09:42
@rjan90
Copy link
Contributor

rjan90 commented Oct 17, 2025

Hey @DarkLord017! Seems like there is a lint-check error that needs to be resolved here

Comment on lines 342 to 343
(uint256 padding, uint8 height,) = Cids.validateCommPv2(tempPieces[resultIndex]);
tempRawSizes[resultIndex] = Cids.pieceSize(padding, height);
Copy link
Contributor

Choose a reason for hiding this comment

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

@DarkLord017 let's just break this API, I don't see any current obvious users of this function so I don't there's a huge risk here. Remove uint256[] memory rawSizes from the return values. It's redundant, since as you're seeing, it's in the CID which we return.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that I added this in the first place, #177, so I'm to blame for the naming here. I take responsibility, but also give permission to break this! It never got consumed by the SDK which was the original intention, but with this fix we can start consuming it.

Copy link
Author

Choose a reason for hiding this comment

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

okay

@DarkLord017
Copy link
Author

please review @rjan90
@rvagg

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

Labels

None yet

Projects

Status: 🔎 Awaiting review

Development

Successfully merging this pull request may close these issues.

Fix & clarify all uses of "rawSize"

3 participants