diff --git a/packages/contracts-periphery/contracts/foundry-tests/OptimistInviter.t.sol b/packages/contracts-periphery/contracts/foundry-tests/OptimistInviter.t.sol
index 52e33c63b111f5bd5f84ea9b7ec10eed38a78ce7..c591053c560305569992995f24a8d757eb84721e 100644
--- a/packages/contracts-periphery/contracts/foundry-tests/OptimistInviter.t.sol
+++ b/packages/contracts-periphery/contracts/foundry-tests/OptimistInviter.t.sol
@@ -83,6 +83,10 @@ contract OptimistInviter_Initializer is Test {
optimistInviterHelper = new OptimistInviterHelper(optimistInviter, "OptimistInviter");
}
+ function _passMinCommitmentPeriod() internal {
+ vm.warp(optimistInviter.MIN_COMMITMENT_PERIOD() + block.timestamp);
+ }
+
/**
* @notice Returns a user's current invite count, as stored in the AttestationStation.
*/
@@ -174,7 +178,7 @@ contract OptimistInviter_Initializer is Test {
optimistInviter.commitInvite(hashedSignature);
// Check that the commitment was stored correctly
- assertTrue(optimistInviter.commitments(hashedSignature));
+ assertEq(optimistInviter.commitmentTimestamps(hashedSignature), block.timestamp);
}
/**
@@ -196,7 +200,12 @@ contract OptimistInviter_Initializer is Test {
_commitInviteAs(_claimer, signature);
// The hash(claimer ++ signature) should be committed
- assertEq(optimistInviter.commitments(keccak256(abi.encode(_claimer, signature))), true);
+ assertEq(
+ optimistInviter.commitmentTimestamps(keccak256(abi.encode(_claimer, signature))),
+ block.timestamp
+ );
+
+ _passMinCommitmentPeriod();
// OptimistInviter should issue a new attestation allowing claimer to mint
vm.expectEmit(true, true, true, true, address(attestationStation));
@@ -324,7 +333,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
bytes32 hashedSignature = keccak256(abi.encode(sally, signature));
optimistInviter.commitInvite(hashedSignature);
- assertTrue(optimistInviter.commitments(hashedSignature));
+ assertEq(optimistInviter.commitmentTimestamps(hashedSignature), block.timestamp);
}
/**
@@ -338,7 +347,24 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
bytes32 hashedSignature = keccak256(abi.encode(eve, signature));
optimistInviter.commitInvite(hashedSignature);
- assertTrue(optimistInviter.commitments(hashedSignature));
+ assertEq(optimistInviter.commitmentTimestamps(hashedSignature), block.timestamp);
+ }
+
+ /**
+ * @notice Attempting to commit the same hash twice should revert. This prevents griefing.
+ */
+ function test_commitInvite_committingSameHashTwice_reverts() external {
+ _grantInvitesTo(bob);
+ (, bytes memory signature) = _issueInviteAs(bobPrivateKey);
+
+ vm.prank(sally);
+ bytes32 hashedSignature = keccak256(abi.encode(eve, signature));
+ optimistInviter.commitInvite(hashedSignature);
+
+ assertEq(optimistInviter.commitmentTimestamps(hashedSignature), block.timestamp);
+
+ vm.expectRevert("OptimistInviter: commitment already made");
+ optimistInviter.commitInvite(hashedSignature);
}
/**
@@ -360,10 +386,9 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
bytes memory signature
) = _issueInviteAs(bobPrivateKey);
- _commitInviteAs(sally, signature);
-
vm.prank(ted);
optimistInviter.commitInvite(keccak256(abi.encode(sally, signature)));
+ _passMinCommitmentPeriod();
vm.expectEmit(true, true, true, true, address(attestationStation));
emit AttestationCreated(
@@ -385,6 +410,23 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
assertFalse(_hasMintAttestation(eve));
}
+ function test_claimInvite_claimBeforeMinCommitmentPeriod_reverts() external {
+ _grantInvitesTo(bob);
+ (
+ OptimistInviter.ClaimableInvite memory claimableInvite,
+ bytes memory signature
+ ) = _issueInviteAs(bobPrivateKey);
+
+ _commitInviteAs(sally, signature);
+
+ // Some time passes, but not enough to meet the minimum commitment period
+ vm.warp(block.timestamp + 10);
+
+ vm.expectRevert("OptimistInviter: minimum commitment period has not elapsed yet");
+ vm.prank(sally);
+ optimistInviter.claimInvite(sally, claimableInvite, signature);
+ }
+
/**
* @notice Signature issued for previous versions of the contract should fail.
*/
@@ -402,6 +444,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
);
_commitInviteAs(sally, signature);
+ _passMinCommitmentPeriod();
vm.expectRevert("OptimistInviter: invalid signature");
vm.prank(sally);
@@ -426,6 +469,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
);
_commitInviteAs(sally, signature);
+ _passMinCommitmentPeriod();
vm.expectRevert("OptimistInviter: invalid signature");
vm.prank(sally);
@@ -450,6 +494,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
);
_commitInviteAs(sally, signature);
+ _passMinCommitmentPeriod();
vm.expectRevert("OptimistInviter: invalid signature");
vm.prank(sally);
@@ -468,13 +513,14 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
) = _issueThenClaimShouldSucceed(bobPrivateKey, sally);
// Sally tries to claim the invite using the same signature
- _commitInviteAs(sally, signature);
vm.expectRevert("OptimistInviter: nonce has already been used");
vm.prank(sally);
optimistInviter.claimInvite(sally, claimableInvite, signature);
// Carol tries to claim the invite using the same signature
_commitInviteAs(carol, signature);
+ _passMinCommitmentPeriod();
+
vm.expectRevert("OptimistInviter: nonce has already been used");
vm.prank(carol);
optimistInviter.claimInvite(carol, claimableInvite, signature);
@@ -498,6 +544,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
// Sally tries to claim the invite
_commitInviteAs(sally, signature);
+ _passMinCommitmentPeriod();
vm.expectEmit(true, true, true, true, address(attestationStation));
emit AttestationCreated(
@@ -543,6 +590,8 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
_commitInviteAs(sally, bobSignature);
_commitInviteAs(sally, carolSignature);
+ _passMinCommitmentPeriod();
+
vm.expectRevert("OptimistInviter: invalid signature");
vm.prank(sally);
optimistInviter.claimInvite(sally, bobClaimableInvite, carolSignature);
@@ -560,6 +609,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
) = _issueInviteAs(bobPrivateKey);
_commitInviteAs(sally, signature);
+ _passMinCommitmentPeriod();
vm.expectRevert("OptimistInviter: issuer has no invites");
vm.prank(sally);
@@ -588,6 +638,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
) = _issueInviteAs(bobPrivateKey);
_commitInviteAs(eve, signature4);
+ _passMinCommitmentPeriod();
vm.expectRevert("OptimistInviter: issuer has no invites");
vm.prank(eve);
diff --git a/packages/contracts-periphery/contracts/testing/helpers/OptimistInviterHelper.sol b/packages/contracts-periphery/contracts/testing/helpers/OptimistInviterHelper.sol
index d8b9a39ce59f7a0f014c58dc9b9971fd00034ce5..20dabb60944f3d1f852438071c3e8c0436bcb878 100644
--- a/packages/contracts-periphery/contracts/testing/helpers/OptimistInviterHelper.sol
+++ b/packages/contracts-periphery/contracts/testing/helpers/OptimistInviterHelper.sol
@@ -10,16 +10,33 @@ import { OptimistInviter } from "../../universal/op-nft/OptimistInviter.sol";
* Made this a separate contract instead of including in OptimistInviter.t.sol for reusability.
*/
contract OptimistInviterHelper {
+ /**
+ * @notice EIP712 typehash for the ClaimableInvite type.
+ */
bytes32 public constant CLAIMABLE_INVITE_TYPEHASH =
keccak256("ClaimableInvite(address issuer,bytes32 nonce)");
+ /**
+ * @notice EIP712 typehash for the EIP712Domain type that is included as part of the signature.
+ */
bytes32 public constant EIP712_DOMAIN_TYPEHASH =
keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
);
+ /**
+ * @notice Address of OptimistInviter contract we are testing.
+ */
OptimistInviter public optimistInviter;
+
+ /**
+ * @notice OptimistInviter contract name. Used to construct the EIP-712 domain.
+ */
string public name;
+
+ /**
+ * @notice Keeps track of current nonce to generate new nonces for each invite.
+ */
uint256 public currentNonce;
constructor(OptimistInviter _optimistInviter, string memory _name) {
@@ -29,7 +46,11 @@ contract OptimistInviterHelper {
}
/**
- * @notice Returns the hash of the struct ClaimableInvite
+ * @notice Returns the hash of the struct ClaimableInvite.
+ *
+ * @param _claimableInvite ClaimableInvite struct to hash.
+ *
+ * @return EIP-712 typed struct hash.
*/
function getClaimableInviteStructHash(OptimistInviter.ClaimableInvite memory _claimableInvite)
public
@@ -49,13 +70,19 @@ contract OptimistInviterHelper {
/**
* @notice Returns a bytes32 nonce that should change everytime. In practice, people should use
* pseudorandom nonces.
+ *
+ * @return Nonce that should be used as part of ClaimableInvite.
*/
function consumeNonce() public returns (bytes32) {
return bytes32(keccak256(abi.encode(currentNonce++)));
}
/**
- * @notice Returns a ClaimableInvite with the issuer and current nonce
+ * @notice Returns a ClaimableInvite with the issuer and current nonce.
+ *
+ * @param _issuer Issuer to include in the ClaimableInvite.
+ *
+ * @return ClaimableInvite that can be hashed & signed.
*/
function getClaimableInviteWithNewNonce(address _issuer)
public
@@ -66,6 +93,10 @@ contract OptimistInviterHelper {
/**
* @notice Computes the EIP712 digest with default correct parameters.
+ *
+ * @param _claimableInvite ClaimableInvite struct to hash.
+ *
+ * @return EIP-712 compatible digest.
*/
function getDigest(OptimistInviter.ClaimableInvite calldata _claimableInvite)
public
@@ -85,6 +116,14 @@ contract OptimistInviterHelper {
/**
* @notice Computes the EIP712 digest with the given domain parameters.
* Used for testing that different domain parameters fail.
+ *
+ * @param _claimableInvite ClaimableInvite struct to hash.
+ * @param _name Contract name to use in the EIP712 domain.
+ * @param _version Contract version to use in the EIP712 domain.
+ * @param _chainid Chain ID to use in the EIP712 domain.
+ * @param _verifyingContract Address to use in the EIP712 domain.
+ *
+ * @return EIP-712 compatible digest.
*/
function getDigestWithEIP712Domain(
OptimistInviter.ClaimableInvite calldata _claimableInvite,
diff --git a/packages/contracts-periphery/contracts/universal/op-nft/OptimistInviter.sol b/packages/contracts-periphery/contracts/universal/op-nft/OptimistInviter.sol
index 55547cce1160fe3ce86fdf087af6888023e7b236..a67c131e23a90c25c9100782cc1446216adfe7d2 100644
--- a/packages/contracts-periphery/contracts/universal/op-nft/OptimistInviter.sol
+++ b/packages/contracts-periphery/contracts/universal/op-nft/OptimistInviter.sol
@@ -31,14 +31,15 @@ import {
* 5) claimer commits the hash of the address they want to receive the invite on and the
* received signature keccak256(abi.encode(addressToReceiveTo, receivedSignature))
* using the commitInvite function
- * 6) claimer reveals the plaintext ClaimableInvite and the signature using the
+ * 6) claimer waits for the MIN_COMMITMENT_PERIOD to pass.
+ * 7) claimer reveals the plaintext ClaimableInvite and the signature using the
* claimInvite function, receiving the "optimist.can-mint-from-invite" attestation
*/
contract OptimistInviter is Semver, EIP712Upgradeable {
/**
* @notice Emitted when an invite is claimed.
*
- * @param issuer Address that issued the signature.
+ * @param issuer Address that issued the signature.
* @param claimer Address that claimed the invite.
*/
event InviteClaimed(address indexed issuer, address indexed claimer);
@@ -78,6 +79,18 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
*/
AttestationStation public immutable ATTESTATION_STATION;
+ /**
+ * @notice Minimum age of a commitment (in seconds) before it can be revealed using claimInvite.
+ * Currently set to 60 seconds.
+ *
+ * Prevents an attacker from front-running a commitment by taking the signature in the
+ * claimInvite call and quickly committing and claiming it before the the claimer's
+ * transaction succeeds. With this, frontrunning a commitment requires that an attacker
+ * be able to prevent the honest claimer's claimInvite transaction from being included
+ * for this long.
+ */
+ uint256 public constant MIN_COMMITMENT_PERIOD = 60;
+
/**
* @notice Struct that represents a claimable invite that will be signed by the issuer.
*
@@ -94,9 +107,9 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
}
/**
- * @notice Maps from hashes to whether or not they have been committed.
+ * @notice Maps from hashes to the timestamp when they were committed.
*/
- mapping(bytes32 => bool) public commitments;
+ mapping(bytes32 => uint256) public commitmentTimestamps;
/**
* @notice Maps from addresses to nonces to whether or not they have been used.
@@ -127,7 +140,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
* claimed yet will no longer be accepted by the claimInvite function. Please make
* sure to notify the issuers that they must re-issue their invite signatures.
*
- * @param _name Contract name
+ * @param _name Contract name.
*/
function initialize(string memory _name) public initializer {
__EIP712_init(_name, EIP712_VERSION);
@@ -181,12 +194,21 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
* scheme, anyone who is watching the mempool can take the signature being submitted
* and front run the transaction to claim the invite to their own address.
*
+ * The same commitment can only be made once, and the function reverts if the
+ * commitment has already been made. This prevents griefing where a malicious party can
+ * prevent the original claimer from being able to claimInvite.
+ *
*
* @param _commitment A hash of the claimer and signature concatenated.
* keccak256(abi.encode(_claimer, _signature))
*/
function commitInvite(bytes32 _commitment) public {
- commitments[_commitment] = true;
+ // Check that the commitment hasn't already been made. This prevents griefing where
+ // a malicious party continuously re-submits the same commitment, preventing the original
+ // claimer from claiming their invite by resetting the minimum commitment period.
+ require(commitmentTimestamps[_commitment] == 0, "OptimistInviter: commitment already made");
+
+ commitmentTimestamps[_commitment] = block.timestamp;
}
/**
@@ -196,9 +218,10 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
* committed using commitInvite. Before issuing the "optimist.can-mint-from-invite"
* attestation, this function checks that
* 1) the hash corresponding to the _claimer and the _signature was committed
- * 2) the _signature is signed correctly by the issuer
- * 3) the _signature hasn't already been used to claim an invite before
- * 4) the _signature issuer has not used up all of their invites
+ * 2) MIN_COMMITMENT_PERIOD has passed since the commitment was made.
+ * 3) the _signature is signed correctly by the issuer
+ * 4) the _signature hasn't already been used to claim an invite before
+ * 5) the _signature issuer has not used up all of their invites
* This function doesn't require that the _claimer is calling this function.
*
* @param _claimer Address that will be granted the invite.
@@ -210,12 +233,22 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
ClaimableInvite calldata _claimableInvite,
bytes memory _signature
) public {
+ uint256 commitmentTimestamp = commitmentTimestamps[
+ keccak256(abi.encode(_claimer, _signature))
+ ];
+
// Make sure the claimer and signature have been committed.
require(
- commitments[keccak256(abi.encode(_claimer, _signature))],
+ commitmentTimestamp > 0,
"OptimistInviter: claimer and signature have not been committed yet"
);
+ // Check that MIN_COMMITMENT_PERIOD has passed since the commitment was made.
+ require(
+ commitmentTimestamp + MIN_COMMITMENT_PERIOD <= block.timestamp,
+ "OptimistInviter: minimum commitment period has not elapsed yet"
+ );
+
// Generate a EIP712 typed data hash to compare against the signature.
bytes32 digest = _hashTypedDataV4(
keccak256(