Merge pull request ArbitrumFoundation#269 from ArbitrumFoundation/proposal-creation-docs

DZGoldman · web-flow · commit ea030a80144d · 2024-03-19T16:43:32.000-04:00
add / update proposal creation docs
diff --git a/docs/creating-a-proposal.md b/docs/creating-a-proposal.md
@@ -0,0 +1,48 @@
+# Creating a core proposal 
+
+The following is a guide to creating an on-chain Arbitrum DAO core governor proposal. 
+
+Prior to creating a proposal, it should be publically discussed on the Arbitrum Forum and a an off-chian temperature-check snapshot poll should be conducted; see [Arbitrum DAO docs](https://docs.arbitrum.foundation/how-tos/create-submit-dao-proposal#step-2-submit-your-on-chain-proposal-using-tally) for details. 
+
+Once a temperature check has passed, the following things can be done:
+
+1. **Create Action Contract(s)**: Create the Governance Action Contract(s) for the proposal. See [here](../src/gov-action-contracts/README.md) for a guide to Governance Action Contracts.
+
+1. **(Optional) Unit Tests**: Write unit tests for the Action Contracts if appropriate, i.e., if they introduce non-trivial new logic. 
+
+1. **Deploy And Verify Action Contracts**: Using `forge create`, deploy Action Contracts on their appropriate chain(s) (Ethereum, Arbitrum One, or Nova) and verify their bytecode on etherscan/arbiscan. (Note that no additional scripting should be used/required for Action Contract deployment.)   
+
+1. **Generate Propoposal Calldata**:
+    a. Create a new directory under ./scripts/proposals/ for your proposal data, e.g.
+
+    ```
+    mkdir ./scipts/proposals/AIPMyProp
+    ```
+    b. Generate proposal data using `yarn gen:proposalData` using the addresses of the deloyed action contracts, and providing a path to store the new JSON file.
+
+    For example:
+    ```
+        yarn gen:proposalData \
+        --govChainProviderRPC https://arb1.arbitrum.io/rpc \
+        --actionChainIds 1 42161 \
+        --actionAddresses 0xAddressA 0xAddressB   \
+        --writeToJsonPath ./scipts/proposals/AIPMyProp/my-prop-data.json  
+    ```
+    Note that the indices for the chain ids correspond  with those of the action contracts. E.g., in the example, 0xAddressA should be deployed on chain 1 (Ethereum) and 0xAddressB should be deployed on chain 42161 (Arbitrum One.)
+
+    Run `yarn gen:proposalData --help` to see all optional parameters.
+
+    Once the data JSON is properly created, it can be included in a public pull request to this repo.
+
+    Note that while some prior proposals include description text and/or deployment scripts, that is no longer necessary. 
+
+1. **Run Simulation**
+
+    Using the calldata generated in the previous step, test the proposal using [the Arbitrum DAO governance seatbelt](https://github.com/ArbitrumFoundation/governance-seatbelt). Include the configuration in a PR to the seatbelt repo ([example](https://github.com/ArbitrumFoundation/governance-seatbelt/pull/26)).
+
+    The seatbelt will generate a human readable report of all of the state changes in the proposal; only proceed if the report's result are what is expected. 
+
+1. **Submit Proposal In Tally UI** See [here](./submit_a_proposal.md). 
+
+
+
diff --git a/docs/submit_a_proposal.md b/docs/submit_a_proposal.md
@@ -1,48 +1,25 @@
-# Submit a Proposal Example: AIP-1.2
+# Submit a Core Proposal Via Tally
 
-Here, we'll go through an example of how an $ARB delegate can permissionlessly submit a DAO proposal using the Tally web app. We'll use [AIP-1.2](https://forum.arbitrum.foundation/t/proposal-aip-1-2-foundation-and-dao-governance/13362) and [AIP-1.1](https://forum.arbitrum.foundation/t/proposal-aip-1-1-lockup-budget-transparency/13360) as our examples.
+Before a proposal can be submitted, its action contrats must be deployed, its JSON data must be prepared, and the it must  pass the simulation tests. See [here](./creating-a-proposal.md) for details on those steps. 
 
-To start, review the 7 general steps to submit a proposal using Tally in the Arbitrum Foundation docs [here](https://docs.arbitrum.foundation/how-tos/create-submit-dao-proposal#step-2-submit-your-on-chain-proposal-using-tally).
+Proposals can be submitted via any means of accessing the DAO governance smart contracts; the Tally UI is recommended. 
 
 
-## Submitting AIP-1.2
-For our example of AIP-1.2, do the following in particular:
-- **Governor Selection (step 4):** 
+## Submitting A Core Proposal
+- **Start create proposal** Navigate to https://www.tally.xyz/gov/arbitrum and click "Create new proposal".
+- **Governor Selection** 
     - Select the "Arbitrum Core" governor. 
-        - _Explanation: AIP-1.2 updates the DAO Constitution Hash and sets the proposal thresholds for both governors to new values; both these these are constitutional actions, so the Core Governor must be used._ 
-- **Proposal Title / Description (step 5):** 
-    - Give the proposal a title and description; the title and description should be the same as the title and description in the corresponding [snapshot post](https://snapshot.org/#/arbitrumfoundation.eth/proposal/0x373dfa89fc9c5ccba8ed83fa3fa4f233edd270075b5f8f4f3902b408318d9d17), with any info about steps prior to proposal submission removed. You can use the description the [proposal JSON data here](../scripts/proposals/AIP12/data/42161-AIP1.2-data.json).
-- **Proposal Action (step 6)**: 
-    1. Select `Custom Action`
+- **Proposal Title / Description** 
+    - Give the proposal a title and description; the title and description may be similar to the title and description in the corresponding snapshot post, tho adding additional info to the description at this stage may be appropriate. 
+- **Proposal Action**: 
+    1. Select`Add Action` and then `Custom Action`
     1. In the `Enter the Target Address` Field" use `0x0000000000000000000000000000000000000064` (the ArbSys precompile).
     1. In the `Contract method` dropdown, select `sendTxToL1`.
-    1. For the `destination` address field, use the [l1ArbitrumTimelock](https://etherscan.io/address/0xE6841D92B0C345144506576eC13ECf5103aC7f49#readProxyContract) address (`0xE6841D92B0C345144506576eC13ECf5103aC7f49`) as provided in the [proposal JSON data](../scripts/proposals/AIP12/data/42161-AIP1.2-data.json).
-         - _Explanation: all executable core governor proposals will use the previous 4 steps; since constitutional proposals require a "round trip" before effectuating, their first step is to use ArbSys to encode an L2 to L1 message to the L1 timelock; for more, see ["Proposal Lifecycle Example"](./proposal_lifecycle_example.md)._ 
-    1. For the `data` bytes field, use the `calldata` bytes in the [proposal JSON data](../scripts/proposals/AIP12/data/42161-AIP1.2-data.json). See below for instructions on regenerating / verifying the calldata locally.
-        - _Explanation: all defining details of a particular core governor proposal are encoding in this calldata, i.e., the [governance action contract address](https://arbiscan.io/address/0x6274106eedD4848371D2C09e0352d67B795ED516) as well as the appropriate inbox/upgrade executor addresses such that the proposal targets the appropriate chain (in this case, Arbitrum One). We use scripts to generate a proposal's calldata_
-    1. For the `ETH`/ `value` field, use `0`.
-        - _Explanation: The Governor allows proposals to include callvalue, tho the vast majority of proposals won't require any_.
+    1. For the `destination` address field, use the [l1ArbitrumTimelock](https://etherscan.io/address/0xE6841D92B0C345144506576eC13ECf5103aC7f49#readProxyContract) address (`0xE6841D92B0C345144506576eC13ECf5103aC7f49`) as provided in the proposal JSON data.
+    1. For the `data` bytes field, use the `calldata` bytes in the proposal JSON data. 
+    1. Unless the proposal requires value to send (which is unusual) set 0 in the `value` field.
+- **Publishing**    
+    - Double check all values. When ready, select "publish" or "save draft" to send to a delegate to publish; note that a mimimum of 1,000,000 voting power is required to publish a proposal. 
+    
 
-## Generating AIP-1.2 Calldata Locally 
-Note that the data is already generated and committed to the git repo; run these steps to regeneate / confirm it.
-1. git clone this repo
-1. run `yarn install` and `yarn build`
-1. Set the following environment variables:
-    - `ARB_URL`: An RPC endpoint for Arbitrum One, e.g. "https://arb1.arbitrum.io/rpc"
-    - `ETH_URL`: An RPC endpoint for L1 Ethereun, e.g., "https://mainnet.infura.io/v3/YOUR-KEY-HERE"
-1. Run:  `yarn ts-node  ./scripts/proposals/AIP12/generateProposalData.ts`
-Output will appear in logs and will be written to the [proposal JSON data](../scripts/proposals/AIP12/data/42161-AIP1.2-data.json) file.
-
-
-## Submitting AIP-1.1
-- **Governor Selection (step 4):** 
-    - Select "Arbitrum Treasury" governor.
-        - _Explanation: AIP-1.1 deals with the Arbitrum's Foundation budget management and relevant transparency; as per the constitution, this qualifies it as a "non-constitution" proposal — such propoposals use the Treasury governor._
-- **Proposal Title / Description (step 5):** 
-    - The on-chain proposal should have the same title and description as its corresponding [snapshot post](https://snapshot.org/#/arbitrumfoundation.eth/proposal/0x7203289844e807781e8d2ec110d4b97a79a29944cae06a52dbe315a16381a2ae).
-- **Proposal Action (step 6)**: 
-    1. Select `Custom Action`
-    1. In the `Enter the Target Address` Field" use `0x9E43f733Da0445b35f038FB34a6Fb8C2947B984C`, a contract ("AIP1Point1Target") deployed for AIP-1.1 
-    1. In the `Contract method` dropdown, select `effectuate`.
-    1. For the `ETH`/ `value` field, use `0`.
-        - _Explanation: AIP-1.1 technically doesn't require any on-chain execution. We use a designated contract anyway as a formality and for bookkeeping purposes_
+You can also review the 7 general steps to submit a proposal using Tally in the Arbitrum Foundation docs [here](https://docs.arbitrum.foundation/how-tos/create-submit-dao-proposal#step-2-submit-your-on-chain-proposal-using-tally).
diff --git a/src/gov-action-contracts/README.md b/src/gov-action-contracts/README.md
@@ -10,14 +10,13 @@ This subdirectory includes a number of Governance Action Contracts Arbitrum Gove
 
 The following standards/guidelines for GACs are meant to maximize the work (engineering, auditing, etc.) that can be performed prior to the point at which an upgrade is planned, and minimize the potential for human error when/if an upgrade is planned for execution. They also tend towards simplicity and consistency:
 
-1. A GAC should have only one external function, and it should be named `perform`. 
-2. `perform` should accept as few parameters as possible, only accepting parameters where absolutely necessary. 
-    Examples:
-    - Instead of `perform` accepting a boolean parameter, create two distinct Upgrade Contracts, one which implements the `true` condition and one which implements the `false` condition.
-    - Instead of `perform` accepting the contract address of a core protocol contract on (say) either Arbitrum One or Arbitrum Nova, deploy two GACs, which get access to the appropriate contract address via constructor params.
-3. GACs should not access or modify their own state (since they are called via `delegatecall`). Any state variables on the contract should thus be `immutable` or `constant` (as these are not stored in state).
-4. GACs contract names should be suffixed with `Action`.
-5. GACs should revert on failure.
+
+1. GACs **must** be safe targets for delegatecall; in particular, they must use no state variables, and only use variables stored in bytecode (`immutable` or `constant` variables.) 
+1. A GAC should have only one external function; it should be named `perform` and it should take no parameters. 
+1. If the GAC needs to use a core protocol contract address, it should retrieve it from one of the [Address Registry contracts](../../src/gov-action-contracts/address-registries/L1AddressRegistry.sol) and set its value in the GAC's constructor (as opposed to passing int the core protocol contract address directly). The deployed addresses of the address registry contracts can be found in the mainnet [deployedContracts.json](../../files/mainnet/deployedContracts.json) file.
+1. A GAC should typically be composed of a general version with values set in the constructor, and then a contract specific to the proposal that inherits the general version, sets all parameters in the parent constructor, and includes no constructor parameters itself (see below).
+1. GACs should preform checks on the expected state before and after the action's execution if possible, and should explicity revert on failure.
+1. GACs contract names should be suffixed with `Action`.
 
 ## Using libraries
 The GAC author should also consider writing the GAC `perform` function logic in a Solidity library, then calling the library from the GAC. When doing this, library authors should make the library functions internal, as an external function will trigger another delegate call. 
@@ -28,3 +27,64 @@ The advantages of using a library are:
 The disadvantage of this approach is that calling multiple libraries from a GAC may lead to a confusing and complicated call path. 
 
 Judging this tradeoff is up to the author, but the focus should always be on readiblity and auditability of the code.
+
+## Example
+In following example, we show an action contract for setting the address 0xa4b174a3D79899E41aA7180f7934fa7a9f63C52F (arbitrarily chose for this example) as a batch-poster on Arbitrum Nova; this involves a simple call to the `sequencerInbox.setIsBatchPoster` method. We include annotation emphasizing the guidelines above:
+
+
+
+```solidity
+// General version of contract, with values set in constructor params, for testing and potential re-use in future proposals.
+// Detailed contract name, suffixed with "Action"
+contract SetIsBatchPosterAction {
+    // No state variables; values are all immutable for safe delegatecalling.
+    // Values are also public for ease of external verification.
+    ISequencerInbox public immutable sequencerInbox;
+    address public immutable batchPoster;
+    bool public immutable newBatchPosterstatus;
+
+    constructor(
+        ISequencerInboxGetter _l1AddressRegistry,
+        address _batchPoster,
+        bool _newBatchPosterstatus
+    ) {
+        // Sequencer inbox is not passed in as param; instead, address registry is, and sequencerInbox is retrieved and then set to immutable variable.
+        sequencerInbox = _l1AddressRegistry.sequencerInbox();
+        batchPoster = _batchPoster;
+        newBatchPosterstatus = _newBatchPosterstatus;
+    }
+
+    // Only external method is perform with no parameters
+    function perform() external {
+        // Preform the expected prior state sanity check; revert on failure.
+        require(
+            sequencerInbox.isBatchPoster(batchPoster) != newBatchPosterstatus,
+            "SetIsBatchPosterAction prior batch poster status"
+        );
+
+        // Perform the external call; note that it's the UpgradeExecutor, with the ownership affordance to call setIsBatchPoster,
+        // that will be making this call (by delegating to this action contract.)
+        sequencerInbox.setIsBatchPoster(batchPoster, newBatchPosterstatus);
+
+        // Perform the expected post state sanity check; revert on failure.
+        require(
+            sequencerInbox.isBatchPoster(batchPoster) == newBatchPosterstatus,
+            "SetIsBatchPosterAction post batch poster status"
+        );
+    }
+}
+// Child contract with values specific to this proposal
+// Detailed name, suffixed with "Action"
+contract SetNovaBatchPosterAction is SetIsBatchPosterAction {
+    // constructor takes no parameters
+    constructor()
+        // all values are set in parent constructor
+        SetIsBatchPosterAction(
+            0x2F06643fc2CC18585Ae790b546388F0DE4Ec6635, // L1 address registry for Nova
+            0xa4B174a3d79899e41Aa7180F7934fA7a9F63C52F, // batch poster
+            true // status to set
+        )
+    {}
+}
+
+```
