Deprecate random.sol

.github/workflows/protocol_tests.yml

@@ -39,8 +39,6 @@ jobs:
 
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1
-        with:
-          version: "nightly-f625d0fa7c51e65b4bf1e8f7931cd1c6e2e285e9"
 
       - name: Install forge dependencies
         run: forge install

packages/protocol/contracts-0.8/common/IsL2Check.sol

@@ -11,6 +11,13 @@ contract IsL2Check {
     _;
   }
 
+  modifier onlyL2() {
+    if (!isL2()) {
+      revert("This method is not supported in L1.");
+    }
+    _;
+  }
+
   function isL2() public view returns (bool) {
     uint32 size;
     address _addr = proxyAdminAddress;
@@ -22,7 +29,7 @@ contract IsL2Check {
 
   function allowOnlyL1() internal view {
     if (isL2()) {
-      revert("This method is not supported in L2 anymore.");
+      revert("This method is no longer supported in L2.");
     }
   }
 }

packages/protocol/contracts/identity/Random.sol

@@ -8,6 +8,7 @@ import "../common/CalledByVm.sol";
 import "../common/Initializable.sol";
 import "../common/UsingPrecompiles.sol";
 import "../common/interfaces/ICeloVersionedContract.sol";
+import "../../contracts-0.8/common/IsL2Check.sol";
 
 /**
  * @title Provides randomness for verifier selection
@@ -18,7 +19,8 @@ contract Random is
   Ownable,
   Initializable,
   UsingPrecompiles,
-  CalledByVm
+  CalledByVm,
+  IsL2Check
 {
   using SafeMath for uint256;
 
@@ -64,24 +66,26 @@ contract Random is
     bytes32 randomness,
     bytes32 newCommitment,
     address proposer
-  ) external onlyVm {
+  ) external onlyL1 onlyVm {
     _revealAndCommit(randomness, newCommitment, proposer);
   }
 
   /**
    * @notice Querying the current randomness value.
    * @return Returns the current randomness value.
+   * @dev Only available on L1.
    */
-  function random() external view returns (bytes32) {
+  function random() external view onlyL1 returns (bytes32) {
     return _getBlockRandomness(block.number, block.number);
   }
 
   /**
    * @notice Get randomness values of previous blocks.
    * @param blockNumber The number of block whose randomness value we want to know.
    * @return The associated randomness value.
+   * @dev Only available on L1.
    */
-  function getBlockRandomness(uint256 blockNumber) external view returns (bytes32) {
+  function getBlockRandomness(uint256 blockNumber) external view onlyL1 returns (bytes32) {
     return _getBlockRandomness(blockNumber, block.number);
   }
 
@@ -93,14 +97,15 @@ contract Random is
    * @return Patch version of the contract.
    */
   function getVersionNumber() external pure returns (uint256, uint256, uint256, uint256) {
-    return (1, 1, 1, 1);
+    return (1, 1, 2, 0);
   }
 
   /**
    * @notice Sets the number of old random blocks whose randomness values can be queried.
    * @param value Number of old random blocks whose randomness values can be queried.
+   * @dev Only available on L1.
    */
-  function setRandomnessBlockRetentionWindow(uint256 value) public onlyOwner {
+  function setRandomnessBlockRetentionWindow(uint256 value) public onlyL1 onlyOwner {
     require(value > 0, "randomnessBlockRetetionWindow cannot be zero");
     randomnessBlockRetentionWindow = value;
     emit RandomnessBlockRetentionWindowSet(value);
@@ -120,8 +125,13 @@ contract Random is
    * @param randomness Bytes that will be added to the entropy pool.
    * @param newCommitment The hash of randomness that will be revealed in the future.
    * @param proposer Address of the block proposer.
+   * @dev Only available on L1.
    */
-  function _revealAndCommit(bytes32 randomness, bytes32 newCommitment, address proposer) internal {
+  function _revealAndCommit(
+    bytes32 randomness,
+    bytes32 newCommitment,
+    address proposer
+  ) internal onlyL1 {
     require(newCommitment != computeCommitment(0), "cannot commit zero randomness");
 
     // ensure revealed randomness matches previous commitment
@@ -149,8 +159,9 @@ contract Random is
    * @param randomness The new randomness added to history.
    * @dev The calls to this function should be made so that on the next call, blockNumber will
    * be the previous one, incremented by one.
+   * @dev Only available on L1.
    */
-  function addRandomness(uint256 blockNumber, bytes32 randomness) internal {
+  function addRandomness(uint256 blockNumber, bytes32 randomness) internal onlyL1 {
     history[blockNumber] = randomness;
     if (blockNumber % getEpochSize() == 0) {
       if (lastEpochBlock < historyFirst) {
@@ -187,8 +198,12 @@ contract Random is
    * @param blockNumber The number of block whose randomness value we want to know.
    * @param cur Number of the current block.
    * @return The associated randomness value.
+   * @dev Only available on L1.
    */
-  function _getBlockRandomness(uint256 blockNumber, uint256 cur) internal view returns (bytes32) {
+  function _getBlockRandomness(
+    uint256 blockNumber,
+    uint256 cur
+  ) internal view onlyL1 returns (bytes32) {
     require(blockNumber <= cur, "Cannot query randomness of future blocks");
     require(
       blockNumber == lastEpochBlock ||

packages/protocol/test-sol/Readme.md

@@ -1,13 +1,56 @@
+### Building
 
-## Naming Convention
+You can build this project by simply running
 
-Our tests generally follow the Foundry Book best [practices](https://book.getfoundry.sh/tutorials/best-practices#general-test-guidance), however, a few notable exepctions are enforced:
+```bash
+forge build
+```
 
-1. Naming of contracts. Contract names for test are called `ContractTest_functionToTest_[When|After]`. In case necesary, a contract with setUp `ContractTest` and basic general test are created. Most other contracts are expected to inherit from this.
-2. Function naming.
-   1. In case of a emit expected `test_Emits_EventName_[When|After]`
-   2. In case of a revert expected `test_Reverts_EventName_[When|After]`
+### Testing
+
+We are in the process of migrating our tests to use [Foundry](https://book.getfoundry.sh/). The tests in this folder have already been migrated from [Truffle](../test).
+
+To run tests with Foundry there's no need to `yarn` or manage any Javascript dependencies. Instead, run
+
+```bash
+forge test
+```
+
+This will run all tests in this folder. To run only a specific file you can use
+
+```bash
+forge test --match-path ./path/to/file.t.sol
+```
+
+To run only a specific contract in a test file, you can use
+
+```bash
+forge test --match-contract CONTRACT_NAME
+```
+
+To run only a specific test, you can use
+
+```bash
+forge test --match-test test_ToMatch
+```
+
+You can specify a verbosity level with the `-v`, `-vv`, `-vvv`, and `-vvvv` flags. The more `v`s you put the more verbose the output will be.
+
+Putting it all together, you might run something like
+
+```bash
+forge test --match-path ./path/to/file.t.sol --match-test test_ToMatch -vvv
+```
+
+You can read more about the `forge test` command [here](https://book.getfoundry.sh/reference/forge/forge-test).
+
+To skip a specific test, you can add `vm.skip(true);` as the first line of the test.
+
+If a test name begins with `testFail` rather than `test`, foundry will expect the test to fail / revert.
+
+Please follow the naming convention `test_NameOfTest` / `testFail_NameOfTest`.
+
+If you're new to Forge / Foundry, we recommend looking through the [Cheatcode Reference](https://book.getfoundry.sh/cheatcodes/) for a list of useful commands that make writing tests easier.
 
 
-Generally, words as "should" are expected to be omitted. The world `If` is generally not used in favor of `When`.
 

packages/protocol/test-sol/common/Accounts.t.sol

@@ -613,7 +613,7 @@ contract AccountsTest_setPaymentDelegation is AccountsTest {
   function test_Revert_SetPaymentDelegation_WhenL2() public {
     _whenL2();
     accounts.createAccount();
-    vm.expectRevert("This method is not supported in L2 anymore.");
+    vm.expectRevert("This method is no longer supported in L2.");
     accounts.setPaymentDelegation(beneficiary, fraction);
   }
 

packages/protocol/test-sol/common/IsL2Check.t.sol

@@ -35,16 +35,20 @@ contract IsL2Check_IsL2Test is IsL2CheckBase {
     helper_WhenProxyAdminAddressIsSet();
     assertTrue(isL2Check.isL2());
   }
-}
 
-contract IsL2Check_OnlyL1 is IsL2CheckBase {
-  function test_WhenIsL1() public view {
-    isL2Check.onlyL1Function();
+  function test_IsL1_WhenProxyAdminSet() public {
+    helper_WhenProxyAdminAddressIsSet();
+    assertFalse(!isL2Check.isL2());
   }
+}
 
+contract IsL2Check_OnlyL1 is IsL2CheckBase {
   function test_WhenIsL2_WhenProxyAdminSet() public {
     helper_WhenProxyAdminAddressIsSet();
-    vm.expectRevert("This method is not supported in L2 anymore.");
+    vm.expectRevert("This method is no longer supported in L2.");
+    isL2Check.onlyL1Function();
+  }
+  function test_WhenIsL1() public view {
     isL2Check.onlyL1Function();
   }
 }