feat: add fallback option if no t_aux file exists #1715
Conversation
| path: cache_path.clone(), | ||
| id: CacheKey::CommRLastTree.to_string(), | ||
| size: Some(tree_size), | ||
| // A default 'rows_to_discard' value will be chosen for tree_r_last, unless the user |
There was a problem hiding this comment.
I think it's possible that this part/assumption may break things. It's reasonable, but may require some more thought/testing across those cases.
There was a problem hiding this comment.
Why would it break things? This is copy and pasted from the original code. The only difference is that we take the default value/the env variable and don't read it from disk.
Is this a comment or blocking a merge? If it's blocking, what do you suggest?
| // Switch t_aux to the passed in cache_path | ||
| res.set_cache_path(cache_path); | ||
| res | ||
| match fs::read(&t_aux_path) { |
There was a problem hiding this comment.
As-is, this won't work for Sector Updates (and maybe other parts?). I don't think that path will be much more complicated, but does require more testing, as t_aux is very much not optional in at least that part of the code.
There was a problem hiding this comment.
Isn't the Sector Updates part self-contained at https://github.com/filecoin-project/rust-fil-proofs/blob/2ec0a0fe4005a6f2f6659bbbc167a1d41ee855e0/filecoin-proofs/src/api/update.rs? This part should only concern the C1.
So would be adding a test that removes the t_aux and then runs C1 be sufficient?
There was a problem hiding this comment.
The sector updates code currently relies on t_aux being available and if it's not available from an earlier step in sealing, future sector updates won't work.
There was a problem hiding this comment.
Sorry, now I get it. The problem is not about the code paths. Each works fine independently. The problem is that if someone seals a CC sector and removes their t_aux, they could then still C1, but they later could not do a sector update.
This means that I also need to take care of the sector update code path. Would adding a similar fallback there be an OK solution?
|
I thought about this again. Would it help that we make it clear in the README that you're on your own if you set the If you don't have a |
The only information from the `t_aux` file that is used during Commitphase 1 (C1) is the number of rows that are discarded from TreeC. If that value is either the default, or set via `FIL_PROOFS_ROWS_TO_DISCARD` accordingly, then all other parts of `t_aux` can be inferred from the function call. When `FIL_PROOFS_T_AUX_IMPLICIT_FALLBACK=1` is set, then those inferred values will be used, in case the `t_aux` file cannot be read or doesn't exist.
|
Superseded by #1717. |
The only information from the
t_auxfile that is used during Commitphase 1 (C1) is the number of rows that are discarded from TreeC. If that value is either the default, or set viaFIL_PROOFS_ROWS_TO_DISCARDaccordingly, then all other parts oft_auxcan be inferred from the function call.When
FIL_PROOFS_T_AUX_IMPLICIT_FALLBACK=1is set, then those inferred values will be used, in case thet_auxfile cannot be read or doesn't exist.