feat(contracts): implement `OPSuccinctEntryPoint.sol` #371

fakedev9999 · 2025-02-05T21:20:34Z

OPSuccinctEntryPoint

OPSuccinctEntryPoint is a contract that:

Stores whitelisted addresses for proposers and challengers.
Calls the disputeGameFactory to create, challenge, prove, and resolve OPSuccinctFaultDisputeGames.
Holds credits for users when games are resolved.

OPSuccinctFaultDisputeGame

Feature for fast finality mode is added. Proposers can now create a game with a proof so that games can be resolved right after creation. This is done by passing additional proof bytes to extraData when creating a game.
claimCredit method is removed since credits are now managed in the entry point contract.

Tests

Tests are updated to use OPSuccinctEntryPoint.
New tests for permissioned proposer and challenger is added.
New test to ensure all actions should be done through entry point contract is added.

github-actions · 2025-02-05T21:26:00Z

Metric	Value
Batch Start	23,716,530
Batch End	23,716,535
Witness Generation (seconds)	0
Execution Duration (seconds)	35
Total Instruction Count	1,445,699,898
Oracle Verify Cycles	248,638,831
Derivation Cycles	855,313,921
Block Execution Cycles	258,907,486
Blob Verification Cycles	109,478,689
Total SP1 Gas	0
Number of Blocks	5
Number of Transactions	39
Ethereum Gas Used	22,331,138
Cycles per Block	289,139,979
Cycles per Transaction	37,069,228
Transactions per Block	7
Gas Used per Block	4,466,227
Gas Used per Transaction	572,593
BN Pair Cycles	0
BN Add Cycles	0
BN Mul Cycles	0
KZG Eval Cycles	0
EC Recover Cycles	216,277
P256 Verify Cycles	0

contracts/src/fp/OPSuccinctEntryPoint.sol

…o parents

Proposers can now create a game with a proof. If a valid proof is provided for game creation, the game can be resolved right after the game creation.

Now, in case of a game retirement and new game type being deployed, you can reference retired game type's games as a parent of new game type's games.

Allows to sweep funds across multiple games, instead of having to send claimReward tx for every game.

…s with no parents" This reverts commit 50b9768.

ratankaliani · 2025-02-11T02:34:59Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+ *         1) Stores whitelisted addresses for proposers and challengers.
+ *         2) Calls the `disputeGameFactory` to create, challenge, prove,
+ *            and resolve `OPSuccinctFaultDisputeGame`s.
+ *         3) Holds credits for users when games are resolved.


Suggested change

* 3) Holds credits for users when games are resolved.

* 3) Holds credits for users, which can be claimed by parties once games are resolved.

ratankaliani · 2025-02-11T02:49:10Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+    ////////////////////////////////////////////////////////////////
+
+    /// @notice Modifier to check if the caller is a whitelisted proposer.
+    /// @dev Whitelisting zero address allows permissionless proposer system.


Suggested change

/// @dev Whitelisting zero address allows permissionless proposer system.

/// @dev If the zero address is marked as `true`, then permissionless proposing is enabled.

ratankaliani · 2025-02-11T02:49:27Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+    }
+
+    /// @notice Modifier to check if the caller is a whitelisted challenger.
+    /// @dev Whitelisting zero address allows permissionless challenger system.


Suggested change

/// @dev Whitelisting zero address allows permissionless challenger system.

/// @dev If the zero address is marked as `true`, then permissionless challenging is enabled.

ratankaliani · 2025-02-11T03:08:21Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+
+    /// @notice Allows the owner to recover by pulling ETHs out of the contract.
+    function recover() external {
+        require(msg.sender == owner(), "OPSuccinctEntryPoint: not owner");


Add an onlyOwner modifier for this

ratankaliani · 2025-02-11T03:08:31Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+        OPSuccinctFaultDisputeGame(address(_game)).resolve();
+    }
+
+    /// @notice Allows the owner to recover by pulling ETHs out of the contract.


Allows the sender to recover all ETH from the contract.

ratankaliani · 2025-02-11T03:08:57Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+        OPSuccinctFaultDisputeGame(address(_game)).resolve();
+    }
+
+    /// @notice Allows the owner to recover by pulling ETHs out of the contract.


You should add another method which allows reclaiming ETH from a specific user that's onlyOwner gated, mirroring the logic inside of the OP FP contracts.

ratankaliani · 2025-02-11T03:46:13Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

@@ -124,6 +127,9 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
    ///         is the amount of the bond that is distributed to the prover when proven with a valid proof.
    uint256 internal immutable PROOF_REWARD;


Shouldn't there be different rewards? Shouldn't these be configurable in the entrypoint?

ratankaliani · 2025-02-11T03:50:57Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

+                    OPSuccinctEntryPoint.addCredit.selector, claimData.counteredBy, address(this).balance
+                )
+            );
+            if (!success) revert BondTransferFailed();

            emit Resolved(status);



I'd add a comment after this function that at this point we know parentGameStatus = GameStatus.DEFENDER_WINS.

ratankaliani · 2025-02-11T03:51:06Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

        if (parentGameStatus == GameStatus.IN_PROGRESS) revert ParentGameNotResolved();

        // INVARIANT: If the parent game's claim is invalid, then the current game's claim is invalid.
+        // INVARIANT: If there is no challenger, i.e. `claimData.counteredBy == address(0)`, the bond is burnt.
        if (parentGameStatus == GameStatus.CHALLENGER_WINS) {


Use if, else-if to make this more clear

ratankaliani · 2025-02-11T03:51:50Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

+                    OPSuccinctEntryPoint.addCredit.selector, claimData.counteredBy, address(this).balance
+                )
+            );
+            if (!success) revert BondTransferFailed();


Add reasons for failure to transfer above this line.

ratankaliani · 2025-02-11T03:52:35Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

+            (bool success,) = ENTRY_POINT.call{value: address(this).balance}(
+                abi.encodeWithSelector(OPSuccinctEntryPoint.addCredit.selector, claimant(), address(this).balance)
+            );
+            if (!success) revert BondTransferFailed();


Add failure cases

ratankaliani · 2025-02-11T03:52:39Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

+                    OPSuccinctEntryPoint.addCredit.selector, claimData.counteredBy, address(this).balance
+                )
+            );
+            if (!success) revert BondTransferFailed();


Add failure cases

ratankaliani · 2025-02-11T03:54:58Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

