fix(state): display DealIDs from market actor for sectors #13140
Conversation
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
The 'lotus state sector' command was showing empty DealIDs for sectors that actually contain deals. This occurred because for actors v13+, deal IDs are stored in the market actor's ProviderSectors HAMT, not in the sector's DeprecatedDealIDs field. This fix: - Adds dual DealIDs display showing both deprecated and market sources - Retrieves deal IDs from market actor's ProviderSectors HAMT for v13+ - Maintains backward compatibility with older network versions - Provides clear labeling of data sources - Handles errors gracefully with informative messages Resolves issue where sectors containing deals incorrectly showed 'DealIDs: []' instead of the actual deal IDs. Example output after fix: DealIDs (deprecated): [] DealIDs (market): [84864966]
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the "lotus state sector" command failed to display deal IDs for sectors with deals by adding dual DealIDs display functionality. Key changes include:
- Dual display of deal IDs from both the deprecated field and the market actor's ProviderSectors HAMT (for actors v13+).
- New function getMarketDealIDs for retrieving deal IDs from the market actor.
- Improved error handling and clear output labelling.
Improve code readability and maintainability by refactoring the getMarketDealIDs function into three focused helper functions: - getMinerActorID: Handles miner address to actor ID conversion - loadMarketState: Manages market actor state loading - extractSectorDealIDs: Extracts deal IDs from ProviderSectors HAMT Each function now has a single responsibility, making the code easier to understand, test, and maintain.
rvagg
reviewed
May 26, 2025
rvagg
reviewed
May 26, 2025
rvagg
requested changes
May 26, 2025
rvagg
reviewed
May 26, 2025
rvagg
approved these changes
May 26, 2025
github-project-automation
bot
moved this from ⌨️ In Progress
to ✔️ Approved by reviewer
in FilOz
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.
The 'lotus state sector' command was showing empty DealIDs for sectors that actually contain deals. This occurred because for actors v13+, deal IDs are stored in the market actor's ProviderSectors HAMT, not in the sector's DeprecatedDealIDs field.
This fix:
Resolves issue where sectors containing deals incorrectly showed 'DealIDs: []' instead of the actual deal IDs.
Example output after fix:
Related Issues
#12791
Proposed Changes
cli/state.go: Added dual DealIDs display functionalityAdditional Info
lotus state sector f08403 16357showing actual deal ID[100951959]Technical details:
market.Load()to access market actor stateProviderSectors()HAMT for efficient deal ID lookupaddress.IDFromAddress()Checklist
Before you mark the PR ready for review, please make sure that: