-
Notifications
You must be signed in to change notification settings - Fork 4
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
ED-4619: Apply L3 Proving changes for ROUTES-1-6-L3 #143
Conversation
@@ -1,6 +1,6 @@ | |||
/* -*- c-basic-offset: 4 -*- */ | |||
// SPDX-License-Identifier: MIT | |||
pragma solidity ^0.8.26; | |||
pragma solidity ^0.8.28; |
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.
not relevant to your PR but i think we should combine IProver and ISimpleProver
// Output slot for the game status (aaaaa) | ||
uint256 internal constant L2_FAULT_DISPUTE_GAME_STATUS_SLOT = 0; | ||
|
||
struct FaultDisputeGameStatusSlotData { |
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.
im overthinking this but how do you feel about the structs being in here? the pure functions for sure absolutely make sense, but the structs to me look a bit odd in a library. If you wanna check out what I did for the Intents type I think that makes the most sense, I stole it from uniswap - kinda makes a file of structs that can be imported individually. But also this is totally fine
} | ||
|
||
proveStorageBytes32( | ||
require( |
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.
custom errors
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.
could put this in the prover library or even a more general utils library since its just about encoding
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.
decent amount of cleanup work, and some reorganization, but mostly looks good
No description provided.