feat: proofClaim in Rollup #8636

just-mitch · 2024-09-19T01:34:24Z

Add the proofClaim to the rollup.

Update the canPrune logic to account for it.

update pruning logic.

just-mitch · 2024-09-19T01:35:50Z

l1-contracts/src/core/Rollup.sol

  function _prune() internal {
+    delete proofClaim;


otherwise it will be there when we reorg

When reached in a call to prune (outside of proposals) this will incur higher cost than something like the approach we discussed with the forced inclusion. Here it is fix size, and should overall make it slightly cheaper when happening in the intended flow, e.g., as part of proposing.

just-mitch · 2024-09-19T01:36:18Z

l1-contracts/src/core/libraries/Errors.sol

@@ -44,23 +44,29 @@ library Errors {
  error Outbox__BlockNotProven(uint256 l2BlockNumber); // 0x0e194a6d

  // Rollup


sorted alphabetically

l1-contracts/test/Rollup.t.sol

LHerskind

Did not read through the full test, but here are some comments for now.

LHerskind · 2024-09-19T08:43:34Z

l1-contracts/src/core/Rollup.sol

@@ -53,6 +53,9 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
  // @todo  #8018
  uint256 public constant TIMELINESS_PROVING_IN_SLOTS = 100;

+  uint256 public constant CLAIM_DURATION_IN_L2_SLOTS = 13;
+  uint256 public constant PROOF_COMMITMENT_MIN_BOND_AMOUNT_IN_TST = 1000;


For the sake of sanity we should probably shove this into a constants file, since we will likely be needing it in the node as well, so having it in one place should help us a bit there.

LHerskind · 2024-09-19T08:46:59Z

l1-contracts/src/core/Rollup.sol

  function _prune() internal {
+    delete proofClaim;


When reached in a call to prune (outside of proposals) this will incur higher cost than something like the approach we discussed with the forced inclusion. Here it is fix size, and should overall make it slightly cheaper when happening in the intended flow, e.g., as part of proposing.

LHerskind · 2024-09-19T08:48:41Z

l1-contracts/src/core/Rollup.sol

@@ -156,6 +160,59 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
    vkTreeRoot = _vkTreeRoot;
  }

+  function claimEpochProofRight(DataStructures.EpochProofQuote calldata _quote)


I'm not sure I like the claim name, mainly as claiming usually is claiming rewards or the like, but here your claim is practically "reducing" your balance because of the stake.

Yeah it's a tricky thing to name, since the proposer is the one doing it, but the provers bond is the one getting staked. Open to suggestions.

LHerskind · 2024-09-19T08:56:28Z

l1-contracts/src/core/Rollup.sol

+    address currentProposer = getCurrentProposer();
+    uint256 epochToProve = getEpochToProve();
+
+    if (currentProposer != address(0) && currentProposer != msg.sender) {


If currentProposer == address(0) you don't have a committee, does it become weird with epochs then? At that point issue is essentially that sure you can have an epoch, but as anyone could propose into it, it seems likely that you would not end up being able to rely on it, so seems more likely that it would be the "proposeAndProof" at the same time similar to the fallback. Might just be the sanest to be in fallback if there is no committee actually.

Might just be the sanest to be in fallback if there is no committee actually.

I think I agree with this. We'll see how this needs to change in response when based fallback lands.

LHerskind · 2024-09-19T09:00:52Z

l1-contracts/src/core/Rollup.sol

+    }
+
+    if (currentSlot % Constants.AZTEC_EPOCH_DURATION >= CLAIM_DURATION_IN_L2_SLOTS) {
+      revert Errors.Rollup__NotInClaimPhase(


I think a more useful error message would likely be:

error Rollup__NotInClaimPhase(uint256 slotInEpoch, uint256 claimSlotsInEpoch);

Reasoning being that you then get information that is more useful for debugging without needing the figure out what the constant is. This way if you get 13, 13 you know instantly that it was sent too late and you know it must be within the first 13 slots so you were 1 off.

LHerskind · 2024-09-19T09:50:01Z

l1-contracts/src/core/Rollup.sol

@@ -53,6 +53,9 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
  // @todo  #8018
  uint256 public constant TIMELINESS_PROVING_IN_SLOTS = 100;


This one is kinda redundant with your stuff just below 🤷

LHerskind · 2024-09-19T11:18:33Z

l1-contracts/src/core/Rollup.sol

+      revert Errors.Rollup__QuoteExpired(currentSlot, _quote.validUntilSlot);
+    }
+
+    address bondProvider = PROOF_COMMITMENT_ESCROW.stakeBond(_quote);


You stake, but there won't ever be an unstake. Bye bye money. See comment in other pr.

I was waiting to unstake the bond until the proof of epoch landed.

I filed #8652.

l1-contracts/test/Rollup.t.sol

Fix #8608 Add the proofClaim to the rollup. Update the `canPrune` logic to account for it.

just-mitch added 2 commits September 18, 2024 15:52

add claimEpochProofRight to rollup.

f39c776

update pruning logic.

add tests and squash bugs

1ce624c

just-mitch requested review from LHerskind and Maddiaa0 September 19, 2024 01:34

just-mitch changed the title ~~Mt/8608-track-proofclaim-in-rollup~~ feat: proofClaim in Rollup Sep 19, 2024

just-mitch commented Sep 19, 2024

View reviewed changes

l1-contracts/test/Rollup.t.sol Outdated Show resolved Hide resolved

just-mitch mentioned this pull request Sep 19, 2024

feat: prune if needed #8617

Merged

LHerskind reviewed Sep 19, 2024

View reviewed changes

just-mitch added 2 commits September 19, 2024 12:24

break out tests

17e61d2

cleanup

f439c7a

just-mitch merged commit 77cdb19 into mt/8572-prune-if-needed Sep 20, 2024
46 checks passed

just-mitch deleted the mt/8608-track-proofclaim-in-rollup branch September 20, 2024 00:19

just-mitch added a commit that referenced this pull request Sep 20, 2024

feat: proofClaim in Rollup (#8636)

6b1a7ba

Fix #8608 Add the proofClaim to the rollup. Update the `canPrune` logic to account for it.

just-mitch added a commit that referenced this pull request Sep 27, 2024

feat: proofClaim in Rollup (#8636)

caad4ff

Fix #8608 Add the proofClaim to the rollup. Update the `canPrune` logic to account for it.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: proofClaim in Rollup #8636

feat: proofClaim in Rollup #8636

just-mitch commented Sep 19, 2024 •

edited

Loading

just-mitch Sep 19, 2024

LHerskind Sep 19, 2024

just-mitch Sep 19, 2024

just-mitch Sep 19, 2024

LHerskind left a comment

LHerskind Sep 19, 2024

LHerskind Sep 19, 2024

LHerskind Sep 19, 2024

just-mitch Sep 19, 2024

LHerskind Sep 19, 2024

just-mitch Sep 19, 2024

LHerskind Sep 19, 2024

LHerskind Sep 19, 2024

LHerskind Sep 19, 2024

just-mitch Sep 19, 2024

		@@ -44,23 +44,29 @@ library Errors {
		error Outbox__BlockNotProven(uint256 l2BlockNumber); // 0x0e194a6d

		// Rollup

		@@ -53,6 +53,9 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
		// @todo #8018
		uint256 public constant TIMELINESS_PROVING_IN_SLOTS = 100;

feat: proofClaim in Rollup #8636

feat: proofClaim in Rollup #8636

Conversation

just-mitch commented Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LHerskind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

just-mitch commented Sep 19, 2024 •

edited

Loading