+                    OPSuccinctEntryPoint.addCredit.selector, claimData.counteredBy, address(this).balance
+                )
+            );
+            if (!success) revert BondTransferFailed();

            emit Resolved(status);
        } else if (claimData.status == ProposalStatus.UnchallengedAndValidProofProvided) {
            claimData.status = ProposalStatus.Resolved;


I'd add a clarifying comment here about the fact that when the game state is unchallenged, there's no reward for a third-party prover. The proposer just gets re-funded the bond. This prevents front-running attacks on the proposer.

ratankaliani · 2025-02-11T04:04:00Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

@@ -304,7 +359,7 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
        if (msg.value != PROOF_REWARD) revert IncorrectBondAmount();


The challengeBond is the prover's reward.

Proposer incentive: Don't create bad proposals.
Challenger incentive: Don't challenge proposals (otherwise you lose proof reward, which should be greater than cost to generate proof by significant amount).
Prover Incentive: Generate proofs if PROOF_REWARD is high enough.

ratankaliani · 2025-02-11T04:05:23Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

@@ -377,13 +438,19 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
        if (parentGameStatus == GameStatus.IN_PROGRESS) revert ParentGameNotResolved();

        // INVARIANT: If the parent game's claim is invalid, then the current game's claim is invalid.
+        // INVARIANT: If there is no challenger, i.e. `claimData.counteredBy == address(0)`, the bond is burnt.


Suggested change

// INVARIANT: If there is no challenger, i.e. `claimData.counteredBy == address(0)`, the bond is burnt.

// INVARIANT: If there is no challenger, i.e. `claimData.counteredBy == address(0)`, the bond will be stored in `address(0)` in the entrypoint. Can only be recovered by the owner of the entrypoint contract.

ratankaliani · 2025-02-11T04:06:14Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol


            emit Resolved(status);
        } else if (claimData.status == ProposalStatus.ChallengedAndValidProofProvided) {
            claimData.status = ProposalStatus.Resolved;
            status = GameStatus.DEFENDER_WINS;
            resolvedAt = Timestamp.wrap(uint64(block.timestamp));

-            // Record the proof reward for the prover
-            credit[claimData.prover] += PROOF_REWARD;
+            // Add prover's reward to the entry point contract.


Suggested change

// Add prover's reward to the entry point contract.

// Send challenger's proving reward to the entry point contract for the prover to claim.

ratankaliani · 2025-02-11T04:06:43Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol


-            // Record the remaining balance (proposer's bond) for the proposer
-            credit[gameCreator()] += address(this).balance - PROOF_REWARD;
+            // Return the proposer's bond to the entry point contract.


Suggested change

// Return the proposer's bond to the entry point contract.

// Return the proposer's bond to the entry point contract where the proposer can claim it.

ratankaliani · 2025-02-11T04:07:03Z

contracts/src/fp/lib/Errors.sol

@@ -19,3 +19,6 @@ error ParentGameNotResolved();

 /// @notice Thrown when the claim has already been proven.
 error AlreadyProven();
+
+/// @notice Thrown when actions are attempted without going through the entry point.
+error NotThroughEntryPoint();


Suggested change

error NotThroughEntryPoint();

error SenderNotEntrypoint();

ratankaliani · 2025-02-11T04:07:34Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol


            emit Resolved(status);
        } else if (claimData.status == ProposalStatus.UnchallengedAndValidProofProvided) {
            claimData.status = ProposalStatus.Resolved;
            status = GameStatus.DEFENDER_WINS;
            resolvedAt = Timestamp.wrap(uint64(block.timestamp));

-            // Record the proposer's reward
-            credit[gameCreator()] += address(this).balance;
+            // Return the proposer's bond to the entry point contract.


I would be explicit when saying it's returned to the entry point contract about who it's being returned to.

ratankaliani · 2025-02-11T04:08:15Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

@@ -397,8 +464,11 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
            status = GameStatus.DEFENDER_WINS;
            resolvedAt = Timestamp.wrap(uint64(block.timestamp));

-            // Record the proposer's reward
-            credit[gameCreator()] += address(this).balance;
+            // Return the proposer's bond to the entry point contract.


Suggested change

// Return the proposer's bond to the entry point contract.

// Send proposer's bond to the entrypoint contract so the proposer can claim it.

ratankaliani · 2025-02-11T04:08:53Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol

@@ -407,51 +477,50 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
            status = GameStatus.CHALLENGER_WINS;
            resolvedAt = Timestamp.wrap(uint64(block.timestamp));

-            // Record the challenger's reward
-            credit[claimData.counteredBy] += address(this).balance;
+            // Add challenger's reward to the entry point contract.


Suggested change

// Add challenger's reward to the entry point contract.

// Send the proposer's bond to the entry point contract so the challenger can claim the reward for a valid challenge.

ratankaliani · 2025-02-11T04:09:18Z

contracts/src/fp/OPSuccinctFaultDisputeGame.sol


            emit Resolved(status);
        } else if (claimData.status == ProposalStatus.UnchallengedAndValidProofProvided) {
            claimData.status = ProposalStatus.Resolved;
            status = GameStatus.DEFENDER_WINS;
            resolvedAt = Timestamp.wrap(uint64(block.timestamp));

-            // Record the proposer's reward
-            credit[gameCreator()] += address(this).balance;
+            // Return the proposer's bond to the entry point contract.


Suggested change

// Return the proposer's bond to the entry point contract.

// Send proposer's bond to the entrypoint contract so the proposer can claim it.

ratankaliani · 2025-02-11T04:10:15Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+        // Append address(msg.sender) to the extraData
+        bytes memory extraDataWithClaimant = abi.encodePacked(_extraData, msg.sender);
+
+        // Call the factory to create the game
+        // Append address(msg.sender) to the extraData to record the claimant
+        IDisputeGame newGame = disputeGameFactory.create{value: msg.value}(gameType, _rootClaim, extraDataWithClaimant);
+
+        // Emit an event with the new game address and the rootClaim


Add periods.

ratankaliani

Pending review from OP Labs, LGTM

fakedev9999 · 2025-02-11T22:31:53Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+     * @dev Credit is added when a game is resolved.
+     */
+    function addCredit(address _user, uint256 _amount) external payable {
+        credit[_user] += _amount;


This should check that the msg.value is sent before adding _amount.

fakedev9999 · 2025-02-11T22:32:24Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+     *
+     * @dev Credit is added when a game is resolved.
+     */
+    function addCredit(address _user, uint256 _amount) external payable {


You don't need to pass the amount. You're just sending ETH.

fakedev9999 · 2025-02-11T22:32:56Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+     * @notice Adds credit to a user's account.
+     *
+     * @param _user The address to add credit to.
+     * @param _amount The amount of credit to add.


fakedev9999 · 2025-02-11T22:50:32Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+     * @notice Resolves an `OPSuccinctFaultDisputeGame`.
+     * @dev Anyone can resolve a game.
+     */
+    function resolveGame(IDisputeGame _game) external {


Pass (address), not IDisputeGame

fakedev9999 · 2025-02-11T22:51:16Z

contracts/src/fp/OPSuccinctEntryPoint.sol

+     * @dev Only whitelisted challengers can call this function.
+     * @dev Exact amount of ETH for proof reward is required when challenging.
+     */
+    function challengeGame(IDisputeGame _game) external payable onlyChallenger {


Pass address, not IDispute

fakedev9999 · 2025-02-12T18:54:12Z

With airgap, it's better to distribute bonds when claimCredit is called after resolve.

With no "Bank" feature in the EntryPoint contract, it's better to have a simple "AccessManager" contract with a very simple interface with isProposerWhitelisted, isChallengerWhitelisted. proposers, challengers.

It's not good to call create, challenge to the EntryPoint and resolve, prove to each games.

fakedev9999 commented Feb 5, 2025

View reviewed changes

contracts/src/fp/OPSuccinctEntryPoint.sol Outdated Show resolved Hide resolved

contracts/src/fp/OPSuccinctEntryPoint.sol Outdated Show resolved Hide resolved

fakedev9999 force-pushed the taehoon/implement-entrypoint-contract branch 4 times, most recently from 4a2474c to 82fa049 Compare February 10, 2025 22:16

fakedev9999 requested a review from ratankaliani February 11, 2025 00:44

fakedev9999 marked this pull request as ready for review February 11, 2025 00:44

fakedev9999 added 13 commits February 10, 2025 16:45

feat(contracts): implement OPSuccinctEntryPoint.sol

5b3949f

feat(contracts): ensure that all games are created through entrypoint

04deefe

feat(contracts): use anchor state as starting output for games with n…

9a7b1ce

…o parents

feat(contracts): implement fast finality feature

3cbfd39

Proposers can now create a game with a proof. If a valid proof is provided for game creation, the game can be resolved right after the game creation.

feat(contracts): allow parent to be not the same game type

2d3dad1

Now, in case of a game retirement and new game type being deployed, you can reference retired game type's games as a parent of new game type's games.

feat(contracts): manage all credits in entry point contract

501c9ce

Allows to sweep funds across multiple games, instead of having to send claimReward tx for every game.

test: update tests and fix contracts

43c2813

update

abcc2b0

update

75232cf

cleanup

23c955e

cleanup

f86d642

Revert "feat(contracts): use anchor state as starting output for game…

5c41441

…s with no parents" This reverts commit 50b9768.

cleanup

b5bb6ca

fakedev9999 force-pushed the taehoon/implement-entrypoint-contract branch from e2db1c4 to b5bb6ca Compare February 11, 2025 00:46

feat(contracts): allow guardian to recover funds from entry point

9b5a84a

ratankaliani reviewed Feb 11, 2025

View reviewed changes

fakedev9999 commented Feb 11, 2025

View reviewed changes

fakedev9999 closed this Feb 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(contracts): implement `OPSuccinctEntryPoint.sol` #371

feat(contracts): implement `OPSuccinctEntryPoint.sol` #371

fakedev9999 commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited

Loading

ratankaliani Feb 11, 2025 •

edited

Loading

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025 •

edited

Loading

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani Feb 11, 2025

ratankaliani left a comment

fakedev9999 Feb 11, 2025

fakedev9999 Feb 11, 2025

fakedev9999 Feb 11, 2025

fakedev9999 Feb 11, 2025

fakedev9999 Feb 11, 2025

fakedev9999 commented Feb 12, 2025

	* 3) Holds credits for users when games are resolved.
	* 3) Holds credits for users, which can be claimed by parties once games are resolved.

	/// @dev Whitelisting zero address allows permissionless proposer system.
	/// @dev If the zero address is marked as `true`, then permissionless proposing is enabled.

		@@ -124,6 +127,9 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
		/// is the amount of the bond that is distributed to the prover when proven with a valid proof.
		uint256 internal immutable PROOF_REWARD;

		@@ -304,7 +359,7 @@ contract OPSuccinctFaultDisputeGame is Clone, ISemver {
		if (msg.value != PROOF_REWARD) revert IncorrectBondAmount();

	// INVARIANT: If there is no challenger, i.e. `claimData.counteredBy == address(0)`, the bond is burnt.
	// INVARIANT: If there is no challenger, i.e. `claimData.counteredBy == address(0)`, the bond will be stored in `address(0)` in the entrypoint. Can only be recovered by the owner of the entrypoint contract.

	// Add prover's reward to the entry point contract.
	// Send challenger's proving reward to the entry point contract for the prover to claim.

	// Return the proposer's bond to the entry point contract.
	// Return the proposer's bond to the entry point contract where the proposer can claim it.

	// Return the proposer's bond to the entry point contract.
	// Send proposer's bond to the entrypoint contract so the proposer can claim it.

	// Add challenger's reward to the entry point contract.
	// Send the proposer's bond to the entry point contract so the challenger can claim the reward for a valid challenge.

feat(contracts): implement OPSuccinctEntryPoint.sol #371

feat(contracts): implement OPSuccinctEntryPoint.sol #371

Conversation

fakedev9999 commented Feb 5, 2025 • edited Loading

OPSuccinctEntryPoint

OPSuccinctFaultDisputeGame

Tests

github-actions bot commented Feb 5, 2025 • edited Loading

ratankaliani Feb 11, 2025 • 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

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

ratankaliani Feb 11, 2025 • 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

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

ratankaliani 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

fakedev9999 commented Feb 12, 2025

feat(contracts): implement `OPSuccinctEntryPoint.sol` #371

feat(contracts): implement `OPSuccinctEntryPoint.sol` #371

fakedev9999 commented Feb 5, 2025 •

edited

Loading

github-actions bot commented Feb 5, 2025 •

edited

Loading

ratankaliani Feb 11, 2025 •

edited

Loading

ratankaliani Feb 11, 2025 •

edited

Loading