update addresses.json file using vm.writeJson (#84)

* update addresses.json file using vm.writeJson * disable quotes solhint error * use vm.toString * fix tests * code cleanup * add test for address file update * update docs and comments * Apply suggestions from code review Co-authored-by: Elliot <34463580+ElliotFriedman@users.noreply.github.com> * pr feedback * Update docs/overview/architecture/proposal-functions.md --------- Co-authored-by: Elliot <34463580+ElliotFriedman@users.noreply.github.com>
solidity-labs-io · Aug 15, 2024 · 066b407 · 066b407
1 parent 553e3b8
commit 066b407
Show file tree

Hide file tree

Showing 10 changed files with 199 additions and 42 deletions.
diff --git a/.solhintrc b/.solhintrc
@@ -19,6 +19,7 @@
         "no-unused-import": "error",
         "explicit-types": "off",
         "const-name-snakecase": "off",
-        "gas-custom-errors": "off"
+        "gas-custom-errors": "off",
+        "quotes": "off"
     }
 }
diff --git a/addresses/Addresses.sol b/addresses/Addresses.sol
@@ -51,27 +51,34 @@ contract Addresses is IAddresses, Test {
     /// @notice array of addresses deployed during a proposal
     RecordedAddress[] private recordedAddresses;
 
-    // @notice array of addresses changed during a proposal
+    /// @notice array of addresses changed during a proposal
     ChangedAddress[] private changedAddresses;
 
-    constructor(string memory addressesPath) {
+    /// @notice array of all address details
+    SavedAddresses[] private savedAddresses;
+
+    /// @notice path of addresses file
+    string private addressesPath;
+
+    constructor(string memory _addressesPath) {
+        addressesPath = _addressesPath;
         string memory addressesData = string(
             abi.encodePacked(vm.readFile(addressesPath))
         );
 
         bytes memory parsedJson = vm.parseJson(addressesData);
 
-        SavedAddresses[] memory savedAddresses = abi.decode(
+        SavedAddresses[] memory fileAddresses = abi.decode(
             parsedJson,
             (SavedAddresses[])
         );
 
-        for (uint256 i = 0; i < savedAddresses.length; i++) {
+        for (uint256 i = 0; i < fileAddresses.length; i++) {
             _addAddress(
-                savedAddresses[i].name,
-                savedAddresses[i].addr,
-                savedAddresses[i].chainId,
-                savedAddresses[i].isContract
+                fileAddresses[i].name,
+                fileAddresses[i].addr,
+                fileAddresses[i].chainId,
+                fileAddresses[i].isContract
             );
         }
     }
@@ -178,6 +185,16 @@ contract Addresses is IAddresses, Test {
             })
         );
 
+        for (uint256 i; i < savedAddresses.length; i++) {
+            if (
+                keccak256(abi.encode(savedAddresses[i].name)) ==
+                keccak256(abi.encode(name)) &&
+                savedAddresses[i].chainId == chainId
+            ) {
+                savedAddresses[i].addr = _addr;
+            }
+        }
+
         data.addr = _addr;
         data.isContract = isContract;
         vm.label(_addr, name);
@@ -327,6 +344,12 @@ contract Addresses is IAddresses, Test {
         }
     }
 
+    /// @dev Update Address json
+    function updateJson() external {
+        string memory json = _constructJson();
+        vm.writeJson(json, addressesPath);
+    }
+
     /// @notice add an address for a specific chainId
     /// @param name the name of the address
     /// @param addr the address to add
@@ -363,7 +386,7 @@ contract Addresses is IAddresses, Test {
             string(
                 abi.encodePacked(
                     "Address: ",
-                    addressToString(addr),
+                    vm.toString(addr),
                     " already set on chain: ",
                     vm.toString(chainId)
                 )
@@ -377,6 +400,15 @@ contract Addresses is IAddresses, Test {
         currentAddress.addr = addr;
         currentAddress.isContract = isContract;
 
+        savedAddresses.push(
+            SavedAddresses({
+                name: name,
+                addr: addr,
+                chainId: chainId,
+                isContract: isContract
+            })
+        );
+
         vm.label(addr, name);
     }
 
@@ -445,16 +477,37 @@ contract Addresses is IAddresses, Test {
         }
     }
 
-    function addressToString(
-        address _addr
-    ) internal pure returns (string memory) {
-        bytes memory alphabet = "0123456789abcdef";
-        bytes20 value = bytes20(_addr);
-        bytes memory str = new bytes(40); // An Ethereum address has 20 bytes, hence 40 characters in hex
-        for (uint256 i = 0; i < 20; i++) {
-            str[i * 2] = alphabet[uint8(value[i] >> 4)];
-            str[1 + i * 2] = alphabet[uint8(value[i] & 0x0f)];
+    /// @notice constructs json string data for address json from saved addresses array
+    function _constructJson() private view returns (string memory) {
+        string memory json = "[";
+
+        for (uint256 i = 0; i < savedAddresses.length; i++) {
+            json = string(
+                abi.encodePacked(
+                    json,
+                    "{",
+                    '"addr": "',
+                    vm.toString(savedAddresses[i].addr),
+                    '",',
+                    '"name": "',
+                    savedAddresses[i].name,
+                    '",',
+                    '"chainId": ',
+                    vm.toString(savedAddresses[i].chainId),
+                    ",",
+                    '"isContract": ',
+                    savedAddresses[i].isContract ? "true" : "false",
+                    "}"
+                )
+            );
+
+            if (i < savedAddresses.length - 1) {
+                json = string(abi.encodePacked(json, ","));
+            }
         }
-        return string(abi.encodePacked("0x", str));
+
+        json = string(abi.encodePacked(json, "]"));
+
+        return json;
     }
 }
diff --git a/docs/overview/architecture/addresses.md b/docs/overview/architecture/addresses.md
@@ -153,6 +153,14 @@ The `isAddressContract` function determines whether an address on the execution
 addresses.isAddressContract("CONTRACT_NAME");
 ```
 
+### Update addresses file
+
+The `updateJson` function updates the `Addresses.json` file with the newly added and changed addresses. This is helpful as a user doesn't need to update the file manually after the proposal run.
+
+```solidity
+addresses.updateJson();
+```
+
 ## Usage
 
 When writing a proposal, set the `addresses` object using the `setAddresses` method. Ensure the correct path for `Addresses.json` file is passed inside the constructor while creating the `addresses` object. Use the `addresses` object to add, update, retrieve, and remove addresses.

diff --git a/docs/overview/architecture/proposal-functions.md b/docs/overview/architecture/proposal-functions.md
@@ -287,7 +287,17 @@ FPS is flexible enough so that for any different governance model, governance pr
 
 <a id="#run-function"></a>
 
--   `run()`: This function serves as the entry point for proposal execution. It selects the `primaryForkId` which will be used to run the proposal simulation. It executes `deploy()`, `afterDeployMock()`, `build()`, `simulate()`, `validate()`, and `print()` in that order if the flag for a function is set to true. `deploy()` is encapsulated in start and stop broadcast. This is done so that contracts can be deployed on-chain. For further reading, see the [run function](../overview/architecture/proposal-functions.md#run-function).
+-   `run()`: This function serves as the entry point for proposal execution. It selects the `primaryForkId` which will be used to run the proposal simulation. It executes `deploy()`, `afterDeployMock()`, `build()`, `simulate()`, `validate()`, `print()` and `addresses.updateJson()` in that order if the flag for a function is set to true. `deploy()` is encapsulated in start and stop broadcast. This is done so that contracts can be deployed on-chain.
+
+    Flags used in `run()` function:
+
+    -   **DO_DEPLOY**: When set to true, triggers the deployment of contracts on-chain. Default value is true.
+    -   **DO_AFTER_DEPLOY_MOCK**: When set to true, initiates post-deployment mocking processes. Used to simulate an action that has not happened yet for testing such as dealing tokens for testing or simulating scenarios after deployment. Default value is true.
+    -   **DO_BUILD**: When set to true, controls the build process and transforms plain solidity code into calldata encoded for the user's governance model. Default value is true.
+    -   **DO_SIMULATE**: When set to true, allows for the simulation of saved actions during the `build` step. Default value is true.
+    -   **DO_VALIDATE**: When set to true, validates the system state post-proposal simulation. Default value is true.
+    -   **DO_PRINT**: When set to true, prints proposal description, actions, and calldata. Default value is true.
+    -   **DO_UPDATE_ADDRESS_JSON**: When set to true, updates the `Addresses.json` file with the newly added and changed addresses. Default value is false.
 
     ```solidity
     function run() public virtual {
@@ -307,6 +317,7 @@ FPS is flexible enough so that for any different governance model, governance pr
         if (DO_SIMULATE) simulate();
         if (DO_VALIDATE) validate();
         if (DO_PRINT) print();
+        if (DO_UPDATE_ADDRESS_JSON) addresses.updateJson();
     }
     ```
 

diff --git a/foundry.toml b/foundry.toml
@@ -3,7 +3,7 @@ libs = ['lib']
 test = 'test'
 auto_detect_solc = true
 # Require read access for 'addresses' and 'out' folder on root repo for bytecode verification
-fs_permissions = [{ access = "read", path = "./"}]
+fs_permissions = [{ access = "read-write", path = "./"}]
 optimizer = true
 optimizer_runs = 200
 

diff --git a/src/interface/IGovernor.sol b/src/interface/IGovernor.sol
@@ -15,7 +15,7 @@ interface IGovernor {
         Executed
     }
 
-     /**
+    /**
      * @notice module:voting
      * @dev A description of the possible `support` values for {castVote} and the way these votes are counted, meant to
      * be consumed by UIs to show correct vote options and interpret the results. The string is a URL-encoded sequence of
@@ -69,20 +69,26 @@ interface IGovernor {
      * snapshot is performed at the end of this block. Hence, voting for this proposal starts at the beginning of the
      * following block.
      */
-    function proposalSnapshot(uint256 proposalId) external view returns (uint256);
+    function proposalSnapshot(
+        uint256 proposalId
+    ) external view returns (uint256);
 
     /**
      * @notice module:core
      * @dev Timepoint at which votes close. If using block number, votes close at the end of this block, so it is
      * possible to cast a vote during this block.
      */
-    function proposalDeadline(uint256 proposalId) external view returns (uint256);
+    function proposalDeadline(
+        uint256 proposalId
+    ) external view returns (uint256);
 
     /**
      * @notice module:core
      * @dev The account that created a proposal.
      */
-    function proposalProposer(uint256 proposalId) external view returns (address);
+    function proposalProposer(
+        uint256 proposalId
+    ) external view returns (address);
 
     /**
      * @notice module:core
@@ -96,7 +102,9 @@ interface IGovernor {
      * @notice module:core
      * @dev Whether a proposal needs to be queued before execution.
      */
-    function proposalNeedsQueuing(uint256 proposalId) external view returns (bool);
+    function proposalNeedsQueuing(
+        uint256 proposalId
+    ) external view returns (bool);
 
     /**
      * @notice module:user-config
@@ -141,7 +149,10 @@ interface IGovernor {
      * Note: this can be implemented in a number of ways, for example by reading the delegated balance from one (or
      * multiple), {ERC20Votes} tokens.
      */
-    function getVotes(address account, uint256 timepoint) external view returns (uint256);
+    function getVotes(
+        address account,
+        uint256 timepoint
+    ) external view returns (uint256);
 
     /**
      * @notice module:reputation
@@ -157,7 +168,10 @@ interface IGovernor {
      * @notice module:voting
      * @dev Returns whether `account` has cast a vote on `proposalId`.
      */
-    function hasVoted(uint256 proposalId, address account) external view returns (bool);
+    function hasVoted(
+        uint256 proposalId,
+        address account
+    ) external view returns (bool);
 
     /**
      * @dev Create a new proposal. Vote start after a delay specified by {IGovernor-votingDelay} and lasts for a
@@ -226,7 +240,10 @@ interface IGovernor {
      *
      * Emits a {VoteCast} event.
      */
-    function castVote(uint256 proposalId, uint8 support) external returns (uint256 balance);
+    function castVote(
+        uint256 proposalId,
+        uint8 support
+    ) external returns (uint256 balance);
 
     /**
      * @dev Cast a vote with a reason
@@ -277,8 +294,7 @@ interface IGovernor {
         bytes memory params,
         bytes memory signature
     ) external returns (uint256 balance);
-
-    }
+}
 
 interface IGovernorTimelockControl {
     /**
@@ -290,4 +306,3 @@ interface IGovernorTimelockControl {
 interface IGovernorVotes {
     function token() external view returns (address);
 }
-
diff --git a/src/interface/IVotes.sol b/src/interface/IVotes.sol
@@ -14,12 +14,20 @@ interface IVotes {
     /**
      * @dev Emitted when an account changes their delegate.
      */
-    event DelegateChanged(address indexed delegator, address indexed fromDelegate, address indexed toDelegate);
+    event DelegateChanged(
+        address indexed delegator,
+        address indexed fromDelegate,
+        address indexed toDelegate
+    );
 
     /**
      * @dev Emitted when a token transfer or delegate change results in changes to a delegate's number of voting units.
      */
-    event DelegateVotesChanged(address indexed delegate, uint256 previousVotes, uint256 newVotes);
+    event DelegateVotesChanged(
+        address indexed delegate,
+        uint256 previousVotes,
+        uint256 newVotes
+    );
 
     /**
      * @dev Returns the current amount of votes that `account` has.
@@ -30,7 +38,10 @@ interface IVotes {
      * @dev Returns the amount of votes that `account` had at a specific moment in the past. If the `clock()` is
      * configured to use block numbers, this will return the value at the end of the corresponding block.
      */
-    function getPastVotes(address account, uint256 timepoint) external view returns (uint256);
+    function getPastVotes(
+        address account,
+        uint256 timepoint
+    ) external view returns (uint256);
 
     /**
      * @dev Returns the total supply of votes available at a specific moment in the past. If the `clock()` is
@@ -40,7 +51,9 @@ interface IVotes {
      * Votes that have not been delegated are still part of total supply, even though they would not participate in a
      * vote.
      */
-    function getPastTotalSupply(uint256 timepoint) external view returns (uint256);
+    function getPastTotalSupply(
+        uint256 timepoint
+    ) external view returns (uint256);
 
     /**
      * @dev Returns the delegate that `account` has chosen.
@@ -55,5 +68,12 @@ interface IVotes {
     /**
      * @dev Delegates votes from signer to `delegatee`.
      */
-    function delegateBySig(address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s) external;
+    function delegateBySig(
+        address delegatee,
+        uint256 nonce,
+        uint256 expiry,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) external;
 }
diff --git a/src/proposals/GovernorBravoProposal.sol b/src/proposals/GovernorBravoProposal.sol
@@ -24,8 +24,8 @@ abstract contract GovernorBravoProposal is Proposal {
     /// @notice Getter function for `GovernorBravoDelegate.propose()` calldata
     function getCalldata()
         public
-        virtual
         view
+        virtual
         override
         returns (bytes memory data)
     {