Fix dependency manager handling of block heights outside of query range#2281
Merged
Fix dependency manager handling of block heights outside of query range#2281
Conversation
jribbink
requested review from
Kay-Zee,
bjartek,
chasefleming,
janezpodhostnik,
mfbz and
nvdtf
as code owners
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
jribbink
force-pushed
the
jribbink/fix-dep-mgr
branch
from
4533725 to
0e609fd
Compare
chasefleming
approved these changes
Feb 25, 2026
There was a problem hiding this comment.
Pull request overview
This PR addresses dependency installation failures when flow.json contains pinned block heights that are no longer queryable (e.g., outside the node’s compatible historical query range), by proactively checking the node’s reported compatible range before attempting historical account queries.
Changes:
- Add a per-network “minimum queryable height” cache and use
GetNodeVersionInfo().CompatibleRangeto decide when to fall back to latest block height. - Update dependency-installer tests to mock
GetNodeVersionInfoand add/rename cases around “before compatible range” pinned heights. - Bump Go module dependencies (notably
flowkit,grpc, andotel), updatinggo.modandgo.sumaccordingly.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/dependencymanager/dependencyinstaller.go | Adds compatible-range querying + caching and changes the frozen/pinned height selection logic to avoid out-of-range queries. |
| internal/dependencymanager/dependencyinstaller_test.go | Updates mocks to include GetNodeVersionInfo and adjusts block-height pinning tests for the new compatible-range behavior. |
| go.mod | Updates module versions (including github.com/onflow/flowkit/v2). |
| go.sum | Updates dependency checksums corresponding to go.mod bumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+478
to
+488
| if err != nil { | ||
| return 0, fmt.Errorf("failed to get node version info for %s: %w", network, err) | ||
| } | ||
|
|
||
| if nodeVersionInfo == nil { | ||
| return 0, fmt.Errorf("node version info is nil for %s", network) | ||
| } | ||
|
|
||
| // Get the minimum queryable block height from the compatible range | ||
| var minHeight uint64 | ||
| if nodeVersionInfo.CompatibleRange != nil { |
There was a problem hiding this comment.
getMinQueryableBlockHeight treats GetNodeVersionInfo failures (or a nil response) as fatal and aborts dependency installation. Since this check is only needed to avoid out-of-range historical queries, it should be best-effort: if node version info can’t be fetched, proceed without the compatible-range optimization (e.g., return minHeight=0) rather than failing installs that would otherwise succeed at the pinned height.
Suggested change
| if err != nil { | |
| return 0, fmt.Errorf("failed to get node version info for %s: %w", network, err) | |
| } | |
| if nodeVersionInfo == nil { | |
| return 0, fmt.Errorf("node version info is nil for %s", network) | |
| } | |
| // Get the minimum queryable block height from the compatible range | |
| var minHeight uint64 | |
| if nodeVersionInfo.CompatibleRange != nil { | |
| // Default to 0 if we can't get node version info or compatible range. | |
| // This makes the check best-effort: dependency installation can proceed | |
| // even if version info is unavailable. | |
| var minHeight uint64 | |
| if err == nil && nodeVersionInfo != nil && nodeVersionInfo.CompatibleRange != nil { |
Comment on lines
+648
to
+668
| // Proactively check if this block height is within the node's compatible range | ||
| minQueryableHeight, err := di.getMinQueryableBlockHeight(networkName) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to check compatible block height range: %w", err) | ||
| } | ||
|
|
||
| if blockHeight < minQueryableHeight { | ||
| // Block height is before the minimum queryable height, need to use latest | ||
| hadSporkRecovery = true | ||
| // Get the current block height (will be cached from above for new deps) | ||
| latestHeight, err := di.getLatestBlockHeight(networkName) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get latest block height: %w", err) | ||
| } | ||
| // Fetch at that specific block height | ||
| accountContracts, err = di.getContractsAtBlockHeight(networkName, address, latestHeight) | ||
| if err != nil { | ||
| return fmt.Errorf("error fetching contracts: %w", err) | ||
| } | ||
| // Update blockHeight so it's used consistently for this dependency | ||
| blockHeight = latestHeight | ||
| } else { | ||
| return fmt.Errorf("error fetching contracts: %w", err) | ||
| } | ||
| } | ||
|
|
||
| accountContracts, err := di.getContractsAtBlockHeight(networkName, address, blockHeight) | ||
| if err != nil { | ||
| return fmt.Errorf("error fetching contracts: %w", err) | ||
| } |
There was a problem hiding this comment.
The previous implementation recovered from out-of-range historical queries by falling back to latest when getContractsAtBlockHeight returned spork/pruning-related errors. The new logic only checks CompatibleRange.StartHeight; if CompatibleRange is nil/zero (or inaccurate), the installer will now return the fetch error instead of recovering. Consider keeping a fallback-to-latest path when the historical fetch fails due to an out-of-range/pruned height, so old flow.json files still install successfully even when compatible-range data isn’t available.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2280
For contributor use:
masterbranchFiles changedin the Github PR explorer