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

Add FaultySectors error to Fallback PoSt. #1274

Merged
merged 3 commits into from
Sep 8, 2020

Conversation

porcuquine
Copy link
Collaborator

@porcuquine porcuquine commented Sep 7, 2020

Instead of just failing when trying to generate a Fallback PoSt which contains faulty sectors, instead return a new FaultySectors error which contains a vector of faulty SectorIds.

In order to make use of the information in the enhanced error, filecoin-ffi will need to handle:

https://github.com/filecoin-project/filecoin-ffi/blob/master/rust/src/proofs/api.rs#L606-L609
https://github.com/filecoin-project/filecoin-ffi/blob/master/rust/src/proofs/types.rs#L253-L258

This might entail a breaking change of the filecoin-ffi consumed by Go, but the current change to rust-fil-proofs is purely additive. It just adds a new, more informative, error in a case where an error would already have been returned.

The included test should give a clear example of how to extract the faulty SectorIds from a potentially faulty result.

@porcuquine porcuquine marked this pull request as ready for review September 7, 2020 18:21
vmx
vmx previously approved these changes Sep 8, 2020
Copy link
Contributor

@vmx vmx left a comment

Choose a reason for hiding this comment

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

Things look correct to me, I've just left comments that are coding style/simplification things that I would do. That's FYI as I find it useful to see how others approach things.

storage-proofs/post/src/fallback/vanilla.rs Show resolved Hide resolved
storage-proofs/post/src/fallback/vanilla.rs Outdated Show resolved Hide resolved
storage-proofs/post/src/fallback/vanilla.rs Outdated Show resolved Hide resolved
storage-proofs/post/src/fallback/vanilla.rs Outdated Show resolved Hide resolved
storage-proofs/post/src/fallback/vanilla.rs Show resolved Hide resolved
storage-proofs/post/src/fallback/vanilla.rs Outdated Show resolved Hide resolved
cryptonemo
cryptonemo previously approved these changes Sep 8, 2020
Copy link
Collaborator

@cryptonemo cryptonemo left a comment

Choose a reason for hiding this comment

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

This looks good to me. Up to you if you want to address @vmx's feedback

@porcuquine porcuquine dismissed stale reviews from cryptonemo and vmx via ef235b3 September 8, 2020 15:51
@porcuquine porcuquine merged commit 168f71b into master Sep 8, 2020
@porcuquine porcuquine deleted the feat/faulty-sectors-error branch September 8, 2020 16:28
eee-byte added a commit to eee-byte/rust-fil-proofs that referenced this pull request Sep 10, 2020
* Feat: pin params to the filecoin collab cluster. (filecoin-project#1263)

This cluster is way faster to ingest data.

The `--local` option makes ingestion even faster (it will put the data
on one peer only while adding and pin everywhere at the end).

Additionally, we trigger pin requests in the main cluster (async) so that
params will be replicated by those nodes too (at some point, once they finish
to fetch them) from the collab cluster.

* Remove unwrap (filecoin-project#1260)

* build(storage-proofs): clippy allow many_single_char_names

clippy emits warning:

        warning: 8 bindings with single-character names in scope

The single character names are a valid use case in this instance,
elements of an array. Elect to allow the usage; instruct clippy to
allow this lint for function `insert_8`.

* refactor(fil-proofs-tooling): remove all uses of unwrap()

Using `expect()` with a useful argument string assists when debugging.

Replace all instances of `unwrap()` with `expect()`.

Makes a start at resolving: filecoin-project#390

* refactor(filecoin-proofs): remove all uses of unwrap()

Using `expect()` with a useful argument string assists when debugging.

Replace all instances of `unwrap()` with `expect()`.

Works towards resolving: filecoin-project#390

* build(fil-proofs-tooling): set clippy lint warn for: unwrap_used

We have removed all the usages of `unwrap()` in `fil-proofs-tooling\`.

In order to resolve filecoin-project#390 instruct clippy to warn for lint
`unwrap_used` for all binaries and the main library within
fil-proofs-tooling.

* build(filecoin-proofs): set clippy lint warn for: unwrap_used

We have removed all the usages of `unwrap()` in `filecoin-proofs\`.

In order to resolve filecoin-project#390 instruct clippy to warn for lint
`unwrap_used` for the filecoin-proofs library.

* build(storage-proofs): set clippy lint warn for: unwrap_used

We have removed all the usages of `unwrap()` in `storage-proofs\`.

In order to resolve filecoin-project#390 instruct clippy to warn for lint
`unwrap_used` for all libraries in store-proofs.

* - fix: manually apply phase2 unwrap changes

* fix: replace all remaining usages of unwrap

For this round of updates, I used nightly to detect all instances, but
have reverted after making sure they've been replaced.

Co-authored-by: tcharding <me@tobin.cc>

* Eliminate wasteful public-input conversions.

* Decompress proofs in parallel.

* feat: accelerate SNARK verification (filecoin-project#1271)

* feat: accelerate SNARK verification

This PR integrates lotus-blst in order to accelerate the SNARK verification.

This feature can be enabled with the `use_fil_blst` setting.

* docs: update changelog for new release

* chore(storage-proofs-core): release 5.1.2

* chore(storage-proofs-porep): release 5.1.2

* chore(storage-proofs-post): release 5.1.2

* chore(storage-proofs): release 5.1.2

* chore(filecoin-proofs): release 5.1.2

* chore(fil-proofs-tooling): release 5.1.2

* feat: accelerate SNARK verification for Window PoSt

This feature can be enabled with the `use_fil_blst` setting.

* chore: add log information when fil-blst is used

* test: add CI for running tests with fil-blst

Ideally the tests with fil-blst enabled can be run without any special
flags, but for now it's good enough to run them on CI.

* docs: update changelog for release

* chore(storage-proofs-core): release 5.1.3

* chore(storage-proofs-porep): release 5.1.3

* chore(storage-proofs-post): release 5.1.3

* chore(storage-proofs): release 5.1.3

* chore(filecoin-proofs): release 5.1.3

* chore(fil-proofs-tooling): release 5.1.3

* Add FaultySectors error to Fallback PoSt. (filecoin-project#1274)

* Add FaultySectors error to Fallback PoSt.

* Test FaultySectors error with invalid PoSt.

* Apply code review.

Co-authored-by: porcuquine <porcuquine@users.noreply.github.com>

* docs: update changelog for release

* chore(storage-proofs-core): release 5.1.4

* chore(storage-proofs-porep): release 5.1.4

* chore(storage-proofs-post): release 5.1.4

* chore(storage-proofs): release 5.1.4

* chore(filecoin-proofs): release 5.1.4

* chore(fil-proofs-tooling): release 5.1.4

Co-authored-by: Hector Sanjuan <code@hector.link>
Co-authored-by: nemo <nemo@protocol.ai>
Co-authored-by: tcharding <me@tobin.cc>
Co-authored-by: porcuquine <porcuquine@users.noreply.github.com>
Co-authored-by: porcuquine <1746729+porcuquine@users.noreply.github.com>
Co-authored-by: Volker Mische <volker.mische@gmail.com>
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.

3 participants