Merge pull request #1 from aragon/fix/space-plugin-suggestions

Space plugin suggestions

README.md

@@ -61,7 +61,7 @@ The current repository provides the plugins necessary to cover two use cases:
 ### Emitting content and managing subspaces
 
 1. When a proposal regarding the space is passed, the `MainVotingPlugin` will call `execute()` on the DAO
-2. The actions from the proposal will target the `setContent()`, `acceptSubspace()` or `removeSubspace()` functions on the `SpacePlugin`.
+2. The actions from the proposal will target the `processGeoProposal()`, `acceptSubspace()` or `removeSubspace()` functions on the `SpacePlugin`.
 3. The `SpacePlugin` will be called by the DAO and emit the corresponding events
 4. An external indexer will fetch all these events and update the current state of this specific space
 
@@ -215,8 +215,8 @@ This plugin is upgradeable.
 
 #### Methods
 
-- `function initialize(IDAO _dao, string _firstBlockContentUri)`
-- `function setContent(uint32 _blockIndex, uint32 _itemIndex, string _contentUri)`
+- `function initialize(IDAO _dao, string _firstBlockContentUri, address predecessorSpace)`
+- `function processGeoProposal(uint32 _blockIndex, uint32 _itemIndex, string _contentUri)`
 - `function acceptSubspace(address _dao)`
 - `function removeSubspace(address _dao)`
 
@@ -233,13 +233,14 @@ Inherited:
 
 #### Events
 
-- `event ContentChanged(uint32 blockIndex, uint32 itemIndex, string contentUri)`
+- `event GeoProposalProcessed(uint32 blockIndex, uint32 itemIndex, string contentUri)`
+- `event SuccessorSpaceCreated(address predecessorSpace)`
 - `event SubspaceAccepted(address dao)`
 - `event SubspaceRemoved(address dao)`
 
 #### Permissions
 
-- The DAO can call `setContent()` on the plugin
+- The DAO can call `processGeoProposal()` on the plugin
 - The DAO can accept/remove a subspace on the plugin
 - The DAO can upgrade the plugin
 - Optionally, a given pluginUpgrader can upgrade the plugin

packages/contracts/src/SpacePlugin.sol

@@ -5,7 +5,7 @@ import {IDAO, PluginUUPSUpgradeable} from "@aragon/osx/core/plugin/PluginUUPSUpg
 import {CONTENT_PERMISSION_ID, SUBSPACE_PERMISSION_ID} from "./constants.sol";
 
 bytes4 constant SPACE_INTERFACE_ID = SpacePlugin.initialize.selector ^
-    SpacePlugin.setContent.selector ^
+    SpacePlugin.processGeoProposal.selector ^
     SpacePlugin.acceptSubspace.selector ^
     SpacePlugin.removeSubspace.selector;
 
