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

feat: actor state accessors: Add accessors to DealState.SectorNumber #11936

Closed
wants to merge 1 commit into from

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Apr 27, 2024

Related Issues

Proposed Changes

Prompted by the fact that this will be really useful for me in RIBS, and very likely for others - it is now much, much easier to match on-chain deals with sectors.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

Nice. I think I recall a conversation during the migration work that 0 is a valid sector number, that's not an active concern here is it? Would -1 be a better signal?

SectorStartEpoch abi.ChainEpoch // -1 if not yet included in proven sector
LastUpdatedEpoch abi.ChainEpoch // -1 if deal state never updated
SlashEpoch abi.ChainEpoch // -1 if deal never slashed
SectorNumber abi.SectorNumber // 0 if not activated or not supported (pre-nv22)
}

func MakeDealState(mds market.DealState) MarketDealState {
Copy link
Member

Choose a reason for hiding this comment

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

this is missing SectorNumber in the constructor

@rvagg
Copy link
Member

rvagg commented May 16, 2024

I accidentally did the same PR at #11998, but I have the minor fix I mentioned above; perhaps someone could review that PR and we can just land it. Or fix this one and land it, either way is fine.

@rvagg
Copy link
Member

rvagg commented May 17, 2024

merged #11998

@rvagg rvagg closed this May 17, 2024
@rvagg rvagg deleted the feat/marketstate-sector-getter branch May 17, 2024 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants