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

FIP-0019: Minor fixes and test #1534

Merged
merged 3 commits into from Nov 27, 2021
Merged

FIP-0019: Minor fixes and test #1534

merged 3 commits into from Nov 27, 2021

Conversation

arajasek
Copy link
Collaborator

As in the title.

@arajasek arajasek requested a review from a team as a code owner November 25, 2021 21:42
} else if !exists {
return xc.ErrNotFound.Wrapf("sector %d not a member of partition %d, deadline %d", sector, pIdx, dlIdx)
return false, xc.ErrNotFound.Wrapf("sector %d not a member of partition %d, deadline %d", sector, pIdx, dlIdx)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

treating a wrong location here as an error failure, agree?

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2021

Codecov Report

Merging #1534 (786dbf7) into master (36d4d5c) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

@@           Coverage Diff            @@
##           master   #1534     +/-   ##
========================================
- Coverage    69.3%   69.3%   -0.1%     
========================================
  Files          72      72             
  Lines        8732    8737      +5     
========================================
  Hits         6060    6060             
- Misses       1812    1817      +5     
  Partials      860     860             

@arajasek arajasek merged commit 2436f91 into master Nov 27, 2021
// Returns an error if the target sector cannot be found, or some other bad state is reached.
// Returns false if the target sector is faulty, terminated, or unproven
// Returns true otherwise
func (st *State) CheckSectorHealthExcludeUnproven(store adt.Store, dlIdx, pIdx uint64, sector abi.SectorNumber) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a more succinct way to put this is CheckSectorActive


// Prepare message.
params := miner.PreCommitSectorBatchParams{Sectors: make([]miner0.SectorPreCommitInfo, batchSize)}
if expiration == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use < 0 or -1


info := vm.SectorInfo(t, v, minerAddrs.RobustAddress, sectorNumber)
require.Equal(t, 1, len(info.DealIDs))
require.Equal(t, dealIDs[0], info.DealIDs[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: check all other sector info fields, especially sector key and sealed sector id

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.

LGTM

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.

None yet

4 participants