Double call to NPP reverts

FilOzone · Nov 20, 2024 · f3e854f · f3e854f
1 parent e0b6c83
commit f3e854f
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 33 deletions.
diff --git a/src/SimplePDPService.sol b/src/SimplePDPService.sol
@@ -67,7 +67,7 @@ contract PDPRecordKeeper {
     }
 }
 
-// SimplePDPServiceApplication is a default implementation of a PDP Application.
+// SimplePDPServiceApplication is a default implementation of a PDP Listener
 // It maintains a record of all events that have occurred in the PDP service,
 // and provides a way to query these events.
 // This contract only supports one PDP service caller, set in the constructor.
@@ -157,7 +157,7 @@ contract SimplePDPService is PDPListener, PDPRecordKeeper, Initializable, UUPSUp
         // Can only get here if calling nextProvingPeriod multiple times within the same proving period
         uint256 prevDeadline = provingDeadlines[proofSetId] - getMaxProvingPeriod();
         if (block.number <= prevDeadline) {
-            return;
+            revert("One call to nextProvingPeriod allowed per proving period");
         }
 
         uint256 periodsSkipped;
@@ -178,4 +178,4 @@ contract SimplePDPService is PDPListener, PDPRecordKeeper, Initializable, UUPSUp
         provingDeadlines[proofSetId] = provingDeadlines[proofSetId] + getMaxProvingPeriod()*(periodsSkipped+1); 
         provenThisPeriod[proofSetId] = false;
     }
