refactor: static stake table simplication, simpler LightClientState #1939

alysiahuggins · 2024-08-29T00:31:06Z

… finalized state

Closes #1938

This PR:

removes states mapping
added global variable for genesisState, genesisStakeState and finalizedState
removes fields from the LightClientState and adds a new struct StakeState
WIP

This PR does not:

Key places to review:

… finalized state

…enesis-state-struct-and-remove-states-mapping

Initial attempt to fix compilation errors.

philippecamacho · 2024-08-31T17:01:29Z

Made some progress fixing the compilation errors but not there yet:

Created a new branch of HotShot (fork of tag = "0.5.72") and added the StakeState struct.
Have the espresso-sequencer repository point to this branch
Fixed several compilation errors.

Right now, for some reason the espresso-types crates does not compile.

cc @alysiahuggins @alxiong

…branch, lc-contract-updates and ensure that all references to the StakeState compiles

alysiahuggins · 2024-09-03T18:51:33Z

the branch, lc-contract-updates, has been added to the following repos so that we can support the new types introduced by this ticket by pointing all repos that rely on hotshot to that branch d391f7e:

…he process of updating the related adapte and hotshot-state-prover code e.g. the MockGenesis action

alysiahuggins · 2024-09-04T02:41:34Z

removed fields from the LightClientState struct so it now looks like this

struct LightClientState {
        uint64 viewNum;
        uint64 blockHeight;
        BN254.ScalarField blockCommRoot;
    }

updated related rust code such as the lightclient adapter and MockGenesis action in the diff-tests, to continue, one would have to also update the MockConsecutiveFinalizedStates, MockSkipBlocks, MockMissEndingBlock etc

3a27dda
this git commit ignored verification checks so that anyone can pick up from where i left off.

contracts/src/LightClient.sol

alxiong

I've looked at adapter/*, diff-test , hotshot-state-prover
they all look very good to me. Alysia is doing great work. 👏 (chore, but she does them with precision)

I have addressed all of my own comments in #1986

contracts/rust/adapter/src/light_client.rs

hotshot-state-prover/src/mock_ledger.rs

hotshot-state-prover/src/circuit.rs

hotshot-state-prover/src/service.rs

alxiong · 2024-09-04T15:15:53Z

@mrain can you also take a look at Alysia's changes to the circuit.rs. The changes look good to me at least.

for reference, we are simplifying light client state as specified in #1938

thanks!

mrain · 2024-09-04T15:21:17Z

@mrain can you also take a look at Alysia's changes to the circuit.rs. The changes look good to me at least.

for reference, we are simplifying light client state as specified in #1938

thanks!

LGTM. Changes are simple enough.

philippecamacho · 2024-09-04T18:54:40Z

contracts/rust/diff-test/src/main.rs

+                    schnorr_key_comm: field_to_u256(stt.stake_table_schnorr_key_comm),
+                    amount_comm: field_to_u256(stt.stake_table_amount_comm),
+                };
+                stake_stakes.push(parsed_stake_state.into());


Looks like stake_stakes vector is not returned. Why is that?

this will be removed in my future commit of mine

removed in af51261

…ude the new stake state struct, not all tests pass and the proofs need to be assessed more carefully

…lient-contract-create-genesis-state-struct-and-remove-states-mapping

alxiong · 2024-09-05T09:57:20Z

hotshot-state-prover/src/circuit.rs

-                circuit.create_public_variable(state.fee_ledger_comm)?,
-                circuit.create_public_variable(state.stake_table_comm.0)?,
-                circuit.create_public_variable(state.stake_table_comm.1)?,
-                circuit.create_public_variable(state.stake_table_comm.2)?,


ah this is wrong actually. 🤦
(i missed it yesterday when I reviewed it, and wasted some time today assuming our Public input is reduced to 4)

we should still keep the stake table commitments in the public inputs for security/integrity!!

but we can safely remove fee_ledger_comm, so PI.length is 7 now.

cc @mrain @alysiahuggins

ah, I need to fix HotShot side as well for the struct declaration.

fixed in ab81b0b

alxiong · 2024-09-05T12:54:37Z

hotshot-state-prover/src/circuit.rs

