diff --git a/proposer/op/proposer/prove.go b/proposer/op/proposer/prove.go index 5ff44d92..16014845 100644 --- a/proposer/op/proposer/prove.go +++ b/proposer/op/proposer/prove.go @@ -39,7 +39,7 @@ func (l *L2OutputSubmitter) ProcessProvingRequests() error { l.Metr.RecordError("get_proof_status", 1) return err } - if proofStatus.Status == SP1FulfillmentStatusFulfilled { + if proofStatus.FulfillmentStatus == SP1FulfillmentStatusFulfilled { // Update the proof in the DB and update status to COMPLETE. l.Log.Info("Fulfilled Proof", "id", req.ProverRequestID) err = l.db.AddFulfilledProof(req.ID, proofStatus.Proof) @@ -50,7 +50,7 @@ func (l *L2OutputSubmitter) ProcessProvingRequests() error { continue } - if proofStatus.Status == SP1FulfillmentStatusUnfulfillable { + if proofStatus.FulfillmentStatus == SP1FulfillmentStatusUnfulfillable { // Record the failure reason. l.Log.Info("Proof is unfulfillable", "id", req.ProverRequestID) l.Metr.RecordProveFailure("unfulfillable") @@ -88,8 +88,6 @@ func (l *L2OutputSubmitter) ProcessWitnessgenRequests() error { // If an error response is received: // - Range Proof: Split in two if the block range is > 1. Retry the same request if range is 1 block. // - Agg Proof: Retry the same request. -// TODO: Once the reserved strategy adds an execution error, update this to retry only when there's an execution error returned. -// TODO: With a new allocator, there will not be OOM issues. func (l *L2OutputSubmitter) RetryRequest(req *ent.ProofRequest, status ProofStatusResponse) error { err := l.db.UpdateProofStatus(req.ID, proofrequest.StatusFAILED) if err != nil { @@ -97,13 +95,30 @@ func (l *L2OutputSubmitter) RetryRequest(req *ent.ProofRequest, status ProofStat return err } - // TODO: Once execution errors are added, update this to split the range only when there's an execution error returned on - // a SPAN proof. - // Retry same request. - err = l.db.NewEntry(req.Type, req.StartBlock, req.EndBlock) - if err != nil { - l.Log.Error("failed to retry proof request", "err", err) - return err + // If there's an execution error AND the request is a SPAN proof AND the block range is > 1, split the request into two requests. + // This is likely caused by an SP1 OOM due to a large block range with many transactions. + // TODO: This solution can be removed once the embedded allocator is used, because then the programs + // will never OOM. + if req.Type == proofrequest.TypeSPAN && status.ExecutionStatus == SP1ExecutionStatusUnexecutable && req.EndBlock-req.StartBlock > 1 { + // Split the request into two requests. + midBlock := (req.StartBlock + req.EndBlock) / 2 + err = l.db.NewEntry(req.Type, req.StartBlock, midBlock) + if err != nil { + l.Log.Error("failed to retry first half of proof request", "err", err) + return err + } + err = l.db.NewEntry(req.Type, midBlock+1, req.EndBlock) + if err != nil { + l.Log.Error("failed to retry second half of proof request", "err", err) + return err + } + } else { + // Retry the same request. + err = l.db.NewEntry(req.Type, req.StartBlock, req.EndBlock) + if err != nil { + l.Log.Error("failed to retry proof request", "err", err) + return err + } } return nil diff --git a/proposer/op/proposer/rpc_types.go b/proposer/op/proposer/rpc_types.go index f968b169..41cc2adc 100644 --- a/proposer/op/proposer/rpc_types.go +++ b/proposer/op/proposer/rpc_types.go @@ -53,7 +53,7 @@ func (d UnclaimDescription) String() string { } } -// SP1ProofStatus represents the status of a proof in the SP1 network. +// SP1FulfillmentStatus represents the status of a proof in the SP1 network. type SP1FulfillmentStatus int const ( @@ -64,8 +64,20 @@ const ( SP1FulfillmentStatusUnfulfillable ) +// SP1ExecutionStatus represents the status of the execution of a proof in the SP1 network. +type SP1ExecutionStatus int + +const ( + SP1ExecutionStatusUnspecified SP1ExecutionStatus = iota + SP1ExecutionStatusUnexecuted + SP1ExecutionStatusExecuted + SP1ExecutionStatusUnexecutable +) + // ProofStatusResponse is the response type for the `/status/:proof_id` RPC from the op-succinct-server. type ProofStatusResponse struct { - Status SP1FulfillmentStatus `json:"status"` - Proof []byte `json:"proof"` + FulfillmentStatus SP1FulfillmentStatus `json:"fulfillment_status"` + ExecutionStatus SP1ExecutionStatus `json:"execution_status"` + Proof []byte `json:"proof"` } + diff --git a/proposer/succinct/bin/server.rs b/proposer/succinct/bin/server.rs index f2e335d5..95f47b2e 100644 --- a/proposer/succinct/bin/server.rs +++ b/proposer/succinct/bin/server.rs @@ -25,7 +25,7 @@ use op_succinct_proposer::{ use sp1_sdk::{ network_v2::{ client::NetworkClient, - proto::network::{FulfillmentStatus, FulfillmentStrategy, ProofMode}, + proto::network::{ExecutionStatus, FulfillmentStatus, FulfillmentStrategy, ProofMode}, }, utils, HashableKey, NetworkProverV2, ProverClient, SP1Proof, SP1ProofWithPublicValues, }; @@ -400,7 +400,8 @@ async fn request_mock_span_proof( Ok(( StatusCode::OK, Json(ProofStatus { - status: sp1_sdk::network::proto::network::ProofStatus::ProofFulfilled.into(), + fulfillment_status: FulfillmentStatus::Fulfilled.into(), + execution_status: ExecutionStatus::UnspecifiedExecutionStatus.into(), proof: proof_bytes, }), )) @@ -489,7 +490,8 @@ async fn request_mock_agg_proof( Ok(( StatusCode::OK, Json(ProofStatus { - status: sp1_sdk::network::proto::network::ProofStatus::ProofFulfilled.into(), + fulfillment_status: FulfillmentStatus::Fulfilled.into(), + execution_status: ExecutionStatus::UnspecifiedExecutionStatus.into(), proof: proof.bytes(), }), )) @@ -517,7 +519,8 @@ async fn get_proof_status( return Ok(( StatusCode::INTERNAL_SERVER_ERROR, Json(ProofStatus { - status: FulfillmentStatus::UnspecifiedFulfillmentStatus.into(), + fulfillment_status: FulfillmentStatus::UnspecifiedFulfillmentStatus.into(), + execution_status: ExecutionStatus::UnspecifiedExecutionStatus.into(), proof: vec![], }), )); @@ -526,16 +529,17 @@ async fn get_proof_status( return Ok(( StatusCode::INTERNAL_SERVER_ERROR, Json(ProofStatus { - status: FulfillmentStatus::UnspecifiedFulfillmentStatus.into(), + fulfillment_status: FulfillmentStatus::UnspecifiedFulfillmentStatus.into(), + execution_status: ExecutionStatus::UnspecifiedExecutionStatus.into(), proof: vec![], }), )); } }; - // TODO: Use execution error for reserved once it's added. - let status = status.fulfillment_status(); - if status == FulfillmentStatus::Fulfilled { + let fulfillment_status = status.fulfillment_status; + let execution_status = status.execution_status; + if fulfillment_status == FulfillmentStatus::Fulfilled as i32 { let proof: SP1ProofWithPublicValues = maybe_proof.unwrap(); match proof.proof { @@ -547,7 +551,8 @@ async fn get_proof_status( return Ok(( StatusCode::OK, Json(ProofStatus { - status: status.into(), + fulfillment_status: fulfillment_status.into(), + execution_status: execution_status.into(), proof: proof_bytes, }), )); @@ -558,7 +563,8 @@ async fn get_proof_status( return Ok(( StatusCode::OK, Json(ProofStatus { - status: status.into(), + fulfillment_status: fulfillment_status.into(), + execution_status: execution_status.into(), proof: proof_bytes, }), )); @@ -569,18 +575,20 @@ async fn get_proof_status( return Ok(( StatusCode::OK, Json(ProofStatus { - status: status.into(), + fulfillment_status: fulfillment_status.into(), + execution_status: execution_status.into(), proof: proof_bytes, }), )); } _ => (), } - } else if status == FulfillmentStatus::Unfulfillable { + } else if fulfillment_status == FulfillmentStatus::Unfulfillable as i32 { return Ok(( StatusCode::OK, Json(ProofStatus { - status: status.into(), + fulfillment_status: fulfillment_status.into(), + execution_status: execution_status.into(), proof: vec![], }), )); @@ -588,7 +596,8 @@ async fn get_proof_status( Ok(( StatusCode::OK, Json(ProofStatus { - status: status.into(), + fulfillment_status: fulfillment_status.into(), + execution_status: execution_status.into(), proof: vec![], }), )) diff --git a/proposer/succinct/src/lib.rs b/proposer/succinct/src/lib.rs index 55fe3b47..1dd8970b 100644 --- a/proposer/succinct/src/lib.rs +++ b/proposer/succinct/src/lib.rs @@ -66,9 +66,9 @@ impl From for UnclaimDescription { #[derive(Serialize, Deserialize)] /// The status of a proof request. pub struct ProofStatus { - // Note: Can't use `SP1FulfillmentStatus` directly because `Serialize_repr` and `Deserialize_repr` aren't derived on it. - // serde_repr::Serialize_repr and Deserialize_repr are necessary to use `SP1FulfillmentStatus` in this struct. - pub status: i32, + // Note: Can't use `FulfillmentStatus`/`ExecutionStatus` directly because `Serialize_repr` and `Deserialize_repr` aren't derived on it. + pub fulfillment_status: i32, + pub execution_status: i32, pub proof: Vec, }