Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

decouple SealVerifyInfo from OnChainSealVerifyInfo (and rename to SealVerifyParams) #378

Merged
merged 8 commits into from
May 21, 2020

Conversation

laser
Copy link
Contributor

@laser laser commented May 13, 2020

Why does this PR exist?

Fixes #276.

  • The OnChainXxxVerifyInfo structures should be renamed as XxParams
  • and moved to the miner actor after other things don't embed them directly.
  • This needs some coordinate with the FFI library which uses these types directly (but discards some unneeded info) - see downstream PR.

What's in this PR?

  • abi.SealVerifyInfo no longer embeds the abi.OnChainSealVerifyInfo (it now mirrors the required fields from the original struct).
  • OnChainSealVerifyInfo renamed to SealVerifyParams.
  • SealVerifyParams moved to miner package.

@laser laser force-pushed the feat/adjusted-verification-iface branch from 32d2e68 to 04b73e0 Compare May 13, 2020 21:28
@laser laser marked this pull request as ready for review May 13, 2020 21:49
@laser laser requested a review from anorth May 13, 2020 21:50
Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM after tweak.

Please don't land this, but wait for me to do so after we have a release branch for testnet-2

actors/abi/sector.go Outdated Show resolved Hide resolved
actors/builtin/miner/miner_state.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #378 into master will increase coverage by 0.04%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   42.73%   42.77%   +0.04%     
==========================================
  Files          40       40              
  Lines        4210     4213       +3     
==========================================
+ Hits         1799     1802       +3     
  Misses       2151     2151              
  Partials      260      260              

@laser
Copy link
Contributor Author

laser commented May 14, 2020

@anorth

Please don't land this, but wait for me to do so after we have a release branch for testnet-2

Sounds good. Please do let me know when it's safe to merge (or just merge it yourself) and I'll handle the filecoin-ffi side of things.

@laser laser force-pushed the feat/adjusted-verification-iface branch from 2f18f4f to 698b8d2 Compare May 14, 2020 16:04
@codecov-commenter
Copy link

Codecov Report

Merging #378 into master will increase coverage by 0.04%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   42.68%   42.72%   +0.04%     
==========================================
  Files          40       40              
  Lines        4215     4218       +3     
==========================================
+ Hits         1799     1802       +3     
  Misses       2156     2156              
  Partials      260      260              

@anorth anorth merged commit 0df536f into master May 21, 2020
@anorth anorth deleted the feat/adjusted-verification-iface branch May 21, 2020 01:45
aarshkshah1992 pushed a commit that referenced this pull request Jun 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decouple SealVerifyInfo from OnChainSealVerifyInfo
4 participants