-    let stake_amount_comm = RescueNativeGadget::<F>::rescue_sponge_with_padding(
+    let _stake_amount_comm = RescueNativeGadget::<F>::rescue_sponge_with_padding(
        &mut circuit,
        &stake_amount_preimage_vars,
        1,
    )?[0];
-    circuit.enforce_equal(
-        stake_amount_comm,
-        lightclient_state_pub_var
-            .stake_table_comm()
-            .stake_amount_comm,
-    )?;


realized that these are wrong, we need these to ensure security.

sry that I miss it during my last review.

(p.s. I'm fixing these locally, will push soon)

fixed in ab81b0b

… not upgraded

…e-struct-and-remove-states-mapping' into 1938-update-state-prover

* update PublicInput length from 8 to 4 * Revert "update PublicInput length from 8 to 4" This reverts commit 40e2c67. * update PublicInput length from 8 to 7. --------- Co-authored-by: Philippe Camacho <[email protected]>

…enesis-state-struct-and-remove-states-mapping

…nt-contract-create-genesis-state-struct-and-remove-states-mapping

Another attempt to update hotshot version to `rc-0.5.74`, aims to supercede #1988

In EspressoSystems/espresso-sequencer#1939 the light client ABI changed and as a result we need to make changes in multiple places. The tracking issue for the changes in our nitro integration is https://github.com/EspressoSystems/nitro-contracts/issues/20

* CI: print docker logs on failure * Pin version before LC ABI changes In EspressoSystems/espresso-sequencer#1939 the light client ABI changed and as a result we need to make changes in multiple places. The tracking issue for the changes in our nitro integration is https://github.com/EspressoSystems/nitro-contracts/issues/20 * Pin the espresso-dev-node image

switch from states mapping to two state variables for the genesis and…

7cfa5ed

… finalized state

alysiahuggins linked an issue Aug 29, 2024 that may be closed by this pull request

simplify light client contract: create genesis state struct and remove states mapping #1938

Closed

alysiahuggins and others added 4 commits August 29, 2024 12:20

Merge branch 'main' into 1938-simplify-light-client-contract-create-g…

2544b26

…enesis-state-struct-and-remove-states-mapping

Merge branch 'main' into 1938-simplify-light-client-contract-create-g…

5bd55c4

…enesis-state-struct-and-remove-states-mapping

created separate stake struct for the genesis state

c70ae3e

Point to HotShot branch lc-contract-updates.

7c9db1c

Initial attempt to fix compilation errors.

ensure that all repos that depend on hotshot are running the feature …

d391f7e

…branch, lc-contract-updates and ensure that all references to the StakeState compiles

remove fields from light cilent state that are not needed and began t…

3a27dda

…he process of updating the related adapte and hotshot-state-prover code e.g. the MockGenesis action

alxiong reviewed Sep 4, 2024

View reviewed changes

contracts/src/LightClient.sol Outdated Show resolved Hide resolved

contracts/src/LightClient.sol Outdated Show resolved Hide resolved

contracts/src/LightClient.sol Outdated Show resolved Hide resolved

contracts/src/LightClient.sol Outdated Show resolved Hide resolved

alxiong reviewed Sep 4, 2024

View reviewed changes

philippecamacho reviewed Sep 4, 2024

View reviewed changes

alysiahuggins and others added 6 commits September 4, 2024 15:15

not verified: working on changes to circuit.rs to support new struct

b28fb7c

in the process of updating the circuit, ledger and snark code to incl…

6daee0e

…ude the new stake state struct, not all tests pass and the proofs need to be assessed more carefully

remove states mapping as it is no longer used

3f44170

remove getter functions for genesis and finalized states

56b88cd

allow --mock arg for gen-vk-contract; update vk

a7a3397

Merge remote-tracking branch 'origin/main' into 1938-simplify-light-c…

f5a811b

…lient-contract-create-genesis-state-struct-and-remove-states-mapping

alxiong reviewed Sep 5, 2024

View reviewed changes

alxiong and others added 6 commits September 6, 2024 01:02

rename: stake_state -> stake_table_state; remove stakeStable prefix

f7c5afa

wip: compile now

ab81b0b

removed unused variables and related function since the staketable is…

c63635e

… not upgraded

Merge branch '1938-simplify-light-client-contract-create-genesis-stat…

f869397

…e-struct-and-remove-states-mapping' into 1938-update-state-prover

state prover tests passing

af51261

alxiong added 4 commits September 6, 2024 18:09

LC contract test passing

8e0bf35

fix some mock-genesis usage

7f0ffe4

fix script for upgrade test failure

b38ab2d

update gas report

f1a92a6

alxiong mentioned this pull request Sep 6, 2024

update light client state prover #1986

Merged

philippecamacho and others added 4 commits September 6, 2024 12:02

Merge branch 'main' into 1938-simplify-light-client-contract-create-g…

b8131bb

…enesis-state-struct-and-remove-states-mapping

Merge branch 'main' into 1938-update-state-prover

9caf2f1

Merge branch '1938-update-state-prover' into 1938-simplify-light-clie…

70ed656

…nt-contract-create-genesis-state-struct-and-remove-states-mapping

updated to latest version of hotshot

8ee74da

alxiong mentioned this pull request Sep 7, 2024

[WEEKLY RELEASE] HotShot 0.5.74 #1988

Closed

alxiong and others added 6 commits September 7, 2024 11:44

point to new hotshot release

ee6c048

reove HotShotCommitments struct

d6df936

expaned on extended state struct in test

b524bd5

remove redundent permissionedProverEnabled flag

33098c6

new versions

44cb934

[WEEKLY RELEASE] HotShot 0.5.74 (#1989)

80b57e0

Another attempt to update hotshot version to `rc-0.5.74`, aims to supercede #1988

alxiong marked this pull request as ready for review September 9, 2024 15:24

alxiong requested review from nomaxg, ImJeremyHe, sveitser, jbearer, tbro and imabdulbasit as code owners September 9, 2024 15:24

alxiong changed the title ~~switch from states mapping to two state variables for the genesis and…~~ refactor: static stake table simplication, simpler LightClientState Sep 9, 2024

alxiong approved these changes Sep 9, 2024

View reviewed changes

alxiong enabled auto-merge September 9, 2024 15:29

alxiong merged commit 745d468 into main Sep 9, 2024
15 checks passed

alxiong deleted the 1938-simplify-light-client-contract-create-genesis-state-struct-and-remove-states-mapping branch September 9, 2024 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: static stake table simplication, simpler LightClientState #1939

refactor: static stake table simplication, simpler LightClientState #1939

alysiahuggins commented Aug 29, 2024 •

edited

Loading

philippecamacho commented Aug 31, 2024 •

edited

Loading

alysiahuggins commented Sep 3, 2024 •

edited

Loading

alysiahuggins commented Sep 4, 2024

alxiong left a comment •

edited

Loading

alxiong commented Sep 4, 2024

mrain commented Sep 4, 2024

philippecamacho Sep 4, 2024

alxiong Sep 5, 2024

alxiong Sep 6, 2024

alxiong Sep 5, 2024 •

edited

Loading

alxiong Sep 5, 2024

alxiong Sep 6, 2024

alxiong Sep 5, 2024 •

edited

Loading

alxiong Sep 6, 2024

refactor: static stake table simplication, simpler LightClientState #1939

refactor: static stake table simplication, simpler LightClientState #1939

Conversation

alysiahuggins commented Aug 29, 2024 • edited Loading

This PR:

This PR does not:

Key places to review:

philippecamacho commented Aug 31, 2024 • edited Loading

alysiahuggins commented Sep 3, 2024 • edited Loading

alysiahuggins commented Sep 4, 2024

alxiong left a comment • edited Loading

Choose a reason for hiding this comment

alxiong commented Sep 4, 2024

mrain commented Sep 4, 2024

philippecamacho Sep 4, 2024

Choose a reason for hiding this comment

alxiong Sep 5, 2024

Choose a reason for hiding this comment

alxiong Sep 6, 2024

Choose a reason for hiding this comment

alxiong Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

alxiong Sep 5, 2024

Choose a reason for hiding this comment

alxiong Sep 6, 2024

Choose a reason for hiding this comment

alxiong Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

alxiong Sep 6, 2024

Choose a reason for hiding this comment

alysiahuggins commented Aug 29, 2024 •

edited

Loading

philippecamacho commented Aug 31, 2024 •

edited

Loading

alysiahuggins commented Sep 3, 2024 •

edited

Loading

alxiong left a comment •

edited

Loading

alxiong Sep 5, 2024 •

edited

Loading

alxiong Sep 5, 2024 •

edited

Loading