fix: number of shards calculation#579
Conversation
There was a problem hiding this comment.
Pull request overview
This PR avoids using the EXPERIMENTAL_protocol_config RPC (which currently fails to deserialize due to an enum-version mismatch in near-primitives-core v0.34.7) by deriving the shard count from an already-fetched block response.
Changes:
- Fetch the full block once and compute
number_of_shardsfromblock.chunks.len()instead of callingEXPERIMENTAL_protocol_config. - Remove the extra RPC request previously used to determine shard layout / shard count.
- Update
Cargo.lockwith a broad set of dependency version changes.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/commands/contract/download_wasm/mod.rs |
Uses the block response (header + chunks) to derive shard count and removes the protocol-config RPC call. |
Cargo.lock |
Large lockfile refresh including near-* patch bumps and transitive dependency upgrades (notably ureq major version bump). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let block_height = block.header.height; | ||
| let number_of_shards = block.chunks.len() as u64; | ||
|
|
||
| for block_height in block_height..=block_height + number_of_shards * 2 { | ||
| tracing::info!( |
There was a problem hiding this comment.
The loop variable block_height shadows the outer block_height derived from the fetched block. This makes the control flow and the final error message harder to reason about. Consider renaming the loop variable (e.g., height) to avoid shadowing and improve readability.
There was a problem hiding this comment.
It was already like that before, I don't see any issue
There was a problem hiding this comment.
No great in general as the block.header.height could have used a more descriptive name that would communicate the fact that it is the first block where should start searching from since it is the block that the transaction landed. Yet, I don't see a reason to re-run the CI just for this
|
We need to do something with flaky tests. 3 reruns with 3 different outcomes is not okay P.S: I see that fix is already in main branch |
|
@frolvanya it should be fixed in general with upcoming 2.11 release of sandboxed nearcore - |
The
EXPERIMENTAL_protocol_configRPC response includes"account_id_validity_rules_version": 2, but theAccountIdValidityRulesVersionenum in the publishednear-primitives-corecrate v0.34.7 only defines variantsV0andV1, so serde rejects the value during deserialization. This will be added innear-primitives-corecrate v0.35.0, but I think it's safer and faster to eliminate additional RPC request by fetching number of shards from an existing block fetch request