@@ -16,7 +16,11 @@ contract SpacePlugin is PluginUUPSUpgradeable {
     /// @param blockIndex The index of the block whose items have new contents.
     /// @param itemIndex The index of the item that has new contents.
     /// @param contentUri An IPFS URI pointing to the new contents behind the block's item.
-    event ContentChanged(uint32 blockIndex, uint32 itemIndex, string contentUri);
+    event GeoProposalProcessed(uint32 blockIndex, uint32 itemIndex, string contentUri);
+
+    /// @notice Announces that the current space plugin is the successor of an already existing Space
+    /// @param predecessorSpace The address of the space contract that the plugin will replace
+    event SuccessorSpaceCreated(address predecessorSpace);
 
     /// @notice Emitted when the DAO accepts another DAO as a subspace.
     /// @param subspaceDao The address of the DAO to be accepted as a subspace.
@@ -29,10 +33,18 @@ contract SpacePlugin is PluginUUPSUpgradeable {
     /// @notice Initializes the plugin when build 1 is installed.
     /// @param _dao The address of the DAO to read the permissions from.
     /// @param _firstBlockContentUri A IPFS URI pointing to the contents of the first block's item (title).
-    function initialize(IDAO _dao, string memory _firstBlockContentUri) external initializer {
+    /// @param _predecessorSpace Optionally, the address of the space contract preceding this one
+    function initialize(
+        IDAO _dao,
+        string memory _firstBlockContentUri,
+        address _predecessorSpace
+    ) external initializer {
         __PluginUUPSUpgradeable_init(_dao);
 
-        emit ContentChanged({blockIndex: 0, itemIndex: 0, contentUri: _firstBlockContentUri});
+        if (_predecessorSpace != address(0)) {
+            emit SuccessorSpaceCreated(_predecessorSpace);
+        }
+        emit GeoProposalProcessed({blockIndex: 0, itemIndex: 0, contentUri: _firstBlockContentUri});
     }
 
     /// @notice Checks if this or the parent contract supports an interface by its ID.
@@ -48,12 +60,12 @@ contract SpacePlugin is PluginUUPSUpgradeable {
     /// @param _blockIndex The index of the block whose items have new contents.
     /// @param _itemIndex The index of the item that has new contents.
     /// @param _contentUri An IPFS URI pointing to the new contents behind the block's item.
-    function setContent(
+    function processGeoProposal(
         uint32 _blockIndex,
         uint32 _itemIndex,
         string memory _contentUri
     ) external auth(CONTENT_PERMISSION_ID) {
-        emit ContentChanged({
+        emit GeoProposalProcessed({
             blockIndex: _blockIndex,
             itemIndex: _itemIndex,
             contentUri: _contentUri

packages/contracts/src/SpacePluginSetup.sol

@@ -22,15 +22,21 @@ contract SpacePluginSetup is PluginSetup {
         bytes memory _data
     ) external returns (address plugin, PreparedSetupData memory preparedSetupData) {
         // Decode incoming params
-        (string memory firstBlockContentUri, address _pluginUpgrader) = abi.decode(
-            _data,
-            (string, address)
-        );
+        (
+            string memory _firstBlockContentUri,
+            address _predecessorAddress,
+            address _pluginUpgrader
+        ) = abi.decode(_data, (string, address, address));
 
         // Deploy new plugin instance
         plugin = createERC1967Proxy(
             pluginImplementation,
-            abi.encodeWithSelector(SpacePlugin.initialize.selector, _dao, firstBlockContentUri)
+            abi.encodeWithSelector(
+                SpacePlugin.initialize.selector,
+                _dao,
+                _firstBlockContentUri,
+                _predecessorAddress
+            )
         );
 
         PermissionLib.MultiTargetPermission[]

packages/contracts/src/space-build-metadata.json

@@ -11,6 +11,11 @@
           "internalType": "string",
           "description": "The inital contents of the first block item."
         },
+        {
+          "internalType": "address",
+          "name": "predecessorAddress",
+          "type": "address"
+        },
         {
           "internalType": "address",
           "name": "pluginUpgrader",

packages/contracts/test/integration-testing/deployment.ts

@@ -115,7 +115,7 @@ describe("PluginRepo Deployment", function () {
           switch (pluginSetupParams.PLUGIN_SETUP_CONTRACT_NAME) {
             case SpacePluginSetupParams.PLUGIN_SETUP_CONTRACT_NAME:
               expect(receivedStriMetadata).to.equal(
-                "ipfs://QmdEAfaFb6iYW4UKE7cnkY3DD3uXdSe2ZnRExjJRbJSfNN",
+                "ipfs://QmcAUbh4UmhwZp4b7aQf9nkpemhCfTVms2eSVng1bZUmmo",
               );
               break;
 

packages/contracts/test/integration-testing/space-setup.ts

@@ -113,7 +113,7 @@ describe("SpacePluginSetup processing", function () {
             .prepareInstallation
             .inputs,
         ),
-        [toHex("ipfs://1234"), pluginUpgrader],
+        [toHex("ipfs://1234"), ADDRESS_ZERO, pluginUpgrader],
       );
       const results = await installPlugin(psp, dao, pluginSetupRef, data);
 

packages/contracts/test/unit-testing/main-voting-plugin.ts

@@ -92,7 +92,11 @@ describe("Main Voting Plugin", function () {
       defaultMainVotingSettings,
       [alice.address],
     );
-    await spacePlugin.initialize(dao.address, defaultInput.contentUri);
+    await spacePlugin.initialize(
+      dao.address,
+      defaultInput.contentUri,
+      ADDRESS_ZERO,
+    );
 
     // Alice is already an editor (see initialize)
 
@@ -153,7 +157,11 @@ describe("Main Voting Plugin", function () {
         ),
       ).to.be.revertedWith("Initializable: contract is already initialized");
       await expect(
-        spacePlugin.initialize(dao.address, defaultInput.contentUri),
+        spacePlugin.initialize(
+          dao.address,
+          defaultInput.contentUri,
+          ADDRESS_ZERO,
+        ),
       ).to.be.revertedWith("Initializable: contract is already initialized");
     });
 

packages/contracts/test/unit-testing/member-access-plugin.ts

@@ -95,7 +95,11 @@ describe("Member Access Plugin", function () {
       defaultMainVotingSettings,
       [alice.address],
     );
-    await spacePlugin.initialize(dao.address, defaultInput.contentUri);
+    await spacePlugin.initialize(
+      dao.address,
+      defaultInput.contentUri,
+      ADDRESS_ZERO,
+    );
 
     // Alice is an editor (see mainVotingPlugin initialize)
 
@@ -1381,7 +1385,11 @@ describe("Member Access Plugin", function () {
           ),
         ).to.be.revertedWith("Initializable: contract is already initialized");
         await expect(
-          spacePlugin.initialize(dao.address, defaultInput.contentUri),
+          spacePlugin.initialize(
+            dao.address,
+            defaultInput.contentUri,
+            ADDRESS_ZERO,
+          ),
         ).to.be.revertedWith("Initializable: contract is already initialized");
       });
 

packages/contracts/test/unit-testing/personal-space-admin-plugin.ts

@@ -86,7 +86,11 @@ describe("Personal Space Admin Plugin", function () {
     spacePlugin = await deployWithProxy<SpacePlugin>(
       new SpacePlugin__factory(alice),
     );
-    await spacePlugin.initialize(dao.address, defaultInput.contentUri);
+    await spacePlugin.initialize(
+      dao.address,
+      defaultInput.contentUri,
+      ADDRESS_ZERO,
+    );
 
     // Personal Space Voting
     const PersonalSpaceVotingFactory = new PersonalSpaceAdminPlugin__factory(
@@ -249,7 +253,7 @@ describe("Personal Space Admin Plugin", function () {
     it("Executed content proposals emit an event", async () => {
       // Encode an action to change some content
       const data = SpacePlugin__factory.createInterface().encodeFunctionData(
-        "setContent",
+        "processGeoProposal",
         [1, 2, "0x"],
       );
       const actions = [
@@ -283,7 +287,7 @@ describe("Personal Space Admin Plugin", function () {
           actions,
           0,
         ),
-      ).to.emit(spacePlugin, "ContentChanged")
+      ).to.emit(spacePlugin, "GeoProposalProcessed")
         .withArgs(1, 2, "0x");
     });
 

packages/contracts/test/unit-testing/space-plugin-setup.ts

@@ -9,7 +9,7 @@ import { deployTestDao } from "../helpers/test-dao";
 import { getNamedTypesFromMetadata, Operation } from "../helpers/types";
 import {
   abiCoder,
-  ADDRESS_TWO,
+  ADDRESS_ONE,
   ADDRESS_ZERO,
   CONTENT_PERMISSION_ID,
   NO_CONDITION,
@@ -42,7 +42,7 @@ describe("Space Plugin Setup", function () {
         getNamedTypesFromMetadata(
           buildMetadata.pluginSetup.prepareInstallation.inputs,
         ),
-        [defaultInitData.contentUri, ADDRESS_ZERO],
+        [defaultInitData.contentUri, ADDRESS_ZERO, ADDRESS_ZERO],
       );
       const nonce = await ethers.provider.getTransactionCount(
         spacePluginSetup.address,
@@ -100,7 +100,7 @@ describe("Space Plugin Setup", function () {
         getNamedTypesFromMetadata(
           buildMetadata.pluginSetup.prepareInstallation.inputs,
         ),
-        [defaultInitData.contentUri, pluginUpgrader],
+        [defaultInitData.contentUri, ADDRESS_ONE, pluginUpgrader],
       );
       const nonce = await ethers.provider.getTransactionCount(
         spacePluginSetup.address,

packages/contracts/test/unit-testing/space-plugin.ts

@@ -5,6 +5,7 @@ import { deployTestDao } from "../helpers/test-dao";
 import {
   ADDRESS_ONE,
   ADDRESS_TWO,
+  ADDRESS_ZERO,
   CONTENT_PERMISSION_ID,
   EXECUTE_PERMISSION_ID,
   SUBSPACE_PERMISSION_ID,
@@ -39,20 +40,67 @@ describe("Space Plugin", function () {
       new SpacePlugin__factory(alice),
     );
 
-    await spacePlugin.initialize(dao.address, defaultInput.contentUri);
+    await spacePlugin.initialize(
+      dao.address,
+      defaultInput.contentUri,
+      ADDRESS_ZERO,
+    );
   });
 
   describe("initialize", async () => {
     it("The Space plugin reverts if trying to re-initialize", async () => {
       await expect(
-        spacePlugin.initialize(dao.address, defaultInput.contentUri),
+        spacePlugin.initialize(
+          dao.address,
+          defaultInput.contentUri,
+          ADDRESS_ZERO,
+        ),
       ).to.be.revertedWith("Initializable: contract is already initialized");
     });
+
+    it("Should emit a new content event", async () => {
+      spacePlugin = await deployWithProxy<SpacePlugin>(
+        new SpacePlugin__factory(alice),
+      );
+
+      await expect(spacePlugin.initialize(
+        dao.address,
+        defaultInput.contentUri,
+        ADDRESS_ZERO,
+      )).to.emit(spacePlugin, "GeoProposalProcessed")
+        .withArgs(0, 0, defaultInput.contentUri);
+    });
+
+    it("Should emit a successor space event", async () => {
+      // 1
+      spacePlugin = await deployWithProxy<SpacePlugin>(
+        new SpacePlugin__factory(alice),
+      );
+
+      await expect(spacePlugin.initialize(
+        dao.address,
+        defaultInput.contentUri,
+        ADDRESS_ONE,
+      )).to.emit(spacePlugin, "SuccessorSpaceCreated")
+        .withArgs(ADDRESS_ONE);
+
+      // 2
+      spacePlugin = await deployWithProxy<SpacePlugin>(
+        new SpacePlugin__factory(alice),
+      );
+
+      await expect(spacePlugin.initialize(
+        dao.address,
+        defaultInput.contentUri,
+        ADDRESS_TWO,
+      )).to.emit(spacePlugin, "SuccessorSpaceCreated")
+        .withArgs(ADDRESS_TWO);
+    });
   });
 
   it("The Space plugin emits an event when new content is published", async () => {
     // Fails by default
-    await expect(spacePlugin.connect(alice).setContent(1, 2, "hello"))
+    await expect(spacePlugin.connect(alice).processGeoProposal(1, 2, "hello"))
       .to.be.revertedWithCustomError(spacePlugin, "DaoUnauthorized")
       .withArgs(
         dao.address,
@@ -69,8 +117,8 @@ describe("Space Plugin", function () {
     );
 
     // Set content
-    await expect(spacePlugin.connect(alice).setContent(1, 2, "hello"))
-      .to.emit(spacePlugin, "ContentChanged")
+    await expect(spacePlugin.connect(alice).processGeoProposal(1, 2, "hello"))
+      .to.emit(spacePlugin, "GeoProposalProcessed")
       .withArgs(1, 2, "hello");
   });
 
@@ -146,13 +194,21 @@ describe("Space Plugin", function () {
     it("Only the DAO can emit content on the space plugin", async () => {
       // They cannot
       await expect(
-        spacePlugin.connect(alice).setContent(1, 2, toHex("ipfs://1234")),
+        spacePlugin.connect(alice).processGeoProposal(
+          1,
+          2,
+          toHex("ipfs://1234"),
+        ),
       ).to.be.reverted;
       await expect(
-        spacePlugin.connect(bob).setContent(1, 2, toHex("ipfs://1234")),
+        spacePlugin.connect(bob).processGeoProposal(1, 2, toHex("ipfs://1234")),
       ).to.be.reverted;
       await expect(
-        spacePlugin.connect(carol).setContent(1, 2, toHex("ipfs://1234")),
+        spacePlugin.connect(carol).processGeoProposal(
+          1,
+          2,
+          toHex("ipfs://1234"),
+        ),
       ).to.be.reverted;
 
       // The DAO can
@@ -161,12 +217,16 @@ describe("Space Plugin", function () {
           to: spacePlugin.address,
           value: 0,
           data: SpacePlugin__factory.createInterface()
-            .encodeFunctionData("setContent", [1, 2, toHex("ipfs://1234")]),
+            .encodeFunctionData("processGeoProposal", [
+              1,
+              2,
+              toHex("ipfs://1234"),
+            ]),
         },
       ];
 
       await expect(dao.execute(ZERO_BYTES32, actions, 0)).to
-        .emit(spacePlugin, "ContentChanged")
+        .emit(spacePlugin, "GeoProposalProcessed")
         .withArgs(1, 2, toHex("ipfs://1234"));
     });