Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ✨ finish File System pallet benchmarking #307

Merged
merged 36 commits into from
Jan 14, 2025

Conversation

TDemeco
Copy link
Contributor

@TDemeco TDemeco commented Jan 6, 2025

This PR completes the benchmarking of the File System pallet.

  • Client changelog:
    • Added two RPC methods to the client (generateFileKeyProofBspConfirm and generateFileKeyProofMspAccept) which generate the file key proof needed by BSPs or MSPs to confirm or accept a storage request, using the special chunk challenges given by the runtime.
    • Added a check to the BSP submit proof task when reacting to the ProcessSubmitProofRequest event, to avoid submitting proofs if there're no challenges to respond to.
      • This should not happen in a real-world scenario since there should always be random challenges, but it was panicking when building with the runtime-benchmarks feature flag since that scenario does not have them, so I though might as well add a check there since it doesn't hurt and gives us a bit more flexibility with random challenges in the future.
  • Runtime changelog:
    • Modified the bsp_confirm_storing extrinsic of the File System pallet to use the already-existing struct FileKeyWithProof instead of a tuple of the file key and proof to make it tidier. Modified all interactions with bsp_confirm_storing to accomodate for this change.
    • Added missing calls to decrease_capacity_used for MSPs in both the delete_file and pending_file_deletion_request_submit_proof extrinsics of the File System pallet. Previously, used capacity of the MSP that held the deleted file wasn't being updated when it should after verifying the inclusion proof.
    • Created the benchmark_proofs_template.rs template file, which is used by the script that generates the proofs required to benchmark the File System pallet.
    • Incremented the StorageRequestTtl when compiling with the runtime-benchmarks feature flag to 2000 to avoid storage requests expiring when generating the required file key proofs in the aforementioned script.
    • Benchmarked the File System pallet completely, considering the input variables for each extrinsic and the worst-case scenario when checking for divergent paths in the weight calculation phase would be too costly. Details of each benchmark can be found in the corresponding benchmarking.rs file.
  • Scripts and integration tests changelog:
    • Developed the script to generate the required inclusion and non-inclusion forest proofs, file keys and file key proofs, bucket IDs and bucket roots for all required benchmark cases of the File System pallet.
    • Updated the newBucket method of the integration testing API to be able to receive the MSP ID that should store the new bucket.

@TDemeco TDemeco requested review from ffarall and snowmead January 6, 2025 04:27
Copy link
Contributor

@snowmead snowmead left a comment

Choose a reason for hiding this comment

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

beast

pallets/file-system/src/utils.rs Show resolved Hide resolved
// Finally, repeat this for each bucket.
// * For a BSP confirming storing, we need the same thing (a non-inclusion forest proof for 1 to 10 file keys that
// * the BSP wants to confirm) and the file key proofs for each one of the file keys.
// Since the BSP only requires one non-inclusion proof and 1 to 10 (MaxBatchBspConfirmStoring) file key proofs, we reutilise the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Since the BSP only requires one non-inclusion proof and 1 to 10 (MaxBatchBspConfirmStoring) file key proofs, we reutilise the
// Since the BSP only requires one non-inclusion proof and 1 to 10 (MaxBatchBspConfirmStoring) file key proofs, we reuse the

// * For a BSP confirming storing, we need the same thing (a non-inclusion forest proof for 1 to 10 file keys that
// * the BSP wants to confirm) and the file key proofs for each one of the file keys.
// Since the BSP only requires one non-inclusion proof and 1 to 10 (MaxBatchBspConfirmStoring) file key proofs, we reutilise the
// non-inclusion proof from the first bucket and generate the corresponding file key proofs since the challenges for the BSP are different.
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this means you'll set the BSP's root to be the root of the bucket, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep!

@TDemeco TDemeco merged commit 0047d10 into main Jan 14, 2025
25 checks passed
@TDemeco TDemeco deleted the feat/file-system-benchmark-script branch January 14, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants