From f9e584eba901644af2be8a9fed7b0346ad033c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Fri, 13 Oct 2023 10:40:22 +0100 Subject: [PATCH 1/3] Renaming the space plugin methods and events --- README.md | 8 +++--- packages/contracts/src/SpacePlugin.sol | 10 +++---- .../personal-space-admin-plugin.ts | 4 +-- .../test/unit-testing/space-plugin.ts | 28 +++++++++++++------ .../src/internal/modules/encoding.ts | 6 +++- 5 files changed, 36 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 7fe766e..9259476 100644 --- a/README.md +++ b/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 @@ -216,7 +216,7 @@ This plugin is upgradeable. #### Methods - `function initialize(IDAO _dao, string _firstBlockContentUri)` -- `function setContent(uint32 _blockIndex, uint32 _itemIndex, string _contentUri)` +- `function processGeoProposal(uint32 _blockIndex, uint32 _itemIndex, string _contentUri)` - `function acceptSubspace(address _dao)` - `function removeSubspace(address _dao)` @@ -233,13 +233,13 @@ Inherited: #### Events -- `event ContentChanged(uint32 blockIndex, uint32 itemIndex, string contentUri)` +- `event GeoProposalProcessed(uint32 blockIndex, uint32 itemIndex, string contentUri)` - `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 diff --git a/packages/contracts/src/SpacePlugin.sol b/packages/contracts/src/SpacePlugin.sol index 794946c..fc9b657 100644 --- a/packages/contracts/src/SpacePlugin.sol +++ b/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,7 @@ 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 Emitted when the DAO accepts another DAO as a subspace. /// @param subspaceDao The address of the DAO to be accepted as a subspace. @@ -32,7 +32,7 @@ contract SpacePlugin is PluginUUPSUpgradeable { function initialize(IDAO _dao, string memory _firstBlockContentUri) external initializer { __PluginUUPSUpgradeable_init(_dao); - emit ContentChanged({blockIndex: 0, itemIndex: 0, contentUri: _firstBlockContentUri}); + emit GeoProposalProcessed({blockIndex: 0, itemIndex: 0, contentUri: _firstBlockContentUri}); } /// @notice Checks if this or the parent contract supports an interface by its ID. @@ -48,12 +48,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 diff --git a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts index 00dc697..822dcf5 100644 --- a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts +++ b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts @@ -249,7 +249,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 +283,7 @@ describe("Personal Space Admin Plugin", function () { actions, 0, ), - ).to.emit(spacePlugin, "ContentChanged") + ).to.emit(spacePlugin, "GeoProposalProcessed") .withArgs(1, 2, "0x"); }); diff --git a/packages/contracts/test/unit-testing/space-plugin.ts b/packages/contracts/test/unit-testing/space-plugin.ts index 07a3957..05dab6b 100644 --- a/packages/contracts/test/unit-testing/space-plugin.ts +++ b/packages/contracts/test/unit-testing/space-plugin.ts @@ -52,7 +52,7 @@ describe("Space Plugin", function () { 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 +69,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 +146,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 +169,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")); }); diff --git a/packages/js-client/src/internal/modules/encoding.ts b/packages/js-client/src/internal/modules/encoding.ts index 0a13568..96a0d97 100644 --- a/packages/js-client/src/internal/modules/encoding.ts +++ b/packages/js-client/src/internal/modules/encoding.ts @@ -21,7 +21,11 @@ export class MyPluginClientEncoding extends ClientCore // implementation of the methods in the interface public storeNumberAction(): DaoAction { const iface = SpacePlugin__factory.createInterface(); - const data = iface.encodeFunctionData("setContent", [1, 4, "ipfs://...."]); + const data = iface.encodeFunctionData("processGeoProposal", [ + 1, + 4, + "ipfs://....", + ]); return { to: this.spacePluginAddress, From 82baf6cb287f78c49e93155fc5b09a0c88a68125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Fri, 13 Oct 2023 12:56:32 +0100 Subject: [PATCH 2/3] Adding an event to announce the space's predecessor --- README.md | 3 +- packages/contracts/src/SpacePlugin.sol | 14 ++++- packages/contracts/src/SpacePluginSetup.sol | 16 ++++-- .../contracts/src/space-build-metadata.json | 5 ++ .../test/unit-testing/main-voting-plugin.ts | 12 ++++- .../test/unit-testing/member-access-plugin.ts | 12 ++++- .../personal-space-admin-plugin.ts | 6 ++- .../test/unit-testing/space-plugin-setup.ts | 6 +-- .../test/unit-testing/space-plugin.ts | 52 ++++++++++++++++++- 9 files changed, 109 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 9259476..c618008 100644 --- a/README.md +++ b/README.md @@ -215,7 +215,7 @@ This plugin is upgradeable. #### Methods -- `function initialize(IDAO _dao, string _firstBlockContentUri)` +- `function initialize(IDAO _dao, string _firstBlockContentUri, address predecessorSpace)` - `function processGeoProposal(uint32 _blockIndex, uint32 _itemIndex, string _contentUri)` - `function acceptSubspace(address _dao)` - `function removeSubspace(address _dao)` @@ -234,6 +234,7 @@ Inherited: #### Events - `event GeoProposalProcessed(uint32 blockIndex, uint32 itemIndex, string contentUri)` +- `event SuccessorSpaceCreated(address predecessorSpace)` - `event SubspaceAccepted(address dao)` - `event SubspaceRemoved(address dao)` diff --git a/packages/contracts/src/SpacePlugin.sol b/packages/contracts/src/SpacePlugin.sol index fc9b657..e733916 100644 --- a/packages/contracts/src/SpacePlugin.sol +++ b/packages/contracts/src/SpacePlugin.sol @@ -18,6 +18,10 @@ contract SpacePlugin is PluginUUPSUpgradeable { /// @param contentUri An IPFS URI pointing to the new contents behind the block's item. 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. event SubspaceAccepted(address subspaceDao); @@ -29,9 +33,17 @@ 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); + if (_predecessorSpace != address(0)) { + emit SuccessorSpaceCreated(_predecessorSpace); + } emit GeoProposalProcessed({blockIndex: 0, itemIndex: 0, contentUri: _firstBlockContentUri}); } diff --git a/packages/contracts/src/SpacePluginSetup.sol b/packages/contracts/src/SpacePluginSetup.sol index e2df1a0..1f5c99a 100644 --- a/packages/contracts/src/SpacePluginSetup.sol +++ b/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[] diff --git a/packages/contracts/src/space-build-metadata.json b/packages/contracts/src/space-build-metadata.json index b65c31d..086c58d 100644 --- a/packages/contracts/src/space-build-metadata.json +++ b/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", diff --git a/packages/contracts/test/unit-testing/main-voting-plugin.ts b/packages/contracts/test/unit-testing/main-voting-plugin.ts index af6b644..721a68b 100644 --- a/packages/contracts/test/unit-testing/main-voting-plugin.ts +++ b/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"); }); diff --git a/packages/contracts/test/unit-testing/member-access-plugin.ts b/packages/contracts/test/unit-testing/member-access-plugin.ts index 0026e15..2ebf1d5 100644 --- a/packages/contracts/test/unit-testing/member-access-plugin.ts +++ b/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"); }); diff --git a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts index 822dcf5..9eb47a3 100644 --- a/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts +++ b/packages/contracts/test/unit-testing/personal-space-admin-plugin.ts @@ -86,7 +86,11 @@ describe("Personal Space Admin Plugin", function () { spacePlugin = await deployWithProxy( 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( diff --git a/packages/contracts/test/unit-testing/space-plugin-setup.ts b/packages/contracts/test/unit-testing/space-plugin-setup.ts index 8458517..2b8c379 100644 --- a/packages/contracts/test/unit-testing/space-plugin-setup.ts +++ b/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, diff --git a/packages/contracts/test/unit-testing/space-plugin.ts b/packages/contracts/test/unit-testing/space-plugin.ts index 05dab6b..25c0d6c 100644 --- a/packages/contracts/test/unit-testing/space-plugin.ts +++ b/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,15 +40,62 @@ 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( + 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( + 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( + 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 () => { From 7b35ed14e8906ef032932d17b79e50f146076eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8r=E2=88=82=C2=A1?= Date: Fri, 13 Oct 2023 13:40:16 +0100 Subject: [PATCH 3/3] Adapting the client --- .../contracts/test/integration-testing/deployment.ts | 2 +- .../test/integration-testing/space-setup.ts | 2 +- packages/contracts/utils/ipfs.ts | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/contracts/test/integration-testing/deployment.ts b/packages/contracts/test/integration-testing/deployment.ts index 8203ce7..e1f6f48 100644 --- a/packages/contracts/test/integration-testing/deployment.ts +++ b/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; diff --git a/packages/contracts/test/integration-testing/space-setup.ts b/packages/contracts/test/integration-testing/space-setup.ts index d01bd29..734f575 100644 --- a/packages/contracts/test/integration-testing/space-setup.ts +++ b/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); diff --git a/packages/contracts/utils/ipfs.ts b/packages/contracts/utils/ipfs.ts index fa0b311..0ebcf06 100644 --- a/packages/contracts/utils/ipfs.ts +++ b/packages/contracts/utils/ipfs.ts @@ -1,4 +1,4 @@ -import { BytesLike, ethers } from "ethers"; +import { ethers } from "ethers"; import IPFS from "ipfs-http-client"; export async function uploadToIPFS( @@ -7,16 +7,16 @@ export async function uploadToIPFS( ): Promise { const client = IPFS.create({ url: testing - ? "https://testing-ipfs-0.aragon.network/api/v0" - : "https://ipfs-0.aragon.network/api/v0", + ? "https://test.ipfs.aragon.network/api/v0" + : "https://prod.ipfs.aragon.network/api/v0", headers: { "X-API-KEY": "b477RhECf8s8sdM7XrkLBs2wHc4kCMwpbcFC55Kt", }, }); - const cid = await client.add(content); - await client.pin.add(cid.cid); - return cid.path; + const result = await client.add(content); + await client.pin.add(result.cid); + return result.path || result.cid.toString(); } export function toHex(input: string): string {