-}
+}
diff --git a/test/PDPVerifier.t.sol b/test/PDPVerifier.t.sol
@@ -10,7 +10,7 @@ import {PDPFees} from "../src/Fees.sol";
 import {SimplePDPService, PDPRecordKeeper} from "../src/SimplePDPService.sol";
 
 contract PDPVerifierProofSetCreateDeleteTest is Test {
-    SimplePDPService listener;
+    TestingRecordKeeperService listener;
     ListenerHelper listenerAssert;
     PDPVerifier pdpVerifier;
 
@@ -23,10 +23,7 @@ contract PDPVerifierProofSetCreateDeleteTest is Test {
         );
         MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
         pdpVerifier = PDPVerifier(address(proxy));
-        SimplePDPService listenerImpl = new SimplePDPService();
-        initializeData = abi.encodeWithSelector(SimplePDPService.initialize.selector, address(pdpVerifier));
-        MyERC1967Proxy listenerProxy = new MyERC1967Proxy(address(listenerImpl), initializeData);
-        listener = SimplePDPService(address(listenerProxy));
+        listener = new TestingRecordKeeperService();
         listenerAssert = new ListenerHelper(address(listener));
     }
     function tearDown() public view {
@@ -147,7 +144,7 @@ contract PDPVerifierProofSetCreateDeleteTest is Test {
 
 contract PDPVerifierOwnershipTest is Test {
     PDPVerifier pdpVerifier;
-    SimplePDPService listener;
+    TestingRecordKeeperService listener;
     address public owner;
     address public nextOwner;
     address public nonOwner;
@@ -160,10 +157,7 @@ contract PDPVerifierOwnershipTest is Test {
         );
         MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
         pdpVerifier = PDPVerifier(address(proxy));
-        SimplePDPService listenerImpl = new SimplePDPService();
-        initializeData = abi.encodeWithSelector(SimplePDPService.initialize.selector, address(pdpVerifier));
-        MyERC1967Proxy listenerProxy = new MyERC1967Proxy(address(listenerImpl), initializeData);
-        listener = SimplePDPService(address(listenerProxy));
+        listener = new TestingRecordKeeperService();
 
         owner = address(this);
         nextOwner = address(0x1234);
@@ -231,7 +225,7 @@ contract PDPVerifierProofSetMutateTest is Test {
     uint256 constant challengeFinalityDelay = 2;
 
     PDPVerifier pdpVerifier;
-    SimplePDPService listener;
+    TestingRecordKeeperService listener;
     ListenerHelper listenerAssert;
 
     function setUp() public {
@@ -242,10 +236,7 @@ contract PDPVerifierProofSetMutateTest is Test {
         );
         MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
         pdpVerifier = PDPVerifier(address(proxy));
-        SimplePDPService listenerImpl = new SimplePDPService();
-        initializeData = abi.encodeWithSelector(SimplePDPService.initialize.selector, address(pdpVerifier));
-        MyERC1967Proxy listenerProxy = new MyERC1967Proxy(address(listenerImpl), initializeData);
-        listener = SimplePDPService(address(listenerProxy));
+        listener = new TestingRecordKeeperService();
         listenerAssert = new ListenerHelper(address(listener));
     }
 
@@ -896,7 +887,7 @@ import "../src/PDPVerifier.sol";
 
 contract SumTreeAddTest is Test {
     SumTreeInternalTestPDPVerifier pdpVerifier;
-    SimplePDPService listener;
+    TestingRecordKeeperService listener;
     uint256 testSetId;
 
     function setUp() public {
@@ -907,10 +898,7 @@ contract SumTreeAddTest is Test {
         );
         MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
         pdpVerifier = SumTreeInternalTestPDPVerifier(address(proxy));
-        SimplePDPService listenerImpl = new SimplePDPService();
-        initializeData = abi.encodeWithSelector(SimplePDPService.initialize.selector, address(pdpVerifier));
-        MyERC1967Proxy listenerProxy = new MyERC1967Proxy(address(listenerImpl), initializeData);
-        listener = SimplePDPService(address(listenerProxy));       
+        listener = new TestingRecordKeeperService();
         testSetId = pdpVerifier.createProofSet{value: PDPFees.sybilFee()}(address(listener));
     }
 
@@ -1272,7 +1260,7 @@ contract PDPListenerIntegrationTest is Test {
 
 contract PDPVerifierE2ETest is Test, ProofBuilderHelper {
     PDPVerifier pdpVerifier;
-    SimplePDPService listener;
+    TestingRecordKeeperService listener;
     uint256 constant challengeFinalityDelay = 2;
 
     function setUp() public {
@@ -1283,10 +1271,7 @@ contract PDPVerifierE2ETest is Test, ProofBuilderHelper {
         );
         MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
         pdpVerifier = PDPVerifier(address(proxy));
-        SimplePDPService listenerImpl = new SimplePDPService();
-        initializeData = abi.encodeWithSelector(SimplePDPService.initialize.selector, address(pdpVerifier));
-        MyERC1967Proxy listenerProxy = new MyERC1967Proxy(address(listenerImpl), initializeData);
-        listener = SimplePDPService(address(listenerProxy));
+        listener = new TestingRecordKeeperService();
     }
 
     function testCompleteProvingPeriodE2E() public {

diff --git a/test/SimplePDPService.t.sol b/test/SimplePDPService.t.sol
@@ -141,8 +141,7 @@ contract SimplePDPServiceFaultsTest is Test {
         pdpService.posessionProven(proofSetId, leafCount, seed, challengeCount);
     }
 
-    // TODO this should change to a revert 
-    function testNextProvingPeriodNoop() public {
+    function testNextProvingPeriodTwiceFails() public {
         // Set up the proving deadline
         pdpService.rootsAdded(proofSetId, 0, new PDPVerifier.RootData[](0));
         vm.roll(block.number + pdpService.getMaxProvingPeriod() - 100);
@@ -153,11 +152,10 @@ contract SimplePDPServiceFaultsTest is Test {
         assertEq(pdpService.provingDeadlines(proofSetId), deadline1, "Proving deadline should not change until nextProvingPeriod.");
         pdpService.nextProvingPeriod(proofSetId, leafCount);
         assertEq(pdpService.provingDeadlines(proofSetId), deadline1 + pdpService.getMaxProvingPeriod(), "Proving deadline should be updated");
-        uint256 deadline2 = pdpService.provingDeadlines(proofSetId);
         assertFalse(pdpService.provenThisPeriod(proofSetId));
 
-        pdpService.nextProvingPeriod(proofSetId, leafCount); // NOOP
-        assertEq(pdpService.provingDeadlines(proofSetId), deadline2, "Proving deadline should not change");
+        vm.expectRevert("One call to nextProvingPeriod allowed per proving period");
+        pdpService.nextProvingPeriod(proofSetId, leafCount); 
     }
 
     function testFaultWithinOpenPeriod() public {