-
Notifications
You must be signed in to change notification settings - Fork 184
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: prover escrow and 712-signed quotes #8877
Conversation
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
|
||
function hash(EpochProofQuote memory quote) internal pure returns (bytes32) { | ||
return keccak256( | ||
abi.encode( |
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.
As mentioned in the other. I think the domain separator is missing here. Consult ERC20Permit
for examples of how it can be used.
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.
The domain separator is injected when the rollup contract computes the digest.
l1-contracts/test/prover-coordination/ProofCommitmentEscrow.t.sol
Outdated
Show resolved
Hide resolved
l1-contracts/test/prover-coordination/ProofCommitmentEscrow.t.sol
Outdated
Show resolved
Hide resolved
|
||
import { type TypedDataDomain, domainSeparator } from 'viem'; | ||
|
||
export const DOMAIN: TypedDataDomain = { |
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.
Similar to what was mentioned in the EIP712 lib I want the address
and chainid
in here
yarn-project/circuit-types/src/prover_coordination/epoch_proof_quote_payload.ts
Outdated
Show resolved
Hide resolved
…provers. The implementation allows the rollup contract to stake deposited bonds, and unstake them. Provers may initiate withdraws which are not executable until 3 epochs after their initiation. Proposers will use the minBalanceAtTime function to ensure that a prover has sufficient funds at the slot they would "cash in" the quote. Additionally, the signatures used on the epoch proof quotes have been updated to use EIP 712 style signing and verification.
1450cd1
to
1cadafe
Compare
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
Docs PreviewHey there! 👋 You can check your preview at https://66fc11560c21d75b594dea67--aztec-docs-dev.netlify.app |
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.
Really minor comment, is acceptable but think we need to cleanup natspecs and some of the modifiers since we can simplify it slightly.
@@ -32,6 +32,7 @@ export class EpochProofQuote extends Gossipable { | |||
return new EpochProofQuote(reader.readObject(EpochProofQuotePayload), reader.readObject(Signature)); | |||
} | |||
|
|||
// TODO: https://github.com/AztecProtocol/aztec-packages/issues/8911 |
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.
🫡
@@ -103,9 +97,8 @@ contract ProofCommitmentEscrow is IProofCommitmentEscrow { | |||
* The prover must have sufficient balance | |||
* The prover's balance will be reduced by the bond amount | |||
*/ | |||
function stakeBond(uint256 _amount, address _prover) external override onlyRollup { | |||
function stakeBond(address _prover, uint256 _amount) external override onlyRollup { |
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.
Natspec should have the params etc. But think we are generally missing some of that. So seems like something that could be acceptable for now, and then lets create a big ass "update natspec" issue.
Created #8912
TOKEN = new TestERC20(); | ||
ESCROW = new ProofCommitmentEscrow(TOKEN, address(this)); | ||
} | ||
|
||
function testDeposit() public setupWithApproval(address(42), 100) { |
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.
Slightly annoying here, but if the input is always the _prover
and it will be the prover
value it don't seem useful to provide as input. Also, if always having the same inputs might as well remove them from here.
Fix #8573
Introduces an escrow contract that is able to store test tokens from provers.
The implementation allows the rollup contract to stake deposited bonds, and unstake them.
Provers may initiate withdraws which are not executable until 3 epochs after their initiation.
Proposers will use the
minBalanceAtTime
function to ensure that a prover has sufficient funds at the slot they would "cash in" the quote.Additionally, the signatures used on the epoch proof quotes have been updated to use EIP 712 style signing and verification as part of the proof claim process.
Other cleanup includes:
Future work includes: