Skip to content

Conversation

yaziciahmet
Copy link
Contributor

@yaziciahmet yaziciahmet commented Oct 14, 2024

Description

Added initial support for light client, and an example test. This PR is intended to give skeleton to light client both file structure-wise and behavior-wise. It is in no way complete functionality of light client.

IMPORTANT:
Before merging this pr, citrea-e2e pr chainwayxyz/citrea-e2e#20 should be merged, and rev in Cargo.toml should be updated accordingly.

Linked Issues

  • Fixes # (issue, if applicable)
  • Related to # (issue)

Testing

Describe how these changes were tested. If you've added new features, have you added unit tests?

Docs

Describe where this code is documented. If it changes a documented interface, have the docs been updated?

kpp and others added 24 commits October 14, 2024 14:21
* kpp's changes for da

* fix tests after kpp's changes

* make prover service accept bytes so that it can work on different inputs

* lint

* rename prover to batch prover

* move prover service to its own crate

* rename prover related functions to batch prover *

* rename more prover related things to batch prover *

* create barebone light client prover crate

* add runner to light client prover

* fix cherry pick mistake on da changes

* write native part skeleton for light client prover

* add mock da and bitcoin da light client circuit guests

* Fix light client method ids

* Fix typo

* Remove unused dep

* Add Send + Sync to LightClientLedgerOps

* Add light client to e2e test

* Handle proof len 0 case

* Cleanup light client l1 handler

* Carry ledger db op outside

* Remove empty checks

* Disable rocksdb default feats

* Implemented wait_for_proving_and_extract_output

* Cleanup waiting for proof

* Specify batch prover name in get code commitments method

* Add light client code commitments by spec

* Carry ledger db op into process

* Rename prover_config_path arg to batch_prover_config_path

* Add light client config opt to cli

* lint

* Update regtest config

* Update gitignore and makefile

* Update docs

* Fix forgotten conflict

* Fix merge conflicts

* Cleanup create_new_light_client_prover

* Add create_light_client_prover_service method

* Remove unused bitcoin-e2e files

* Use single code commitment

---------

Co-authored-by: Roman Proskuryakoff <r.proskuryakoff@gmail.com>
Co-authored-by: eyusufatik <esadyusufatik@gmail.com>
Copy link

codecov bot commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 15.93186% with 839 lines in your changes missing coverage. Please review.

Project coverage is 76.2%. Comparing base (82ca586) to head (610eb17).
Report is 1 commits behind head on nightly.

Files with missing lines Patch % Lines
crates/light-client-prover/src/da_block_handler.rs 0.0% 177 Missing ⚠️
crates/bitcoin-da/src/verifier.rs 0.5% 166 Missing ⚠️
crates/bitcoin-da/src/service.rs 0.0% 138 Missing ⚠️
crates/light-client-prover/src/runner.rs 0.0% 119 Missing ⚠️
bin/citrea/src/rollup/mod.rs 16.9% 49 Missing ⚠️
bin/citrea/src/main.rs 0.0% 31 Missing ⚠️
bin/citrea/src/rollup/bitcoin.rs 0.0% 29 Missing ⚠️
crates/light-client-prover/src/circuit.rs 0.0% 29 Missing ⚠️
...ates/sovereign-sdk/adapters/mock-da/src/service.rs 59.0% 25 Missing ⚠️
crates/prover-services/src/parallel/mod.rs 51.0% 23 Missing ⚠️
... and 10 more
Additional details and impacted files
Files with missing lines Coverage Δ
crates/batch-prover/src/db_migrations/mod.rs 100.0% <ø> (ø)
crates/batch-prover/src/errors.rs 0.0% <ø> (ø)
crates/batch-prover/src/lib.rs 50.0% <ø> (ø)
crates/batch-prover/src/runner.rs 89.8% <100.0%> (ø)
...c/helpers/builders/light_client_proof_namespace.rs 5.5% <ø> (ø)
crates/bitcoin-da/src/helpers/builders/tests.rs 98.0% <100.0%> (ø)
crates/bitcoin-da/src/helpers/mod.rs 72.0% <ø> (ø)
crates/common/src/da.rs 94.1% <100.0%> (-0.4%) ⬇️
crates/prover-services/src/parallel/prover.rs 88.2% <100.0%> (ø)
crates/risc0-bonsai/src/host.rs 16.6% <100.0%> (-0.9%) ⬇️
... and 27 more

/// Give the guest a piece of advice non-deterministically
fn add_hint<T: BorshSerialize>(&mut self, item: T);
/// `item` is a borsh serialized input to the guest
fn add_hint(&mut self, item: Vec<u8>);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

had some problems regarding types idk

Copy link
Contributor

Choose a reason for hiding this comment

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

We should fix it then?

Copy link
Member

Choose a reason for hiding this comment

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

is it really important?

every adapter was doing borsh::to_vec()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is important as this is more error-prone. Enforcing a codec on data sharing level would be much safer and dev friendly to use.

@eyusufatik eyusufatik added HOLD-MERGE PR is not draft but should not be merged yet and removed HOLD-MERGE PR is not draft but should not be merged yet labels Oct 17, 2024
@eyusufatik eyusufatik force-pushed the esad/light-client-starter branch from 65d73dd to 4d38d84 Compare October 23, 2024 12:24
/// Give the guest a piece of advice non-deterministically
fn add_hint<T: BorshSerialize>(&mut self, item: T);
/// `item` is a borsh serialized input to the guest
fn add_hint(&mut self, item: Vec<u8>);
Copy link
Member

Choose a reason for hiding this comment

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

is it really important?

every adapter was doing borsh::to_vec()

]

[[package]]
name = "brotli"
Copy link
Member

Choose a reason for hiding this comment

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

it's coming from primitives, and also bitcoin da verifier now needs it, need a lot of feature gating to prevent this I guess

@eyusufatik eyusufatik merged commit 7da205c into nightly Oct 25, 2024
12 of 14 checks passed
@eyusufatik eyusufatik deleted the esad/light-client-starter branch October 25, 2024 10:47
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.

4 participants