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

fix(storage-provider): concurrency & logic bugs in add_piece #737

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

jmg-duarte
Copy link
Collaborator

Description

Fixes a BIG logic bug in the find sector logic & fixes a concurrency bug/issue in the overall process for add piece.

Blocking #733

To test, apply the following change to start_sp.sh

diff --git a/examples/start_sp.sh b/examples/start_sp.sh
index eca6231d..b1ac1e31 100755
--- a/examples/start_sp.sh
+++ b/examples/start_sp.sh
@@ -31,16 +31,16 @@ wait
 # It's a test setup based on the local verifying keys, everyone can run those extrinsics currently.
 # Each of the keys is different, because the processes are running in parallel.
 # If they were running in parallel on the same account, they'd conflict with each other on the transaction nonce.
-RUST_LOG=debug target/release/storagext-cli --sr25519-key "//Charlie" storage-provider register "$PEER_ID" &
-RUST_LOG=debug target/release/storagext-cli --sr25519-key "//Alice" proofs set-porep-verifying-key @2KiB.porep.vk.scale &
-RUST_LOG=debug target/release/storagext-cli --sr25519-key "//Bob" proofs set-post-verifying-key @2KiB.post.vk.scale &
+RUST_LOG=debug target/release/storagext-cli --sr25519-key "//Charlie" storage-provider register --post-proof 8MiB "$PEER_ID" &
+RUST_LOG=debug target/release/storagext-cli --sr25519-key "//Alice" proofs set-porep-verifying-key @8MiB.porep.vk.scale &
+RUST_LOG=debug target/release/storagext-cli --sr25519-key "//Bob" proofs set-post-verifying-key @8MiB.post.vk.scale &
 wait
 
 echo '{
-    "seal_proof": "2KiB",
-    "post_proof": "2KiB",
-    "porep_parameters": "2KiB.porep.params",
-    "post_parameters": "2KiB.post.params",
+    "seal_proof": "8MiB",
+    "post_proof": "8MiB",
+    "porep_parameters": "8MiB.porep.params",
+    "post_parameters": "8MiB.post.params",
     "p2p_key": "@/tmp/private.pem",
     "rendezvous_point_address": "/ip4/127.0.0.1/tcp/62649",
     "sealing_configuration": {

And run examples/rpc_publish_multiple_sectors.sh with a file (for easier testing) around ~3MB

Checklist

  • Have you tested this solution?
  • Did you document new (or modified) APIs?

@jmg-duarte jmg-duarte self-assigned this Feb 10, 2025
@jmg-duarte jmg-duarte added the ready for review Review is needed label Feb 10, 2025
@jmg-duarte jmg-duarte added this to the Phase 3 milestone Feb 10, 2025
@jmg-duarte jmg-duarte enabled auto-merge (squash) February 10, 2025 17:36
th7nder
th7nder previously approved these changes Feb 10, 2025
@jmg-duarte jmg-duarte added ready for review Review is needed and removed ready for review Review is needed labels Feb 11, 2025
Copy link
Member

@cernicc cernicc left a comment

Choose a reason for hiding this comment

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

Great catch

@jmg-duarte jmg-duarte merged commit 324ecb9 into develop Feb 11, 2025
6 checks passed
@jmg-duarte jmg-duarte deleted the fix/multipiece branch February 11, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Review is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants