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(proof_data_handler): TEE blob fetching error handling #2674

Merged
merged 11 commits into from
Aug 26, 2024

Conversation

pbeza
Copy link
Collaborator

@pbeza pbeza commented Aug 16, 2024

We ran into a problem in the staging environment where TEE blob fetching failed because of a 30-day retention policy on blobs in Google Cloud Storage. The TEE prover was failing for all old batches (l1_batch_number < 58300). This commit fixes the issue by adding better error handling when the blob for a given batch number isn't available.

What ❔

Graceful error handling for the TEE proof data handler when there is no blob in Google Cloud Storage for the specified batch number.

Why ❔

We need more robust error handling.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@pbeza pbeza force-pushed the tee/fix/tee-prover-error-handling branch 2 times, most recently from f84702b to 56b0c80 Compare August 16, 2024 15:43
We ran into a problem in the staging environment where TEE blob fetching
failed because of a 30-day retention policy on blobs in Google Cloud
Storage. The TEE prover was failing for all old batches
(`l1_batch_number < 58300`). This commit fixes the issue by adding
better error handling when the blob for a given batch number isn't
available.
@pbeza pbeza force-pushed the tee/fix/tee-prover-error-handling branch from 56b0c80 to 61804e3 Compare August 16, 2024 15:45
@pbeza pbeza requested review from thomasknauth and popzxc August 16, 2024 16:01
@pbeza
Copy link
Collaborator Author

pbeza commented Aug 16, 2024

Tested. It seems to be working. ✅

Not sure who to ask for a review while @popzxc is on vacation. I used to ask him for all my zksync-era PRs. Any suggestions? @RomanBrodetski @EmilLuta @slowli

@pbeza pbeza marked this pull request as ready for review August 16, 2024 16:12
@pbeza pbeza requested a review from haraldh August 16, 2024 16:12
@pbeza pbeza requested a review from RomanBrodetski August 16, 2024 16:55
Also, cherry-pick some of the ideas introduced in
#2486
@pbeza pbeza force-pushed the tee/fix/tee-prover-error-handling branch from 2476f18 to 27360a9 Compare August 22, 2024 10:50
@pbeza
Copy link
Collaborator Author

pbeza commented Aug 22, 2024

@slowli Could you please approve if it looks good? Thanks!

@pbeza pbeza requested a review from slowli August 22, 2024 18:40
Copy link
Contributor

@slowli slowli left a comment

Choose a reason for hiding this comment

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

Parts that I understand look good, but since I'm not a domain expert, I'd suggest to wait for another approval 🙃

@pbeza
Copy link
Collaborator Author

pbeza commented Aug 23, 2024

@thomasknauth / @haraldh / @slowli, could you merge this? I don't have the permissions. I think 2 approvals ✅✅ should be good enough. Thanks!

@haraldh haraldh added this pull request to the merge queue Aug 26, 2024
Merged via the queue into main with commit c162510 Aug 26, 2024
49 checks passed
@haraldh haraldh deleted the tee/fix/tee-prover-error-handling branch August 26, 2024 09:17
github-merge-queue bot pushed a commit that referenced this pull request Aug 27, 2024
🤖 I have created a release *beep* *boop*
---


##
[24.22.0](core-v24.21.0...core-v24.22.0)
(2024-08-27)


### Features

* add flag to enable/disable DA inclusion verification
([#2647](#2647))
([b425561](b425561))
* **Base token:** add cbt metrics
([#2720](#2720))
([58438eb](58438eb))
* Change default_protective_reads_persistence_enabled to false
([#2716](#2716))
([8d0eee7](8d0eee7))
* **vm:** Extract oneshot VM executor interface
([#2671](#2671))
([951d5f2](951d5f2))
* **zk_toolbox:** Add holesky testnet as layer1 network
([#2632](#2632))
([d9266e5](d9266e5))


### Bug Fixes

* **api:** `tx.gas_price` field
([#2734](#2734))
([aea3726](aea3726))
* **base_token_adjuster:** bug with a wrong metrics namespace
([#2744](#2744))
([64b2ff8](64b2ff8))
* **eth-sender:** missing Gateway migration changes
([#2732](#2732))
([a4170e9](a4170e9))
* **proof_data_handler:** TEE blob fetching error handling
([#2674](#2674))
([c162510](c162510))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <zksync-era-bot@users.noreply.github.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.

5 participants