-
Notifications
You must be signed in to change notification settings - Fork 0
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
Better handle SP1 errors #39
Conversation
b303b4c
to
5e7f22f
Compare
// TODO: We cannot clone KeccakInclusionToDataRootProofInput thus we cannot insert into a JobStatus::DataAvalibile(proof_input) | ||
// So we just redo the work from scratch for the DA side as a stupid workaround | ||
job_status = | ||
JobStatus::Failed(e.clone(), Some(JobStatus::DataAvailabilityPending.into())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not great. Interally to that type,there is a hasher
that isn't Copy/Clone we likely can't get (at least directly.
Thinking of other ways that don't need a clone here 🤔
data_base | ||
.insert( | ||
job_key, | ||
bincode::serialize(&update_status) | ||
.map_err(|e| InclusionServiceError::GeneralError(e.to_string()))?, | ||
) | ||
.map_err(|e| InclusionServiceError::GeneralError(e.to_string()))?; | ||
(&self.queue_db, &self.finished_db) | ||
.transaction(|(queue_tx, finished_tx)| { | ||
finished_tx.remove(job_key.clone())?; | ||
queue_tx.insert( | ||
job_key.clone(), | ||
bincode::serialize(&update_status) | ||
.expect("Always given serializable job status"), | ||
)?; | ||
Ok::<(), sled::transaction::ConflictableTransactionError<InclusionServiceError>>(()) | ||
}) | ||
.map_err(|e| InclusionServiceError::InternalError(e.to_string()))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated fix to ensure we always have jobs finished OR in queue.
Fixes #36 (at least in part)