Whisk (SSLE) with Curdleproofs - rebased #3342

dapplion · 2023-04-30T02:44:09Z

Rebase of [WIP] Introduce consensus code for Whisk (SSLE) with Curdleproofs #3205

Preserved original commits for context and history, since they contain relevant remarks and design decisions.

In detail the branch dapplion:consensus-specs:whisk-curdleproofs process is:

git checkout nalinbhardwaj:whisk-curdleproofs
git rebase ethereum:dev

Resolve conflicts manually, dropping all changes on these files

configs/mainnet.yaml
configs/minimal.yaml
tests/core/pyspec/eth2spec/merge/__init__.py
tests/core/pyspec/eth2spec/merge/mainnet.py
tests/core/pyspec/eth2spec/merge/minimal.py
tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py
tests/core/pyspec/eth2spec/test/context.py
tests/core/pyspec/eth2spec/test/helpers/constants.py
tests/core/pyspec/eth2spec/test/helpers/genesis.py
tests/core/pyspec/eth2spec/test/whisk/__init__.py
tests/core/pyspec/eth2spec/test/whisk/unittests/__init__.py

Two final commits to move whisk spec to _features and remove dummy test.

Next steps

@hwwhww should I also drop changes to setup.py? We can merge the spec not executable first, and make executable latter. In parallel propose improvements to nalinbhardwaj:whisk-curdleproofs and add spec tests

hwwhww

Thanks @dapplion!

should I also drop changes to setup.py? We can merge the spec not executable first, and make executable latter. In parallel propose improvements to nalinbhardwaj:whisk-curdleproofs and add spec tests

I think given the complexity of this feature, it's good to have some basic reviews now since we can only review it line by line in this PR.

Therefore, I pushed some fixes based on the linter errors. The only remaining linter error is "undefined name 'WhiskTrackerProof'".

I think for this PR, the goals can be set to (i) make linter pass and (ii) review the whisk logic. 👍 on adding tests in later PRs.

specs/_features/whisk/beacon-chain.md

hwwhww · 2023-05-01T09:32:05Z

curdleproofs package requires Python version >=Python3.9. IMO we can merge #2964 first.

asn-d6 · 2023-05-02T14:45:33Z

Thanks @dapplion!

should I also drop changes to setup.py? We can merge the spec not executable first, and make executable latter. In parallel propose improvements to nalinbhardwaj:whisk-curdleproofs and add spec tests

I think given the complexity of this feature, it's good to have some basic reviews now since we can only review it line by line in this PR.

Therefore, I pushed some fixes based on the linter errors. The only remaining linter error is "undefined name 'WhiskTrackerProof'".

I think for this PR, the goals can be set to (i) make linter pass and (ii) review the whisk logic. +1 on adding tests in later PRs.

ACK. Makes sense. Will do a review here either this or next week.

tests/core/pyspec/eth2spec/test/whisk/unittests/test_whisk.py

asn-d6 · 2023-05-03T09:25:38Z

curdleproofs package requires Python version >=Python3.9. IMO we can merge #2964 first.

This is now merged. I guess if we rebase this on top, the tests will not fail anymore?

@asn-d6

@asn-d6: As discussed, I fixed a few bugs along the way but likely also introduced some bugs :)

This PR changes the Whisk tracker format to be of the form `(r * pubkey, r * BLS_GT_GENERATOR)` instead of `(r * k * BLS_G1_GENERATOR, r * BLS_G1_GENERATOR)`. This allows for non-interactive tracker registrations from validator pubkeys, removing ~50 lines the code. It also significantly reduces the amount of state overhead. This PR also removes permutation commitments, though those can be easily readded if deemed necessary.

@asn-d6

@asn-d6: Readded a consistency check for `IsValidWhiskOpeningProof` (involving `pubkey` instead of `k_commitment`).

This needs its own ethresearch post, and some additional analysis to see if we can do the shuffle ZKP in the allowed timeframe. This reverts commit 8517aca.

Ditch the Feistel logic and instead have each shuffler pick the row they shuffle using their RANDAO reveal.

This commit further simplifies 0faef30 by removing the entire squareshuffle. The latest version of https://eprint.iacr.org/2022/560 proposes that each shuffler picks random indices from the entire candidate set instead of organizing validators into a square.

…nter happier.

dapplion · 2023-05-05T03:02:17Z

Lint should pass now, recent changes:

Declared custom types WhiskShuffleProof WhiskTrackerProof d721e3a
Ignore pylint rules for unimplemented crypto 7d509c1
Fix types multiple functions f11e9df
Implement initial k generation for fork.md f11e9df

specs/_features/whisk/beacon-chain.md

asn-d6

Thanks for the updates @dapplion . LGTM!

hwwhww

