Commit 59413a14 authored by James Kim's avatar James Kim

apply some review changes

parent 43ec7efb
...@@ -23,10 +23,10 @@ contract OptimistInviter_Initializer is Test { ...@@ -23,10 +23,10 @@ contract OptimistInviter_Initializer is Test {
bytes32 CLAIMABLE_INVITE_TYPEHASH; bytes32 CLAIMABLE_INVITE_TYPEHASH;
bytes32 EIP712_DOMAIN_TYPEHASH; bytes32 EIP712_DOMAIN_TYPEHASH;
address constant alice_inviteGranter = address(128); address internal alice_inviteGranter;
address constant sally = address(512); address internal sally;
address constant ted = address(1024); address internal ted;
address constant eve = address(2048); address internal eve;
address internal bob; address internal bob;
uint256 internal bobPrivateKey; uint256 internal bobPrivateKey;
...@@ -43,6 +43,11 @@ contract OptimistInviter_Initializer is Test { ...@@ -43,6 +43,11 @@ contract OptimistInviter_Initializer is Test {
function setUp() public { function setUp() public {
currentNonce = 0; currentNonce = 0;
alice_inviteGranter = makeAddr("alice_inviteGranter");
sally = makeAddr("sally");
ted = makeAddr("ted");
eve = makeAddr("eve");
bobPrivateKey = 0xB0B0B0B0; bobPrivateKey = 0xB0B0B0B0;
bob = vm.addr(bobPrivateKey); bob = vm.addr(bobPrivateKey);
...@@ -55,13 +60,9 @@ contract OptimistInviter_Initializer is Test { ...@@ -55,13 +60,9 @@ contract OptimistInviter_Initializer is Test {
vm.deal(alice_inviteGranter, 1 ether); vm.deal(alice_inviteGranter, 1 ether);
vm.deal(bob, 1 ether); vm.deal(bob, 1 ether);
vm.deal(sally, 1 ether); vm.deal(sally, 1 ether);
vm.deal(ted, 1 ether);
vm.deal(eve, 1 ether); vm.deal(eve, 1 ether);
vm.label(alice_inviteGranter, "alice_inviteGranter");
vm.label(bob, "bob");
vm.label(sally, "sally");
vm.label(carol, "carol");
CLAIMABLE_INVITE_TYPEHASH = keccak256("ClaimableInvite(address issuer,bytes32 nonce)"); CLAIMABLE_INVITE_TYPEHASH = keccak256("ClaimableInvite(address issuer,bytes32 nonce)");
EIP712_DOMAIN_TYPEHASH = keccak256( EIP712_DOMAIN_TYPEHASH = keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
...@@ -75,10 +76,11 @@ contract OptimistInviter_Initializer is Test { ...@@ -75,10 +76,11 @@ contract OptimistInviter_Initializer is Test {
*/ */
function _initializeContracts() internal { function _initializeContracts() internal {
attestationStation = new AttestationStation(); attestationStation = new AttestationStation();
vm.expectEmit(false, false, false, false);
emit Initialized(1);
optimistInviter = new OptimistInviter(alice_inviteGranter, attestationStation); optimistInviter = new OptimistInviter(alice_inviteGranter, attestationStation);
vm.expectEmit(true, true, true, true, address(optimistInviter));
emit Initialized(1);
optimistInviter.initialize("OptimistInviter"); optimistInviter.initialize("OptimistInviter");
} }
...@@ -450,7 +452,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer { ...@@ -450,7 +452,7 @@ contract OptimistInviterTest is OptimistInviter_Initializer {
); );
// Should emit an event indicating that the invite was claimed // Should emit an event indicating that the invite was claimed
vm.expectEmit(true, false, false, false, address(optimistInviter)); vm.expectEmit(true, true, true, true, address(optimistInviter));
emit InviteClaimed(bob, sally); emit InviteClaimed(bob, sally);
vm.prank(eve); vm.prank(eve);
......
...@@ -3,9 +3,7 @@ pragma solidity 0.8.15; ...@@ -3,9 +3,7 @@ pragma solidity 0.8.15;
import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semver.sol"; import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semver.sol";
import { AttestationStation } from "./AttestationStation.sol"; import { AttestationStation } from "./AttestationStation.sol";
import { import { SignatureChecker } from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol";
SignatureCheckerUpgradeable
} from "@openzeppelin/contracts-upgradeable/utils/cryptography/SignatureCheckerUpgradeable.sol";
import { import {
EIP712Upgradeable EIP712Upgradeable
} from "@openzeppelin/contracts-upgradeable/utils/cryptography/draft-EIP712Upgradeable.sol"; } from "@openzeppelin/contracts-upgradeable/utils/cryptography/draft-EIP712Upgradeable.sol";
...@@ -27,18 +25,27 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -27,18 +25,27 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
*/ */
event InviteClaimed(address indexed issuer, address indexed claimer); event InviteClaimed(address indexed issuer, address indexed claimer);
/**
* @notice Version identifier, used for upgrades.
*/
uint8 public constant VERSION = 1;
/** /**
* @notice EIP712 typehash for the ClaimableInvite type. * @notice EIP712 typehash for the ClaimableInvite type.
* keccak256("ClaimableInvite(address issuer,bytes32 nonce)") * keccak256("ClaimableInvite(address issuer,bytes32 nonce)")
*/ */
bytes32 public immutable CLAIMABLE_INVITE_TYPEHASH = bytes32 public constant CLAIMABLE_INVITE_TYPEHASH =
0x6529fd129351e725d7bcbc468b0b0b4675477e56b58514e69ab7e66ddfd20fce; 0x6529fd129351e725d7bcbc468b0b0b4675477e56b58514e69ab7e66ddfd20fce;
/**
* @notice Attestation key for granting invites.
* bytes32("optimist.can-invite")
*/
bytes32 public constant CAN_INVITE_ATTESTATION_KEY =
0x6f7074696d6973742e63616e2d696e7669746500000000000000000000000000;
/**
* @notice Attestation key allowing the attested account to mint.
* bytes32("optimist.can-mint-from-invite")
*/
bytes32 public constant CAN_MINT_FROM_INVITE_ATTESTATION_KEY =
0x6f7074696d6973742e63616e2d6d696e742d66726f6d2d696e76697465000000;
/** /**
* @notice Granter who can set accounts' invite counts. * @notice Granter who can set accounts' invite counts.
*/ */
...@@ -90,7 +97,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -90,7 +97,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
* *
* @param _name Contract name * @param _name Contract name
*/ */
function initialize(string memory _name) public reinitializer(VERSION) { function initialize(string memory _name) public initializer {
__EIP712_init(_name, version()); __EIP712_init(_name, version());
} }
...@@ -112,7 +119,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -112,7 +119,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
for (uint256 i; i < length; ) { for (uint256 i; i < length; ) {
// The granted invites are stored as an attestation from this contract on the // The granted invites are stored as an attestation from this contract on the
// AttestationStation contract. Number of invites is stored as a encoded uint256 in the // AttestationStation contract. Number of invites is stored as a encoded uint256 in the
// data field of the attetation. // data field of the attestation.
ATTESTATION_STATION.attest( ATTESTATION_STATION.attest(
_accounts[i], _accounts[i],
bytes32("optimist.can-invite"), bytes32("optimist.can-invite"),
...@@ -120,7 +127,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -120,7 +127,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
); );
unchecked { unchecked {
i++; ++i;
} }
} }
} }
...@@ -130,7 +137,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -130,7 +137,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
* This is necessary to prevent front-running when the invitee is claiming the invite. * This is necessary to prevent front-running when the invitee is claiming the invite.
* *
* @param _commitment A hash of the claimer and signature concatenated. * @param _commitment A hash of the claimer and signature concatenated.
keccak256(abi.encode(_claimer, _signature)) * keccak256(abi.encode(_claimer, _signature))
*/ */
function commitInvite(bytes32 _commitment) public { function commitInvite(bytes32 _commitment) public {
commitments[_commitment] = true; commitments[_commitment] = true;
...@@ -172,20 +179,16 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -172,20 +179,16 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
// wants to revoke a signature, they can use a smart contract wallet to issue the signature, // wants to revoke a signature, they can use a smart contract wallet to issue the signature,
// then invalidate the signature after issuing it. // then invalidate the signature after issuing it.
require( require(
SignatureCheckerUpgradeable.isValidSignatureNow( SignatureChecker.isValidSignatureNow(_claimableInvite.issuer, digest, _signature),
_claimableInvite.issuer,
digest,
_signature
),
"OptimistInviter: invalid signature" "OptimistInviter: invalid signature"
); );
// The issuer includes a pseudorandom nonce in the signature to prevent replay attacks. // The issuer's signature commits to a nonce to prevent replay attacks.
// This checks that the nonce has not been used for this issuer before. The nonces are // This checks that the nonce has not been used for this issuer before. The nonces are
// scoped to the issuer address, so the same nonce can be used by different issuers without // scoped to the issuer address, so the same nonce can be used by different issuers without
// clashing. // clashing.
require( require(
!usedNonces[_claimableInvite.issuer][_claimableInvite.nonce], usedNonces[_claimableInvite.issuer][_claimableInvite.nonce] == false,
"OptimistInviter: nonce has already been used" "OptimistInviter: nonce has already been used"
); );
...@@ -196,7 +199,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -196,7 +199,7 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
bytes memory attestation = ATTESTATION_STATION.attestations( bytes memory attestation = ATTESTATION_STATION.attestations(
address(this), address(this),
_claimableInvite.issuer, _claimableInvite.issuer,
bytes32("optimist.can-invite") CAN_INVITE_ATTESTATION_KEY
); );
// Failing this check means that the issuer was never granted any invites to begin with. // Failing this check means that the issuer was never granted any invites to begin with.
require(attestation.length > 0, "OptimistInviter: issuer has no invites"); require(attestation.length > 0, "OptimistInviter: issuer has no invites");
...@@ -210,16 +213,19 @@ contract OptimistInviter is Semver, EIP712Upgradeable { ...@@ -210,16 +213,19 @@ contract OptimistInviter is Semver, EIP712Upgradeable {
// The invite issuer is included in the data of the attestation. // The invite issuer is included in the data of the attestation.
ATTESTATION_STATION.attest( ATTESTATION_STATION.attest(
_claimer, _claimer,
bytes32("optimist.can-mint-from-invite"), CAN_MINT_FROM_INVITE_ATTESTATION_KEY,
abi.encode(_claimableInvite.issuer) abi.encode(_claimableInvite.issuer)
); );
// Reduce the issuer's invite count by 1 by re-attesting the optimist.can-invite attestation // Reduce the issuer's invite count by 1 by re-attesting the optimist.can-invite attestation
// with the new count. // with the new count.
count--; unchecked {
--count;
}
ATTESTATION_STATION.attest( ATTESTATION_STATION.attest(
_claimableInvite.issuer, _claimableInvite.issuer,
bytes32("optimist.can-invite"), CAN_INVITE_ATTESTATION_KEY,
abi.encode(count) abi.encode(count)
); );
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment