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

fix(tee_verifier): correctly initialize storage for re-execution #3017

Merged
merged 48 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
e241949
fix(tee_verifier): correctly initialize storage for re-execution
haraldh Oct 2, 2024
fa06b18
fix: sql query
haraldh Oct 7, 2024
b615378
fix: remove obsolete `tee_type` check
haraldh Oct 7, 2024
39b4536
fix: clippy lint
haraldh Oct 7, 2024
ff3e6b2
Merge branch 'main' into tee_prover_new_2
haraldh Oct 7, 2024
5525997
Merge branch 'main' into tee_prover_new_2
haraldh Oct 7, 2024
b73e6e3
Merge branch 'main' into tee_prover_new_2
haraldh Oct 7, 2024
a96663a
fix: logic in tee_prover verify
haraldh Oct 8, 2024
d0b8e76
Merge branch 'main' into tee_prover_new_2
haraldh Oct 8, 2024
3a97168
fix: simplify SQL expression
haraldh Oct 9, 2024
77f44fd
feat(proof-data-handler): return 204 HTTP status code if no proofs ar…
pbeza Oct 8, 2024
5699299
WIP
pbeza Oct 9, 2024
70c82b6
Give up more comprehensive tests - need too complex mockups
pbeza Oct 9, 2024
e7c7840
fix: remove sort from sql query
haraldh Oct 8, 2024
f7e1caa
Merge remote-tracking branch 'origin/tee/feat/return-402-http-status-…
haraldh Oct 10, 2024
b93cf5d
Merge remote-tracking branch 'origin/main' into tee_prover_new_2
haraldh Oct 10, 2024
e6969d5
fix: remove L2ChainId from VMRunWitnessInputData
haraldh Oct 10, 2024
b07ba6b
fixup! fix: remove L2ChainId from VMRunWitnessInputData
haraldh Oct 10, 2024
ab23c8b
fixup! fixup! fix: remove L2ChainId from VMRunWitnessInputData
haraldh Oct 10, 2024
d852f31
fixup! fixup! fixup! fix: remove L2ChainId from VMRunWitnessInputData
haraldh Oct 10, 2024
3882898
Merge branch 'main' into tee_prover_new_2
haraldh Oct 10, 2024
2e043d4
Merge branch 'main' into tee_prover_new_2
haraldh Oct 10, 2024
8a7bd60
fixup! Merge remote-tracking branch 'origin/main' into tee_prover_new_2
haraldh Oct 10, 2024
8cb829e
Merge remote-tracking branch 'origin/tee_prover_new_2' into tee_prove…
haraldh Oct 10, 2024
8a59e0b
fix: don't use `Instrumented::new` when `.instrumented` can be used
haraldh Oct 10, 2024
fa42354
fix: merge extra match hand with already handled case
haraldh Oct 10, 2024
1087a8d
fix: use "500 Internal Server Error" for internal error
haraldh Oct 10, 2024
c521f52
fix: remove type annotation for readability
haraldh Oct 10, 2024
8556e52
fixup! fix: don't use `Instrumented::new` when `.instrumented` can be…
haraldh Oct 10, 2024
d908979
fixup! fix: merge extra match hand with already handled case
haraldh Oct 10, 2024
91cad42
fixup! fix(tee_verifier): correctly initialize storage for re-execution
haraldh Oct 10, 2024
76b5acf
Merge branch 'main' into tee_prover_new_2
haraldh Oct 10, 2024
fc30e5b
fix(migration): correct .down.sql script to ensure proper rollback
pbeza Oct 10, 2024
0122e62
fix: add a comment on storage in tee_verifier verify method
haraldh Oct 10, 2024
ab6e565
fix(tee-prover): simplify TeeProofGenerationDataResponse
pbeza Oct 10, 2024
e4264ee
Merge branch 'main' into tee_prover_new_2
haraldh Oct 11, 2024
3d77b90
Merge branch 'main' into tee_prover_new_2
haraldh Oct 11, 2024
e446fbd
Merge remote-tracking branch 'origin/tee/fix/simplify-TeeProofGenerat…
haraldh Oct 11, 2024
59f7756
fix: use connection_tagged()
haraldh Oct 16, 2024
01453f8
fix: error handling, if no job is available
haraldh Oct 16, 2024
bcaa6c5
fix: storage_log.kind is unused in match
haraldh Oct 16, 2024
c3d0833
Merge branch 'main' into tee_prover_new_2
haraldh Oct 16, 2024
beb3eef
fix: use NO_CONTENT instead of NOT_FOUND for no job
haraldh Oct 16, 2024
cb03ac8
fixup! fix: use NO_CONTENT instead of NOT_FOUND for no job
haraldh Oct 16, 2024
7e4c984
Merge branch 'main' into tee_prover_new_2
haraldh Oct 16, 2024
bf03aaf
Merge remote-tracking branch 'origin/main' into tee_prover_new_2
haraldh Oct 16, 2024
1d1b243
fixup! fix: use NO_CONTENT instead of NOT_FOUND for no job
haraldh Oct 17, 2024
cc8d14e
Merge branch 'main' into tee_prover_new_2
haraldh Oct 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions core/bin/zksync_tee_prover/src/api_client.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
use reqwest::Client;
use reqwest::{Client, Response, StatusCode};
use secp256k1::{ecdsa::Signature, PublicKey};
use serde::{de::DeserializeOwned, Serialize};
use serde::Serialize;
use url::Url;
use zksync_basic_types::H256;
use zksync_prover_interface::{
api::{
RegisterTeeAttestationRequest, RegisterTeeAttestationResponse, SubmitTeeProofRequest,
SubmitTeeProofResponse, TeeProofGenerationDataRequest, TeeProofGenerationDataResponse,
},
api::{RegisterTeeAttestationRequest, SubmitTeeProofRequest, TeeProofGenerationDataRequest},
inputs::TeeVerifierInput,
outputs::L1BatchTeeProofForL1,
};
Expand All @@ -31,10 +28,9 @@ impl TeeApiClient {
}
}

