diff --git a/.solhintrc b/.solhintrc index da50159..41b9f7b 100644 --- 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 index be48927..d0cb4d4 100644 --- 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 index a9a63ba..cbaa78a 100644 --- 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 index 397edbf..15a4dd5 100644 --- 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 -- `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 index 41042af..b9cb03f 100644 --- 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 index 7d15622..1f2a84a 100644 --- 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 index e5ec815..14e76d7 100644 --- 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 index 12c5862..5fdab41 100644 --- 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) { diff --git a/src/proposals/Proposal.sol b/src/proposals/Proposal.sol index 6a69b52..2e604ec 100644 --- a/src/proposals/Proposal.sol +++ b/src/proposals/Proposal.sol @@ -23,14 +23,24 @@ abstract contract Proposal is Test, Script, IProposal { /// they all follow the same structure Action[] public actions; - /// @notice debug flag to print internal proposal logs + /// @notice flag to print internal proposal logs, default is false bool internal DEBUG; + /// @notice flag to trigger the deployment of contracts on-chain, default is true bool internal DO_DEPLOY; + /// @notice flag to initiate post-deployment mocking processes, default is true bool internal DO_AFTER_DEPLOY_MOCK; + /// @notice flag to transform plain solidity code into calldata encoded for the + /// user's governance model, default is true bool internal DO_BUILD; + /// @notice flag to simulate saved actions during the `build` step, default is true bool internal DO_SIMULATE; + /// @notice flag to validate the system state post-proposal simulation, default is true bool internal DO_VALIDATE; + /// @notice flag to print proposal description, actions, and calldata, default is true bool internal DO_PRINT; + /// @notice flag to update the `Addresses.json` file with the newly added and changed + /// addresses, default is false + bool internal DO_UPDATE_ADDRESS_JSON; /// @notice Addresses contract Addresses public addresses; @@ -57,6 +67,7 @@ abstract contract Proposal is Test, Script, IProposal { DO_SIMULATE = vm.envOr("DO_SIMULATE", true); DO_VALIDATE = vm.envOr("DO_VALIDATE", true); DO_PRINT = vm.envOr("DO_PRINT", true); + DO_UPDATE_ADDRESS_JSON = vm.envOr("DO_UPDATE_ADDRESS_JSON", false); } /// @notice proposal name, e.g. "BIP15". @@ -87,6 +98,7 @@ abstract contract Proposal is Test, Script, IProposal { if (DO_SIMULATE) simulate(); if (DO_VALIDATE) validate(); if (DO_PRINT) print(); + if (DO_UPDATE_ADDRESS_JSON) addresses.updateJson(); } /// @notice return proposal calldata. diff --git a/test/Addresses.t.sol b/test/Addresses.t.sol index f14b4d4..af71101 100644 --- a/test/Addresses.t.sol +++ b/test/Addresses.t.sol @@ -275,6 +275,43 @@ contract TestAddresses is Test { assertEq(addresses.isAddressContract("TEST"), true); } + function test_checkAddressFileUpdate() public { + address test1 = vm.addr(1); + + addresses.addAddress("TEST1", test1, 123, false); + + // update addresses.json file + addresses.updateJson(); + + string memory addressesPath = "./addresses/Addresses.json"; + addresses = new Addresses(addressesPath); + + // check Addresses.json is updated correctly and TEST1 address is set + addresses.isAddressSet("TEST1"); + assertEq(addresses.getAddress("TEST1", 123), test1); + + test1 = vm.addr(2); + address test2 = vm.addr(3); + + // change TEST1 address + addresses.changeAddress("TEST1", test1, 123, false); + + // add TEST2 address + addresses.addAddress("TEST2", test2, block.chainid, false); + + // update addresses.json file + addresses.updateJson(); + + // update addresses object with updated addresses.json + addresses = new Addresses(addressesPath); + + // check TEST1 address is updated + assertEq(addresses.getAddress("TEST1", 123), test1); + + // check TEST2 address is added + assertEq(addresses.getAddress("TEST2"), test2); + } + function addressIsPresent() public { address test = vm.addr(1); @@ -319,7 +356,7 @@ contract TestAddresses is Test { address test = vm.addr(1); vm.expectRevert( - "Address: 0x7e5f4552091a69125d5dfcb7b8c2659029395bdf already set on chain: 123" + "Address: 0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf already set on chain: 123" ); addresses.addAddress("TEST_2", test, 123, false); } @@ -329,7 +366,7 @@ contract TestAddresses is Test { memory addressesPath = "./addresses/AddressesDuplicatedDifferentName.json"; vm.expectRevert( - "Address: 0x9679e26bf0c470521de83ad77bb1bf1e7312f739 already set on chain: 31337" + "Address: 0x9679E26bf0C470521DE83Ad77BB1bf1e7312f739 already set on chain: 31337" ); new Addresses(addressesPath); }