Some minor suggestions.

It's good to merge this alpha version. Thank you @dapplion!

specs/_features/whisk/beacon-chain.md

hwwhww · 2023-05-16T13:58:45Z

specs/_features/whisk/beacon-chain.md

+def is_k_commitment_unique(state: BeaconState, k_commitment: BLSG1Point) -> bool:
+    return all([validator.whisk_k_commitment != k_commitment for validator in state.validators])


This function looks scary.

scary due to its syntax, efficiency or else? If the former should I use a for loop?

Agreed. Let's use a for loop?

Apologies for the confusion. I was scared by the efficiency. IIUC Python all() is like for loop implementation, it returns False immediately once it finds validator.whisk_k_commitment == k_commitment. But it scares me as it is used in the while True of get_unique_whisk_k.

Regarding efficiency: Yes on a deposit this requires a full loop over the validator list. But a naive implementation of the beacon chain already requires that to check if the pubkey of a deposit has been seen before or not. This addition will require clients to mantain cached set of k_commitment to avoid that loop if they choose. AFAIK spec should not be concerned with computational efficiency but correctness, and the current whisk spec new k_commitment to be unique.

Regarding syntax: the all syntax is pretty readable, I can switch to a for loop, but the current implementation is simpler IMO

Indeed, the efficiency hit here can be salvaged using a hash map of all k_commitments. When writing the spec here with @JustinDrake, we tried to focus on correctness/readability over optimizations, hence the loop here.

specs/_features/whisk/beacon-chain.md

@asn-d6

* Introduce consensus code for Whisk * polish, simplify, clean up (~100 fewer lines) @asn-d6: As discussed, I fixed a few bugs along the way but likely also introduced some bugs :) * minor cleanups and fixes * simplify is_k_commitment_unique * Update beacon-chain.md * Update beacon-chain.md * Initialize `k` in `get_validator_from_deposit()` * minor cleanups * Update beacon-chain.md * Create beacon-chain.md This PR changes the Whisk tracker format to be of the form `(r * pubkey, r * BLS_GT_GENERATOR)` instead of `(r * k * BLS_G1_GENERATOR, r * BLS_G1_GENERATOR)`. This allows for non-interactive tracker registrations from validator pubkeys, removing ~50 lines the code. It also significantly reduces the amount of state overhead. This PR also removes permutation commitments, though those can be easily readded if deemed necessary. * A couple of fixes to the no-registration simplification @asn-d6: Readded a consistency check for `IsValidWhiskOpeningProof` (involving `pubkey` instead of `k_commitment`). * remove unused helpers * use Mary's suggested tracker * Update beacon-chain.md * Revert G_t element optimization This needs its own ethresearch post, and some additional analysis to see if we can do the shuffle ZKP in the allowed timeframe. This reverts commit 8517aca. * Implement new shuffling strategy Ditch the Feistel logic and instead have each shuffler pick the row they shuffle using their RANDAO reveal. * Curdleproofs edits * working whisk eth2spec * working whisk dummy test * add more boilerplate set up code * rebase constants * Implement even newer and simplified shuffling strategy This commit further simplifies 0faef30 by removing the entire squareshuffle. The latest version of https://eprint.iacr.org/2022/560 proposes that each shuffler picks random indices from the entire candidate set instead of organizing validators into a square. * Move to _features * remove dummy test * Run doctoc * Change Whisk's previous fork to Capella instead of Bellatrix. Make linter happier. * Fix lint * Fix pylint * Fix mypy issues * Clean-up get_beacon_proposer_index * Fix doc headers * Fix capella link * Update apply_deposit * Rename process_shuffled_trackers --------- Co-authored-by: George Kadianakis <desnacked@riseup.net> Co-authored-by: Justin <drakefjustin@gmail.com> Co-authored-by: Nalin Bhardwaj <nalinbhardwaj@nibnalin.me> Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>

dapplion changed the title ~~Whisk curdleproofs~~ Whisk (SSLE) with Curdleproofs - rebased Apr 30, 2023

hwwhww reviewed May 1, 2023

View reviewed changes

specs/_features/whisk/beacon-chain.md Show resolved Hide resolved

asn-d6 reviewed May 2, 2023

View reviewed changes

tests/core/pyspec/eth2spec/test/whisk/unittests/test_whisk.py Outdated Show resolved Hide resolved

asn-d6 and others added 24 commits May 5, 2023 10:23

Introduce consensus code for Whisk

cc4a19c

polish, simplify, clean up (~100 fewer lines)

a5de25d

@asn-d6: As discussed, I fixed a few bugs along the way but likely also introduced some bugs :)

minor cleanups and fixes

4ef8b62

simplify is_k_commitment_unique

4d43ea1

Update beacon-chain.md

