Attempt to remove circular dependencies (#149)

Author: LeoPatOZ

gas-reports/minimal_inbox_publish.json

@@ -1,2 +1 @@
-{num_publications:20,
-average_gas_used_publish: 44563}
+{ "num_publications": 20, "average_gas_used_publish": 43453 }

snapshots/TaikoInboxTest.json

@@ -1,3 +1,3 @@
 {
-  "publish": "891278"
+  "publish": "869078"
 }

src/protocol/CheckpointTracker.sol

@@ -6,7 +6,9 @@ import {ICommitmentStore} from "./ICommitmentStore.sol";
 import {IInbox} from "./IInbox.sol";
 import {IVerifier} from "./IVerifier.sol";
 
-contract CheckpointTracker is ICheckpointTracker {
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+
+contract CheckpointTracker is ICheckpointTracker, Ownable {
     /// @dev The number of delayed publications up to the proven checkpoint
     uint256 private _totalDelayedPublications;
 
@@ -16,26 +18,31 @@ contract CheckpointTracker is ICheckpointTracker {
     IInbox public immutable inbox;
     IVerifier public immutable verifier;
     ICommitmentStore public immutable commitmentStore;
-    address public immutable proverManager;
+
+    address public proverManager;
 
     /// @param _genesis the checkpoint commitment describing the initial state of the rollup
     /// @param _inbox the inbox contract that contains the publication feed
     /// @param _verifier a contract that can verify the validity of a transition from one checkpoint to another
-    /// @param _proverManager contract responsible for managing the prover auction
     /// @param _commitmentStore contract responsible storing historical commitments
-    constructor(bytes32 _genesis, address _inbox, address _verifier, address _proverManager, address _commitmentStore) {
+    constructor(bytes32 _genesis, address _inbox, address _verifier, address _commitmentStore) Ownable(msg.sender) {
         // set the genesis checkpoint commitment of the rollup - genesis is trusted to be correct
         require(_genesis != 0, ZeroGenesisCommitment());
         inbox = IInbox(_inbox);
         uint256 latestPublicationId = inbox.getNextPublicationId() - 1;
 
         verifier = IVerifier(_verifier);
         commitmentStore = ICommitmentStore(_commitmentStore);
-        proverManager = _proverManager;
 
         _saveCommitment(latestPublicationId, _genesis);
     }
 
+    /// @inheritdoc ICheckpointTracker
+    function updateProverManager(address _proverManager) external onlyOwner {
+        proverManager = _proverManager;
+        emit ProverManagerUpdated(_proverManager);
+    }
+
     /// @inheritdoc ICheckpointTracker
     /// @dev Accepts the last proven checkpoint (or an older one) as the start checkpoint. The reason we allow for an
     /// older checkpoint is to prevent cases where a prover spends time generating a larger proof and the checkpoint

src/protocol/ICheckpointTracker.sol

@@ -43,9 +43,17 @@ interface ICheckpointTracker {
     /// @param commitment the commitment of the latest proven checkpoint
     event CommitmentSaved(uint256 indexed publicationId, bytes32 commitment);
 
+    /// @notice Emitted when ProverManager is updated
+    /// @param proverManager The address of the ProverManager contract
+    event ProverManagerUpdated(address indexed proverManager);
+
     /// @return _ The last proven publication ID
     function provenPublicationId() external view returns (uint256);
 
+    /// @notice Updates the address of the ProverManager contract
+    /// @param _proverManager The address of the ProverManager contract
+    function updateProverManager(address _proverManager) external;
+
     /// @notice Verifies a transition between two checkpoints. Update the latest `provenCheckpoint` if possible
     /// @param start The initial checkpoint before the transition
     /// @param end The final checkpoint after the transition

src/protocol/IInbox.sol

@@ -17,6 +17,9 @@ interface IInbox {
     /// @param attributes The data contained within the publication
     event Published(bytes32 indexed pubHash, PublicationHeader header, bytes[] attributes);
 
+    /// @dev Event emitted when ProposerFees is initialized
+    event ProposerFeesInitialized(address proposerFees);
+
     /// @notice Publish blobs with anchor block information
     /// @param nBlobs Number of blobs to publish
     /// @param anchorBlockId The anchor block ID
@@ -35,4 +38,8 @@ interface IInbox {
     /// @param header The header to validate
     /// @return _ True if the header is valid, false otherwise
     function validateHeader(PublicationHeader calldata header) external view returns (bool);
+
+    /// @notice Updates the proposer fees contract address (prover manager contract)
+    /// @param _proposerFees Address of the proposer fees contract
+    function updateProposerFees(address _proposerFees) external;
 }

src/protocol/taiko_alethia/TaikoInbox.sol

@@ -10,7 +10,9 @@ import {IInbox} from "../IInbox.sol";
 import {ILookahead} from "../ILookahead.sol";
 import {IProposerFees} from "../IProposerFees.sol";
 
-contract TaikoInbox is IInbox, DelayedInclusionStore {
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+
+contract TaikoInbox is IInbox, DelayedInclusionStore, Ownable {
     /// @dev Caller is not the current preconfer
     error NotCurrentPreconfer();
 
@@ -20,17 +22,14 @@ contract TaikoInbox is IInbox, DelayedInclusionStore {
     /// @dev Blockhash is not available for the anchor block
     error BlockhashUnavailable();
 
-    /// @dev Proposer fee set to zero address
-    error ZeroProposerFees();
-
     struct Metadata {
         uint256 anchorBlockId;
         bytes32 anchorBlockHash;
         bool isDelayedInclusion;
     }
 
     ILookahead public immutable lookahead;
-    IProposerFees public immutable proposerFees;
+    IProposerFees public proposerFees;
     uint256 public immutable maxAnchorBlockIdOffset;
 
     // attributes associated with the publication
@@ -43,23 +42,24 @@ contract TaikoInbox is IInbox, DelayedInclusionStore {
     /// @param _lookahead Address of the lookahead contract
     /// @param _blobRefRegistry Address of the blob reference registry contract
     /// @param _maxAnchorBlockIdOffset Maximum offset allowed for anchor block ID
-    /// @param _proposerFees Address of the proposer fees contract (usually prover manager)
     /// @param _inclusionDelay How long before delayed inclusion must be processed
-    constructor(
-        address _lookahead,
-        address _blobRefRegistry,
-        uint256 _maxAnchorBlockIdOffset,
-        address _proposerFees,
-        uint256 _inclusionDelay
-    ) DelayedInclusionStore(_inclusionDelay, _blobRefRegistry) {
-        require(_proposerFees != address(0), ZeroProposerFees());
+    constructor(address _lookahead, address _blobRefRegistry, uint256 _maxAnchorBlockIdOffset, uint256 _inclusionDelay)
+        DelayedInclusionStore(_inclusionDelay, _blobRefRegistry)
+        Ownable(msg.sender)
+    {
         lookahead = ILookahead(_lookahead);
         maxAnchorBlockIdOffset = _maxAnchorBlockIdOffset;
-        proposerFees = IProposerFees(_proposerFees);
+
         // guarantee there is always a previous hash
         _publicationHashes.push(0);
     }
 
+    /// @inheritdoc IInbox
+    function updateProposerFees(address _proposerFees) external onlyOwner {
+        proposerFees = IProposerFees(_proposerFees);
+        emit ProposerFeesInitialized(_proposerFees);
+    }
+
     /// @inheritdoc IInbox
     function publish(uint256 nBlobs, uint64 anchorBlockId) external {
         if (address(lookahead) != address(0)) {
@@ -100,7 +100,9 @@ contract TaikoInbox is IInbox, DelayedInclusionStore {
     /// @param attributes The data to publish
     /// @param isDelayed Whether this is a delayed inclusion publication
     function _publish(bytes[] memory attributes, bool isDelayed) internal {
-        proposerFees.payPublicationFee(msg.sender, isDelayed);
+        if (address(proposerFees) != address(0)) {
+            proposerFees.payPublicationFee(msg.sender, isDelayed);
+        }
 
         uint256 nAttributes = attributes.length;
         bytes32[] memory attributeHashes = new bytes32[](nAttributes);

test/CheckpointTracker.t.sol

@@ -3,6 +3,8 @@ pragma solidity ^0.8.28;
 
 import {MockInbox} from "./mocks/MockInbox.sol";
 import {MockVerifier} from "./mocks/MockVerifier.sol";
+
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 import "forge-std/Test.sol";
 import {CheckpointTracker} from "src/protocol/CheckpointTracker.sol";
 import {ICheckpointTracker} from "src/protocol/ICheckpointTracker.sol";
@@ -15,6 +17,7 @@ contract CheckpointTrackerTest is Test {
     SignalService signalService;
     address proverManager = makeAddr("proverManager");
     bytes32 genesis = keccak256(abi.encode("genesis"));
+    address deployer = makeAddr("deployer");
 
     ICheckpointTracker.Checkpoint start;
     ICheckpointTracker.Checkpoint end;
@@ -24,13 +27,15 @@ contract CheckpointTrackerTest is Test {
         inbox = new MockInbox();
         verifier = new MockVerifier();
         signalService = new SignalService();
-        tracker =
-            new CheckpointTracker(genesis, address(inbox), address(verifier), proverManager, address(signalService));
+        vm.startPrank(deployer);
+        tracker = new CheckpointTracker(genesis, address(inbox), address(verifier), address(signalService));
+        tracker.updateProverManager(address(proverManager));
+        vm.stopPrank();
     }
 
     function test_constructor_shouldRevertWithZeroGenesis() public {
         vm.expectRevert(ICheckpointTracker.ZeroGenesisCommitment.selector);
-        new CheckpointTracker(bytes32(0), address(inbox), address(verifier), proverManager, address(signalService));
+        new CheckpointTracker(bytes32(0), address(verifier), proverManager, address(signalService));
     }
 
     function test_constructor_shouldSetExternalContracts() public view {
@@ -49,6 +54,38 @@ contract CheckpointTrackerTest is Test {
         assertEq(savedCommitment, genesis, "Did not save genesis");
     }
 
+    function test_updateProverManager_shouldSetProverManager() public {
+        vm.startPrank(deployer);
+        CheckpointTracker uninitializedTracker =
+            new CheckpointTracker(genesis, address(inbox), address(verifier), address(signalService));
+        uninitializedTracker.updateProverManager(proverManager);
+        assertEq(address(uninitializedTracker.proverManager()), proverManager, "Did not set prover manager");
+    }
+
+    function test_proveTransition_shouldSucceedWithNoProverManager() public {
+        CheckpointTracker uninitializedTracker =
+            new CheckpointTracker(genesis, address(inbox), address(verifier), address(signalService));
+        _constructValidTransition();
+        uninitializedTracker.proveTransition(start, end, proof);
+    }
+
+    function test_initializeProverManager_onlyOwner() public {
+        vm.prank(deployer);
+        CheckpointTracker uninitializedTracker =
+            new CheckpointTracker(genesis, address(inbox), address(verifier), address(signalService));
+
+        address notDeployer = makeAddr("notdeployer");
+        vm.prank(notDeployer);
+        vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notDeployer));
+        uninitializedTracker.updateProverManager(proverManager);
+    }
+
+    function test_updateProverManager_shouldUpdateProverManager() public {
+        vm.prank(deployer);
+        tracker.updateProverManager(makeAddr("newProverManager"));
+        assertEq(address(tracker.proverManager()), makeAddr("newProverManager"), "Did not update prover manager");
+    }
+
     function test_proveTransition_shouldRevertIfNotCalledByProverManager() public {
         _constructValidTransition();
         vm.expectRevert(ICheckpointTracker.OnlyProverManager.selector);

test/Gas/Inbox.t.sol

@@ -24,11 +24,8 @@ contract TaikoInboxTest is Test {
     uint256 inclusionDelay = uint256(10000000);
 
     function setUp() public {
-        MockProposerFees mockProposerFees = new MockProposerFees();
         blobRefRegistry = new BlobRefRegistry();
-        taikoInbox = new TaikoInbox(
-            lookaheadAddr, address(blobRefRegistry), maxAnchorBlockIdOffset, address(mockProposerFees), inclusionDelay
-        );
+        taikoInbox = new TaikoInbox(lookaheadAddr, address(blobRefRegistry), maxAnchorBlockIdOffset, inclusionDelay);
     }
 
     function test_gas_TaikoPublishFunction() public ProposeMultiplePublications(10) {

test/mocks/MockCheckpointTracker.sol

@@ -24,6 +24,8 @@ contract MockCheckpointTracker is ICheckpointTracker {
         return (nPublications, nDelayedPublications);
     }
 
+    function updateProverManager(address _proverManager) external {}
+
     function setValid(bool isValid) external {
         valid = isValid;
     }

test/mocks/MockInbox.sol

@@ -16,6 +16,10 @@ contract MockInbox is IInbox {
         revert NotImplemented();
     }
 
+    function updateProposerFees(address) external pure {
+        revert NotImplemented();
+    }
+
     function getPublicationHash(uint256 id) external view returns (bytes32) {
         if (id >= numPublications) return 0;
         return keccak256(abi.encode("MockInbox", id));