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

replace SimpleQaPower with flags #212

Conversation

LesnyRumcajs
Copy link
Contributor

See filecoin-project/builtin-actors#1275
and filecoin-project/builtin-actors#1395 (most notably @ZenGround0 comment)

In short, we want to have an easily extensible flags field for the SectorOnChainInfo instead of a field per flag, which is wasteful.

@LesnyRumcajs LesnyRumcajs force-pushed the replace-qa-power-with-flags-miner branch from bbd32e3 to dd56b95 Compare September 1, 2023 21:16
@LesnyRumcajs LesnyRumcajs marked this pull request as ready for review September 1, 2023 21:18
Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

Two small things but otherwise LGTM

builtin/v12/migration/miner.go Outdated Show resolved Hide resolved
builtin/v12/miner/miner_state.go Outdated Show resolved Hide resolved
@LesnyRumcajs LesnyRumcajs force-pushed the replace-qa-power-with-flags-miner branch from d2aa21d to e3b5c01 Compare September 1, 2023 21:48
@LesnyRumcajs
Copy link
Contributor Author

LesnyRumcajs commented Sep 1, 2023

Hm, should the type of the flag be aligned with the type in bultin actors? I.e., both either 32 or 64 bits?

@ZenGround0
Copy link
Contributor

Hm, should the type of the flag be aligned with the type in bultin actors? I.e., both either 32 or 64 bits?

I gave it a try and cbor gen does not work with int32s. This should be fine for the long foreseeable future until with have 31 new flags. I'll file an issue.

@ZenGround0 ZenGround0 merged commit 11d4a00 into filecoin-project:master Sep 2, 2023
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.

2 participants