b45ea2f

Update beacon-chain.md

99f8fbd

Initialize k in get_validator_from_deposit()

478a092

minor cleanups

365d1f2

Update beacon-chain.md

c7666dc

A couple of fixes to the no-registration simplification

683686c

@asn-d6: Readded a consistency check for `IsValidWhiskOpeningProof` (involving `pubkey` instead of `k_commitment`).

remove unused helpers

271ee32

use Mary's suggested tracker

ab53b9b

Update beacon-chain.md

ff37625

Revert G_t element optimization

3bc83a0

This needs its own ethresearch post, and some additional analysis to see if we can do the shuffle ZKP in the allowed timeframe. This reverts commit 8517aca.

Implement new shuffling strategy

75d5372

Ditch the Feistel logic and instead have each shuffler pick the row they shuffle using their RANDAO reveal.

Curdleproofs edits

7cbe2b8

working whisk eth2spec

1a603b1

working whisk dummy test

e163e6a

add more boilerplate set up code

6a134b9

rebase constants

8ce736c

Move to _features

c304d64

remove dummy test

e2f6f09

dapplion and others added 2 commits May 5, 2023 10:24

Run doctoc

5a3e046

Change Whisk's previous fork to Capella instead of Bellatrix. Make li…

670845b

…nter happier.

dapplion force-pushed the whisk-curdleproofs branch from 9e68294 to 670845b Compare May 5, 2023 01:24

dapplion added 3 commits May 5, 2023 11:05

Fix lint

d721e3a

Fix pylint

7d509c1

Fix mypy issues

f11e9df

asn-d6 reviewed May 9, 2023

View reviewed changes

specs/_features/whisk/beacon-chain.md Outdated Show resolved Hide resolved

dapplion added 2 commits May 10, 2023 20:02

Clean-up get_beacon_proposer_index

8789a67

Fix doc headers

e9445bb

dapplion force-pushed the whisk-curdleproofs branch from 2ef68e2 to e9445bb Compare May 10, 2023 11:07

asn-d6 approved these changes May 15, 2023

View reviewed changes

hwwhww approved these changes May 16, 2023

View reviewed changes

dapplion added 2 commits May 17, 2023 14:04

Fix capella link

9a412d9

Update apply_deposit

45fce04

dapplion force-pushed the whisk-curdleproofs branch from 1ea91d4 to 45fce04 Compare May 18, 2023 16:33

Rename process_shuffled_trackers

35fe953

hwwhww merged commit 241e52a into ethereum:dev Jun 8, 2023

hwwhww added the general:enhancement New feature or request label Jun 8, 2023

This was referenced Jun 8, 2023

Whisk: Move validator whisk trackers and commitments to state #3407

Merged

Whisk: move whisk_opening_proof to block body #3408

Merged

hwwhww mentioned this pull request Jun 8, 2023

[WIP] Introduce consensus code for Whisk (SSLE) with Curdleproofs #3205

Closed

dapplion mentioned this pull request Jul 5, 2023

Add WHISK fork only tests #3442

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Whisk (SSLE) with Curdleproofs - rebased #3342

Whisk (SSLE) with Curdleproofs - rebased #3342

dapplion commented Apr 30, 2023

hwwhww left a comment

hwwhww commented May 1, 2023

asn-d6 commented May 2, 2023

asn-d6 commented May 3, 2023

dapplion commented May 5, 2023

asn-d6 left a comment

hwwhww left a comment

hwwhww May 16, 2023

dapplion May 17, 2023

asn-d6 May 17, 2023

hwwhww May 18, 2023

dapplion May 18, 2023

asn-d6 May 22, 2023

		def is_k_commitment_unique(state: BeaconState, k_commitment: BLSG1Point) -> bool:
		return all([validator.whisk_k_commitment != k_commitment for validator in state.validators])

Whisk (SSLE) with Curdleproofs - rebased #3342

Whisk (SSLE) with Curdleproofs - rebased #3342

Conversation

dapplion commented Apr 30, 2023

hwwhww left a comment

Choose a reason for hiding this comment

hwwhww commented May 1, 2023

asn-d6 commented May 2, 2023

asn-d6 commented May 3, 2023

dapplion commented May 5, 2023

asn-d6 left a comment

Choose a reason for hiding this comment

hwwhww left a comment

Choose a reason for hiding this comment

hwwhww May 16, 2023

Choose a reason for hiding this comment

dapplion May 17, 2023

Choose a reason for hiding this comment

asn-d6 May 17, 2023

Choose a reason for hiding this comment

hwwhww May 18, 2023

Choose a reason for hiding this comment

dapplion May 18, 2023

Choose a reason for hiding this comment

asn-d6 May 22, 2023

Choose a reason for hiding this comment