async fn post<Req, Resp, S>(&self, endpoint: S, request: Req) -> Result<Resp, reqwest::Error>
async fn post<Req, S>(&self, endpoint: S, request: Req) -> Result<Response, reqwest::Error>
where
Req: Serialize + std::fmt::Debug,
Resp: DeserializeOwned,
S: AsRef<str>,
{
let url = self.api_base_url.join(endpoint.as_ref()).unwrap();
Expand All @@ -46,9 +42,7 @@ impl TeeApiClient {
.json(&request)
.send()
.await?
.error_for_status()?
.json::<Resp>()
.await
.error_for_status()
}

/// Registers the attestation quote with the TEE prover interface API, effectively proving that
Expand All @@ -63,8 +57,7 @@ impl TeeApiClient {
attestation: attestation_quote_bytes,
pubkey: public_key.serialize().to_vec(),
};
self.post::<_, RegisterTeeAttestationResponse, _>("/tee/register_attestation", request)
.await?;
self.post("/tee/register_attestation", request).await?;
tracing::info!(
"Attestation quote was successfully registered for the public key {}",
public_key
Expand All @@ -77,12 +70,17 @@ impl TeeApiClient {
pub async fn get_job(
&self,
tee_type: TeeType,
) -> Result<Option<Box<TeeVerifierInput>>, TeeProverError> {
) -> Result<Option<TeeVerifierInput>, TeeProverError> {
let request = TeeProofGenerationDataRequest { tee_type };
let response = self
.post::<_, TeeProofGenerationDataResponse, _>("/tee/proof_inputs", request)
.await?;
Ok(response.0)
let response = self.post("/tee/proof_inputs", request).await?;
match response.status() {
StatusCode::OK => Ok(Some(response.json::<TeeVerifierInput>().await?)),
StatusCode::NO_CONTENT => Ok(None),
_ => response
.json::<Option<TeeVerifierInput>>()
.await
.map_err(TeeProverError::Request),
}
}

/// Submits the successfully verified proof to the TEE prover interface API.
Expand All @@ -101,7 +99,7 @@ impl TeeApiClient {
tee_type,
}));
let observer = METRICS.proof_submitting_time.start();
self.post::<_, SubmitTeeProofResponse, _>(
self.post(
format!("/tee/submit_proofs/{batch_number}").as_str(),
request,
)
Expand Down
9 changes: 5 additions & 4 deletions core/bin/zksync_tee_prover/src/tee_prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ impl TeeProver {
}

async fn step(&self, public_key: &PublicKey) -> Result<Option<L1BatchNumber>, TeeProverError> {
match self.api_client.get_job(self.config.tee_type).await? {
Some(job) => {
let (signature, batch_number, root_hash) = self.verify(*job)?;
match self.api_client.get_job(self.config.tee_type).await {
Ok(Some(job)) => {
let (signature, batch_number, root_hash) = self.verify(job)?;
self.api_client
.submit_proof(
batch_number,
Expand All @@ -104,10 +104,11 @@ impl TeeProver {
.await?;
Ok(Some(batch_number))
}
None => {
Ok(None) => {
tracing::trace!("There are currently no pending batches to be proven");
Ok(None)
}
Err(err) => Err(err),
}
}
}
Expand Down
8 changes: 3 additions & 5 deletions core/lib/dal/src/tee_proof_generation_dal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl TeeProofGenerationDal<'_, '_> {
) -> DalResult<Option<L1BatchNumber>> {
let processing_timeout = pg_interval_from_duration(processing_timeout);
let min_batch_number = min_batch_number.map_or(0, |num| i64::from(num.0));
let batch_number = sqlx::query!(
sqlx::query!(
r#"
WITH upsert AS (
SELECT
Expand Down Expand Up @@ -106,10 +106,8 @@ impl TeeProofGenerationDal<'_, '_> {
.with_arg("processing_timeout", &processing_timeout)
.with_arg("l1_batch_number", &min_batch_number)
.fetch_optional(self.storage)
.await?
.map(|row| L1BatchNumber(row.l1_batch_number as u32));

Ok(batch_number)
.await
.map(|record| record.map(|record| L1BatchNumber(record.l1_batch_number as u32)))
}

pub async fn unlock_batch(
Expand Down
2 changes: 1 addition & 1 deletion core/lib/prover_interface/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub enum ProofGenerationDataResponse {
}

#[derive(Debug, Serialize, Deserialize)]
pub struct TeeProofGenerationDataResponse(pub Option<Box<TeeVerifierInput>>);
pub struct TeeProofGenerationDataResponse(pub Box<TeeVerifierInput>);

#[derive(Debug, Serialize, Deserialize)]
pub enum SubmitProofResponse {
Expand Down
18 changes: 9 additions & 9 deletions core/lib/tee_verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,16 @@ fn map_log_tree(
idx: &mut u64,
) -> anyhow::Result<TreeInstruction> {
let key = storage_log.key.hashed_key_u256();
let tree_instruction = match (storage_log.is_write(), storage_log.kind, *tree_log_entry) {
(true, _, TreeLogEntry::Updated { leaf_index, .. }) => {
let tree_instruction = match (storage_log.is_write(), *tree_log_entry) {
(true, TreeLogEntry::Updated { leaf_index, .. }) => {
TreeInstruction::write(key, leaf_index, H256(storage_log.value.into()))
}
(true, _, TreeLogEntry::Inserted) => {
(true, TreeLogEntry::Inserted) => {
let leaf_index = *idx;
*idx += 1;
TreeInstruction::write(key, leaf_index, H256(storage_log.value.into()))
}
(false, _, TreeLogEntry::Read { value, .. }) => {
(false, TreeLogEntry::Read { value, .. }) => {
if storage_log.value != value {
tracing::error!(
?storage_log,
Expand All @@ -238,11 +238,11 @@ fn map_log_tree(
}
TreeInstruction::Read(key)
}
(false, _, TreeLogEntry::ReadMissingKey { .. }) => TreeInstruction::Read(key),
(true, _, TreeLogEntry::Read { .. })
| (true, _, TreeLogEntry::ReadMissingKey)
| (false, _, TreeLogEntry::Inserted)
| (false, _, TreeLogEntry::Updated { .. }) => {
(false, TreeLogEntry::ReadMissingKey { .. }) => TreeInstruction::Read(key),
(true, TreeLogEntry::Read { .. })
| (true, TreeLogEntry::ReadMissingKey)
| (false, TreeLogEntry::Inserted)
| (false, TreeLogEntry::Updated { .. }) => {
tracing::error!(
?storage_log,
?tree_log_entry,
Expand Down
26 changes: 14 additions & 12 deletions core/node/proof_data_handler/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use zksync_dal::DalError;
use zksync_object_store::ObjectStoreError;

pub(crate) enum RequestProcessorError {
NoJob,
GeneralError(String),
ObjectStore(ObjectStoreError),
Dal(DalError),
Expand All @@ -19,34 +20,35 @@ impl From<DalError> for RequestProcessorError {

impl IntoResponse for RequestProcessorError {
fn into_response(self) -> Response {
let (status_code, message) = match self {
match self {
RequestProcessorError::GeneralError(err) => {
haraldh marked this conversation as resolved.
Show resolved Hide resolved
tracing::error!("Error: {:?}", err);
(
StatusCode::INTERNAL_SERVER_ERROR,
"An internal error occurred".to_owned(),
)
.into_response()
}
RequestProcessorError::ObjectStore(err) => {
tracing::error!("GCS error: {:?}", err);
(
StatusCode::BAD_GATEWAY,
"Failed fetching/saving from GCS".to_owned(),
)
.into_response()
}
RequestProcessorError::Dal(err) => {
tracing::error!("Sqlx error: {:?}", err);
match err.inner() {
zksync_dal::SqlxError::RowNotFound => {
(StatusCode::NOT_FOUND, "Non existing L1 batch".to_owned())
}
_ => (
StatusCode::BAD_GATEWAY,
"Failed fetching/saving from db".to_owned(),
),
}
(
StatusCode::BAD_GATEWAY,
"Failed fetching/saving from db".to_owned(),
)
.into_response()
haraldh marked this conversation as resolved.
Show resolved Hide resolved
}
RequestProcessorError::NoJob => {
haraldh marked this conversation as resolved.
Show resolved Hide resolved
tracing::trace!("No job found");
(StatusCode::NO_CONTENT, ()).into_response()
}
};
(status_code, message).into_response()
}
}
}
5 changes: 2 additions & 3 deletions core/node/proof_data_handler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use zksync_dal::{ConnectionPool, Core};
use zksync_object_store::ObjectStore;
use zksync_prover_interface::api::{
ProofGenerationDataRequest, RegisterTeeAttestationRequest, SubmitProofRequest,
SubmitTeeProofRequest, TeeProofGenerationDataRequest, TeeProofGenerationDataResponse,
SubmitTeeProofRequest, TeeProofGenerationDataRequest,
};
use zksync_types::{commitment::L1BatchCommitmentMode, L2ChainId};

Expand All @@ -31,7 +31,7 @@ pub async fn run_server(
mut stop_receiver: watch::Receiver<bool>,
) -> anyhow::Result<()> {
let bind_address = SocketAddr::from(([0, 0, 0, 0], config.http_port));
tracing::debug!("Starting proof data handler server on {bind_address}");
tracing::info!("Starting proof data handler server on {bind_address}");
let app = create_proof_processing_router(
blob_store,
connection_pool,
Expand Down Expand Up @@ -109,7 +109,6 @@ fn create_proof_processing_router(
.await;

match result {
Ok(Json(TeeProofGenerationDataResponse(None))) => (StatusCode::NO_CONTENT, Json("No new TeeVerifierInputs are available yet")).into_response(),
Ok(data) => (StatusCode::OK, data).into_response(),
Err(e) => e.into_response(),
}
Expand Down
30 changes: 13 additions & 17 deletions core/node/proof_data_handler/src/tee_request_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,16 @@ impl TeeRequestProcessor {
let mut missing_range: Option<(L1BatchNumber, L1BatchNumber)> = None;

let result = loop {
let l1_batch_number = match self
let l1_batch_number = self
.lock_batch_for_proving(request.tee_type, min_batch_number)
.await?
{
Some(number) => number,
None => break Ok(Json(TeeProofGenerationDataResponse(None))),
};
.await?;

match self
.tee_verifier_input_for_existing_batch(l1_batch_number)
.await
{
Ok(input) => {
break Ok(Json(TeeProofGenerationDataResponse(Some(Box::new(input)))));
break Ok(Json(TeeProofGenerationDataResponse(Box::new(input))));
}
Err(RequestProcessorError::ObjectStore(ObjectStoreError::KeyNotFound(_))) => {
missing_range = match missing_range {
Expand Down Expand Up @@ -111,7 +107,7 @@ impl TeeRequestProcessor {

let mut connection = self
.pool
.connection()
.connection_tagged("tee_request_processor")
.await
.map_err(RequestProcessorError::Dal)?;

Expand Down Expand Up @@ -156,19 +152,19 @@ impl TeeRequestProcessor {
&self,
tee_type: TeeType,
min_batch_number: Option<L1BatchNumber>,
) -> Result<Option<L1BatchNumber>, RequestProcessorError> {
let result = self
.pool
.connection()
) -> Result<L1BatchNumber, RequestProcessorError> {
self.pool
.connection_tagged("tee_request_processor")
.await?
.tee_proof_generation_dal()
.lock_batch_for_proving(
tee_type,
self.config.proof_generation_timeout(),
min_batch_number,
)
.await?;
Ok(result)
.await
.map_err(RequestProcessorError::Dal)
.and_then(|batch_number| batch_number.ok_or(RequestProcessorError::NoJob))
}

async fn unlock_batch(
Expand All @@ -177,7 +173,7 @@ impl TeeRequestProcessor {
tee_type: TeeType,
) -> Result<(), RequestProcessorError> {
self.pool
.connection()
.connection_tagged("tee_request_processor")
.await?
.tee_proof_generation_dal()
.unlock_batch(l1_batch_number, tee_type)
Expand All @@ -191,7 +187,7 @@ impl TeeRequestProcessor {
Json(proof): Json<SubmitTeeProofRequest>,
) -> Result<Json<SubmitProofResponse>, RequestProcessorError> {
let l1_batch_number = L1BatchNumber(l1_batch_number);
let mut connection = self.pool.connection().await?;
let mut connection = self.pool.connection_tagged("tee_request_processor").await?;
let mut dal = connection.tee_proof_generation_dal();

tracing::info!(
Expand All @@ -217,7 +213,7 @@ impl TeeRequestProcessor {
) -> Result<Json<RegisterTeeAttestationResponse>, RequestProcessorError> {
tracing::info!("Received attestation: {:?}", payload);

let mut connection = self.pool.connection().await?;
let mut connection = self.pool.connection_tagged("tee_request_processor").await?;
let mut dal = connection.tee_proof_generation_dal();

dal.save_attestation(&payload.pubkey, &payload.attestation)
Expand Down
Loading