feat: hello-world namespace support for ADVZ VID scheme #389

ggutoski · 2023-10-30T23:51:16Z

closes: #388

Revised 2023-11-06

Force-pushed update incorporating comments from previous reviews and offline discussion.

Add PayloadProver subtrait for VidScheme. It's a generic trait with two implementations for ADVZ. See rustdoc:

jellyfish/primitives/src/vid/advz/payload_prover.rs

Lines 9 to 15 in 93e396e

    
           //! Two implementations: 
        
           //! 1. `PROOF = `[`Proof`]: KZG batch proof for the data. Useful for small 
        
           //!    sub-slices of `payload` such as an individual transaction within a block. 
        
           //!    Not snark-friendly because it requires a pairing. 
        
           //! 2. `PROOF = `[`CommitRecovery`]: Rebuild the KZG commitment. Useful for 
        
           //!    larger sub-slices of `payload` such as a complete namespace. 
        
           //!    Snark-friendly because it does not require a pairing.

Change VidScheme to use &[u8] for payload type as per offline discussion.
move bytes_to_field from conversion.rs to a new bytes_to_field.rs file owned by advz.

Excerpts from original description

change the bytes-to-field conversion in conversion.rs. It's much simpler now because I decided we don't need to add metadata into the output field elements. Instead, it's the responsibility of the caller to know where exactly their bytes begin/end in the encoded field elements.
advz.rs has a large diff but it's mostly just moving code around.
fix a bug in encoding payload bytes to polynomials wherein FFT and inverse-FFT were accidentally swapped.
fix some nix stuff

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (main)
Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
Wrote unit tests
Updated relevant documentation in the code
Added a relevant changelog entry to the Pending section in CHANGELOG.md
Re-reviewed Files changed in the GitHub PR explorer

primitives/src/vid/namespace.rs

primitives/src/vid.rs

primitives/src/vid/advz.rs

primitives/src/vid/advz/namespace.rs

…, github CI does not)

…duce noise in output

…ll/389/files/0d9f70443d98ac78a6c398ff0e7a9cf3ba05592c#r1377950064

primitives/src/vid/payload_prover.rs

primitives/src/vid/advz/payload_prover.rs

primitives/src/pcs/univariate_kzg/mod.rs

primitives/src/vid/advz.rs

primitives/src/vid/advz/payload_prover.rs

chancharles92

LGTM, can approve after addressing Jeb's concern about APIs.

…omment)

jbearer

Looks good. I think we might be missing some important traits on the proof types, particularly serdes. But if you want we can get this merged and add those later. Or add them now. Either way

primitives/src/vid/advz/payload_prover.rs

jbearer · 2023-11-07T13:54:17Z

primitives/src/vid/advz/payload_prover.rs

