-
Notifications
You must be signed in to change notification settings - Fork 81
Fix dependency manager handling of block heights outside of query range #2281
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,6 @@ import ( | |
| "encoding/hex" | ||
| "fmt" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/psiemens/sconfig" | ||
|
|
||
|
|
@@ -130,24 +129,25 @@ func (f *DependencyFlags) AddToCommand(cmd *cobra.Command) { | |
| } | ||
|
|
||
| type DependencyInstaller struct { | ||
| Gateways map[string]gateway.Gateway | ||
| Logger output.Logger | ||
| State *flowkit.State | ||
| SaveState bool | ||
| TargetDir string | ||
| SkipDeployments bool | ||
| SkipAlias bool | ||
| SkipUpdatePrompts bool | ||
| Update bool | ||
| DeploymentAccount string | ||
| Name string | ||
| logs categorizedLogs | ||
| dependencies map[string]config.Dependency | ||
| accountAliases map[string]map[string]flowsdk.Address // network -> account -> alias | ||
| installCount int // Track number of dependencies installed | ||
| pendingPrompts []pendingPrompt // Dependencies that need prompts after tree display | ||
| prompter Prompter // Optional: for testing. If nil, uses real prompts | ||
| blockHeightCache map[string]uint64 // Cache of latest block heights per network for consistent pinning | ||
| Gateways map[string]gateway.Gateway | ||
| Logger output.Logger | ||
| State *flowkit.State | ||
| SaveState bool | ||
| TargetDir string | ||
| SkipDeployments bool | ||
| SkipAlias bool | ||
| SkipUpdatePrompts bool | ||
| Update bool | ||
| DeploymentAccount string | ||
| Name string | ||
| logs categorizedLogs | ||
| dependencies map[string]config.Dependency | ||
| accountAliases map[string]map[string]flowsdk.Address // network -> account -> alias | ||
| installCount int // Track number of dependencies installed | ||
| pendingPrompts []pendingPrompt // Dependencies that need prompts after tree display | ||
| prompter Prompter // Optional: for testing. If nil, uses real prompts | ||
| blockHeightCache map[string]uint64 // Cache of latest block heights per network for consistent pinning | ||
| minQueryableHeightCache map[string]uint64 // Cache of minimum queryable block heights per network (from CompatibleRange) | ||
| } | ||
|
|
||
| type Prompter interface { | ||
|
|
@@ -189,23 +189,24 @@ func NewDependencyInstaller(logger output.Logger, state *flowkit.State, saveStat | |
| } | ||
|
|
||
| return &DependencyInstaller{ | ||
| Gateways: gateways, | ||
| Logger: logger, | ||
| State: state, | ||
| SaveState: saveState, | ||
| TargetDir: targetDir, | ||
| SkipDeployments: flags.skipDeployments, | ||
| SkipAlias: flags.skipAlias, | ||
| SkipUpdatePrompts: flags.skipUpdatePrompts, | ||
| Update: flags.update, | ||
| DeploymentAccount: flags.deploymentAccount, | ||
| Name: flags.name, | ||
| dependencies: make(map[string]config.Dependency), | ||
| logs: categorizedLogs{}, | ||
| accountAliases: make(map[string]map[string]flowsdk.Address), | ||
| pendingPrompts: make([]pendingPrompt, 0), | ||
| prompter: prompter{}, | ||
| blockHeightCache: make(map[string]uint64), | ||
| Gateways: gateways, | ||
| Logger: logger, | ||
| State: state, | ||
| SaveState: saveState, | ||
| TargetDir: targetDir, | ||
| SkipDeployments: flags.skipDeployments, | ||
| SkipAlias: flags.skipAlias, | ||
| SkipUpdatePrompts: flags.skipUpdatePrompts, | ||
| Update: flags.update, | ||
| DeploymentAccount: flags.deploymentAccount, | ||
| Name: flags.name, | ||
| dependencies: make(map[string]config.Dependency), | ||
| logs: categorizedLogs{}, | ||
| accountAliases: make(map[string]map[string]flowsdk.Address), | ||
| pendingPrompts: make([]pendingPrompt, 0), | ||
| prompter: prompter{}, | ||
| blockHeightCache: make(map[string]uint64), | ||
| minQueryableHeightCache: make(map[string]uint64), | ||
| }, nil | ||
| } | ||
|
|
||
|
|
@@ -461,6 +462,40 @@ func (di *DependencyInstaller) processDependency(dependency config.Dependency) e | |
| return di.processDependencies(dependency) | ||
| } | ||
|
|
||
| func (di *DependencyInstaller) getMinQueryableBlockHeight(network string) (uint64, error) { | ||
| // Check cache first | ||
| if height, ok := di.minQueryableHeightCache[network]; ok { | ||
| return height, nil | ||
| } | ||
|
|
||
| gw, ok := di.Gateways[network] | ||
| if !ok { | ||
| return 0, fmt.Errorf("gateway for network %s not found", network) | ||
| } | ||
|
|
||
| ctx := context.Background() | ||
| nodeVersionInfo, err := gw.GetNodeVersionInfo(ctx) | ||
| 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 { | ||
| minHeight = nodeVersionInfo.CompatibleRange.StartHeight | ||
| } | ||
|
|
||
| // Cache the result (only if cache map is initialized) | ||
| if di.minQueryableHeightCache != nil { | ||
| di.minQueryableHeightCache[network] = minHeight | ||
| } | ||
| return minHeight, nil | ||
| } | ||
|
|
||
| // getLatestBlockHeight returns the current block height for a given network. | ||
| // Results are cached per network to ensure all dependencies in a single install | ||
| // operation get pinned to the same block height for consistency. | ||
|
|
@@ -609,33 +644,29 @@ func (di *DependencyInstaller) fetchDependenciesWithDepth(dependency config.Depe | |
| } else { | ||
| // Use pinned block height for frozen dependencies | ||
| blockHeight = existingDependency.BlockHeight | ||
| } | ||
|
|
||
| accountContracts, err := di.getContractsAtBlockHeight(networkName, address, blockHeight) | ||
| if err != nil { | ||
| // If we get a spork-related error (block height too old), fall back to latest | ||
| // This happens when flow.json has old block heights from before the current spork | ||
| // We'll check the hash later - if it matches, we just update metadata; if not, normal update flow applies | ||
| if strings.Contains(err.Error(), "spork root block height") || strings.Contains(err.Error(), "key not found") { | ||
| di.Logger.Info(fmt.Sprintf(" %s Block height %d is from before current spork, fetching latest version", util.PrintEmoji("⚠️"), blockHeight)) | ||
| // 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) | ||
| } | ||
|
Comment on lines
+648
to
+668
|
||
|
|
||
| contract, ok := accountContracts[contractName] | ||
| if !ok { | ||
| return fmt.Errorf("contract %s not found at address %s", contractName, address.String()) | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getMinQueryableBlockHeighttreatsGetNodeVersionInfofailures (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.