-
Notifications
You must be signed in to change notification settings - Fork 40
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
Conversation
There was a problem hiding this 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
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; |
There was a problem hiding this comment.
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);
ddb13ea
to
5a12444
Compare
46b9656
to
9207ff0
Compare
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
Built on top of #526
Adds additional connection across checkpoints for the previous block hashes.
Notes:
Field
->RichField
is also necessary for the type2 anyway, so harmless in this regard.Hasher
/Field
we're using, and keep the information at the level ofevm_arithmetization
, while also doing part of move constants fromevm_arithmetization
tocommon
#531 which is more widespread than I thought.