+/// KZG batch proofs and accompanying metadata.
+///
+/// TODO use batch proof instead of `Vec<P>` <https://github.com/EspressoSystems/jellyfish/issues/387>
+pub struct SmallRangeProof<P> {


The proof types should at least derive Clone, Debug, and serde traits

Definitely true. I'm going to merge now anyway because (i) this PR is already too big, (ii) don't want to initiate another round of approval, (iii) there will certainly be back-and-forth over issues like this as we integrate into the sequencer.

primitives/src/vid/payload_prover.rs

* update comments as per #389 (comment) * add commit_only_timer test * tidy: idiomatic construction of polys * remove P: PolynomialCommitmentScheme from GenericAdvz * remove V: MerkleTreeScheme from GenericAdvz * remove type alias Advz, rename GenericAdvz -> Advz * remove some unneeded trait bounds * enforce only H: HasherDigest bound * remove old comments, tidy derivations * fmt * type aliases for kzg * type aliases for MerkleTreeScheme * more use of type aliases * fix rustdoc

ggutoski requested review from chancharles92 and jbearer October 30, 2023 23:51

jbearer reviewed Oct 31, 2023

View reviewed changes

primitives/src/vid/namespace.rs Outdated Show resolved Hide resolved

primitives/src/vid/namespace.rs Outdated Show resolved Hide resolved

primitives/src/vid/namespace.rs Outdated Show resolved Hide resolved

chancharles92 reviewed Oct 31, 2023

View reviewed changes

primitives/src/vid/advz/namespace.rs Outdated Show resolved Hide resolved

ggutoski added 25 commits November 1, 2023 13:33

restore simple bytes-to-field from 03d7d87

9ca622e

rename vid/mod.rs -> vid.rs

8f0d7f5

new assoc type VidScheme::Payload instead of Vec<u8>

f25b9e1

tidy: move boilerplate to the bottom

20ac68f

new Namespacer trait skeleton

a1070c9

wip: untested impl for namespace_verify

ddccddb

port prove_namespace test to advz/namespaces.rs

f146fd2

wip: untested implementation of data_proof

af98cb6

fix: use ifft instead of fft, debugging code for posterity

75011f6

remove debugging code

efc81c5

fix bug in modulus_byte_len, should refactor later

2e65705

test for data_proof

b8f1505

impl data_verify, fix bug fft -> ifft

29187f5

refactor polynomial construction

2cc8acb

refactor computation of block hash from polynomial commitments

26bb9ec

delete superfluous modulus_byte_len

2453404

tidy index conversion utils

c753718

tidy

08a536e

fix test build (my vscode settings has std feature enabled by default…

7d36361

…, github CI does not)

update changelog (is this always worthwhile?)

293ed1a

remove broken rustdoc links

96e8f87

add timer data to VID disperse, remove it temmporarily from KZG to re…

4dd89f4

…duce noise in output

remove namespace_verify, add chunk_prove, chunk_verify

58feac5

wip: stubs

daabb28

new chunk_proof_2 use Range instead of start, len

cc5e3c6

add commit arg to verify_share()

ff764b4

ggutoski mentioned this pull request Nov 6, 2023

VID namespace verify allow for precomputation #397

Closed

ggutoski added 2 commits November 6, 2023 13:15

add issue comment for https://github.com/EspressoSystems/jellyfish/pu…

89c9fea

…ll/389/files/0d9f70443d98ac78a6c398ff0e7a9cf3ba05592c#r1377950064

clippy and fmt

93e396e

ggutoski force-pushed the gg/vid-namespace branch from 0d9f704 to 93e396e Compare November 6, 2023 18:19

ggutoski added 3 commits November 6, 2023 13:31

clarify rustdoc

254c2e5

fix bench

beeaea9

move bytes_to_field, etc from conversion.rs to new bytes_to_field.rs

39d16e8

jbearer reviewed Nov 6, 2023

View reviewed changes

primitives/src/vid/payload_prover.rs Outdated Show resolved Hide resolved

primitives/src/vid/advz/payload_prover.rs Show resolved Hide resolved

primitives/src/vid/advz/payload_prover.rs Outdated Show resolved Hide resolved

ggutoski added 2 commits November 6, 2023 14:24

replace compile_time_checks with new elem_byte_capacity owned by advz

4b3a806

fmt and doctest

7dcb84f

chancharles92 reviewed Nov 6, 2023

View reviewed changes

primitives/src/pcs/univariate_kzg/mod.rs Outdated Show resolved Hide resolved

primitives/src/vid/advz.rs Outdated Show resolved Hide resolved

primitives/src/vid/advz/payload_prover.rs Show resolved Hide resolved

ggutoski added 4 commits November 6, 2023 15:26

timer tidy as per #389 (comment)

a067833

fix rustdoc

b7e1480

fmt

68453c6

Merge branch 'main' into gg/vid-namespace

613a097

chancharles92 reviewed Nov 6, 2023

View reviewed changes

ggutoski added 4 commits November 6, 2023 19:52

new Statement type to simplify args for payload_verify as per #389 (c…

f399a4f

…omment)

fmt

4805bfb

Merge branch 'main' into gg/vid-namespace

d51f76e

rename proof types, tidy, comment

eb8fde4

ggutoski requested review from jbearer and chancharles92 November 7, 2023 02:44

jbearer approved these changes Nov 7, 2023

View reviewed changes

ggutoski merged commit 11c7829 into main Nov 7, 2023
3 checks passed

ggutoski deleted the gg/vid-namespace branch November 7, 2023 16:58

ggutoski added a commit that referenced this pull request Nov 9, 2023

update comments as per #389 (comment)

d404dba

ggutoski mentioned this pull request Nov 21, 2023

Traits for PayloadProver proof types #419

Closed

ggutoski mentioned this pull request Dec 5, 2023

feat: add tx table entry proofs to tx payload proof EspressoSystems/espresso-sequencer#796

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hello-world namespace support for ADVZ VID scheme #389

feat: hello-world namespace support for ADVZ VID scheme #389

ggutoski commented Oct 30, 2023 •

edited

Loading

chancharles92 left a comment

jbearer left a comment

jbearer Nov 7, 2023

ggutoski Nov 7, 2023

	//! Two implementations:
	//! 1. `PROOF = `[`Proof`]: KZG batch proof for the data. Useful for small
	//! sub-slices of `payload` such as an individual transaction within a block.
	//! Not snark-friendly because it requires a pairing.
	//! 2. `PROOF = `[`CommitRecovery`]: Rebuild the KZG commitment. Useful for
	//! larger sub-slices of `payload` such as a complete namespace.
	//! Snark-friendly because it does not require a pairing.

feat: hello-world namespace support for ADVZ VID scheme #389

feat: hello-world namespace support for ADVZ VID scheme #389

Conversation

ggutoski commented Oct 30, 2023 • edited Loading

Revised 2023-11-06

Excerpts from original description

chancharles92 left a comment

Choose a reason for hiding this comment

jbearer left a comment

Choose a reason for hiding this comment

jbearer Nov 7, 2023

Choose a reason for hiding this comment

ggutoski Nov 7, 2023

Choose a reason for hiding this comment

ggutoski commented Oct 30, 2023 •

edited

Loading