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

feat: add consolidated block hashes across checkpoints #551

Merged
merged 21 commits into from
Sep 12, 2024

Conversation

Nashtare
Copy link
Collaborator

@Nashtare Nashtare commented Aug 27, 2024

Built on top of #526

Adds additional connection across checkpoints for the previous block hashes.

Notes:

  • The switch Field -> RichField is also necessary for the type2 anyway, so harmless in this regard.
  • I'll probably refactor in a follow-up PR the types being used across so that higher crates do not need to care about which Hasher / Field we're using, and keep the information at the level of evm_arithmetization, while also doing part of move constants from evm_arithmetization to common #531 which is more widespread than I thought.

@Nashtare Nashtare self-assigned this Aug 27, 2024
@github-actions github-actions bot added crate: trace_decoder Anything related to the trace_decoder crate. crate: proof_gen Anything related to the proof_gen crate. crate: evm_arithmetization Anything related to the evm_arithmetization crate. crate: zero_bin Anything related to the zero-bin subcrates. labels Aug 27, 2024
@Nashtare Nashtare changed the title feat: consolidated hashes feat: add consolidated block hashes across checkpoints Aug 27, 2024
Copy link
Contributor

@0xaatif 0xaatif left a comment

Choose a reason for hiding this comment

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

lgtm from trace_decoder's pov

trace_decoder/src/lib.rs Show resolved Hide resolved
use processed_block_trace::ProcessedTxnInfo;
use serde::{Deserialize, Serialize};
use typed_mpt::{StateTrie, StorageTrie, TrieKey};

/// The base field on which statements are being proven.
// TODO(Robin): https://github.com/0xPolygonZero/zk_evm/issues/531
pub type Field = GoldilocksField;
Copy link
Contributor

Choose a reason for hiding this comment

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

very nit/style:

I think this could be confusing because it's name collides with a type parameter name:

type Inner = u8;
struct Foo<Inner>(Inner);

When I see Foo<Inner>, is it like Vec<u8> or Vec<T>.

I prefer to use T as a suffix for actually descriptive type parameters.

type Field = GoldilocksField.
struct Foo<FieldT>(FieldT);

zero_bin/rpc/src/lib.rs Outdated Show resolved Hide resolved
@Nashtare Nashtare force-pushed the feat/consolidated_hashes branch from ddb13ea to 5a12444 Compare August 27, 2024 23:39
@Nashtare Nashtare added this to the Type 1 - Q3 2024 milestone Aug 27, 2024
@Nashtare Nashtare force-pushed the feat/consolidated_hashes branch from 46b9656 to 9207ff0 Compare August 28, 2024 12:20
Base automatically changed from feat/prune_values to develop August 29, 2024 15:35
Copy link
Contributor

@hratoanina hratoanina left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@@ -368,10 +450,13 @@ impl BlockMetadata {

/// Additional block data that are specific to the local transaction being
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: they aren't all local to the transaction anymore.

@Nashtare Nashtare merged commit 4ef831b into develop Sep 12, 2024
18 checks passed
@Nashtare Nashtare deleted the feat/consolidated_hashes branch September 12, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate. crate: proof_gen Anything related to the proof_gen crate. crate: trace_decoder Anything related to the trace_decoder crate. crate: zero_bin Anything related to the zero-bin subcrates.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants