F/amended osx commons

CHANGELOG.md

@@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## v1.2
+
+### Changed
+
+- Recompiled against an amended `osx-commons` `RuledCondition._evalLogic`. The IF_ELSE starting rule now evaluates with `_where`/`_who` in the same order as the surrounding `_evalRule` call. No setup interface change.
+
+### Added
+
+- SPP-level regression tests for `SPPRuleCondition.isGranted` covering an asymmetric IF_ELSE predicate (success/failure routing and a swapped-args caller path).
+- Unit and fork tests for `prepareUpdate`.
+- `script/NewVersion.s.sol` now also prints the management DAO multisig `createProposal` calldata wrapping the `createVersion` action — including the pinned `PROPOSAL_METADATA` URI as the proposal metadata — so a multisig member can submit it directly.
+- `script/Deploy.s.sol` now publishes `PlaceholderSetup` builds for builds 1..VERSION_BUILD-1 on a fresh repo before publishing the real `SPPSetup` build, keeping on-chain build numbers aligned across networks.
+- `PROPOSAL_METADATA` and `PLACEHOLDER_BUILD_METADATA` constants in `PluginSettings.sol`, and `script/new-version-proposal-metadata.json` as the v1.2 proposal metadata source.
+
 ## v1.1
 
 ### Added

README.md

@@ -40,18 +40,40 @@ forge build
 ```shell
 just test                  # unit tests
 just test-fork             # fork tests (requires RPC_URL)
-just validate-upgrade SPPStorageV1 StagedProposalProcessor  # storage layout check
+just check-upgrade SPPStorageV1 StagedProposalProcessor    # storage layout compatibility check
 ```
 
 ## Deploy
 
 ```shell
 just deploy                # initial deployment (creates plugin repo, publishes v1)
-just new-version           # deploy new setup + print DAO proposal calldata
+just new-version           # deploy new setup + print management DAO multisig proposal calldata
 ```
 
 Set `SPP_ENS_SUBDOMAIN=spp` in `.env` for production deployments. Omitting it generates a unique name (`spp-<timestamp>`), which is useful for testing.
 
+### Publishing a new build
+
+1. Bump `VERSION_BUILD` in `src/utils/PluginSettings.sol`.
+2. Edit `src/build-metadata.json` and `script/new-version-proposal-metadata.json` for this build (and `src/release-metadata.json` if shipping a new release).
+3. Pin and update the matching constants in `PluginSettings.sol`:
+   ```shell
+   just ipfs-pin src/build-metadata.json                   # → BUILD_METADATA
+   just ipfs-pin script/new-version-proposal-metadata.json # → PROPOSAL_METADATA
+   just ipfs-pin src/release-metadata.json                 # → RELEASE_METADATA (only on a new release)
+   ```
+4. Run `just new-version`. The script deploys the new `SPPSetup` and prints two calldata blobs:
+   - the inner `createVersion` action (`to = SPP_PLUGIN_REPO_ADDRESS`), and
+   - the outer management DAO multisig `createProposal` call (`to = MANAGEMENT_DAO_MULTISIG_ADDRESS`) including the pinned `PROPOSAL_METADATA` URI — submit it from any listed multisig member to publish the version.
+
+On a brand-new network, `just deploy` automatically publishes `PlaceholderSetup` builds for any build numbers below `VERSION_BUILD` before publishing the real one, so build numbers stay aligned with networks where prior builds shipped.
+
+### Upgrading existing installations
+
+Publishing a new build does not upgrade installed plugins. Each DAO running an older build needs a proposal that calls `psp.applyUpdate(...)`. 
+
+Version 1.2 is published with the same `IMPLEMENTATION` as 1.1 (bytecode is identical), so `applyUpdate` skips the proxy upgrade — no `UPGRADE_PLUGIN_PERMISSION` grant/revoke bracket is required.
+
 ### Deployment Checklist
 
 - [ ] I have cloned the official repository on my computer and I have checked out the `main` branch
@@ -73,6 +95,7 @@ Set `SPP_ENS_SUBDOMAIN=spp` in `.env` for production deployments. Omitting it ge
   - [ ] I have created a new burner wallet with `cast wallet new` and used its private key as `DEPLOYER_KEY`
   - [ ] I am the only person of the ceremony that will operate the deployment wallet
 - [ ] All the tests run clean (`just test`)
+- [ ] `just check-upgrade OldContract NewContract` reports the storage layout check passed
 - My computer:
   - [ ] Is running in a safe location and using a trusted network
   - [ ] It exposes no services or ports

justfile

@@ -8,6 +8,12 @@ docs:
     cd docs-gen && bun install && bash prepare-docs.sh && bun prepare-docs.js
 
 DEPLOY_SCRIPT := "script/Deploy.s.sol:Deploy"
+NEW_VERSION_SCRIPT := "script/NewVersion.s.sol:NewVersion"
+
+# Dry-run the new-version script (no broadcast) — eyeball the printed multisig calldata
+[group('upgrade')]
+pre-new-version:
+    just dry-run {{ NEW_VERSION_SCRIPT }}
 
 # Publish a new SPP plugin version (deploys setup, prints DAO proposal calldata)
 [group('upgrade')]
@@ -18,6 +24,6 @@ new-version *args:
     mkdir -p logs
     LOG_FILE="logs/new-version-$NETWORK_NAME-$(date +"%y-%m-%d-%H-%M").log"
     just test 2>&1 | tee -a "$LOG_FILE"
-    just run script/NewVersion.s.sol:NewVersion {{ args }} 2>&1 | tee -a "$LOG_FILE"
+    just run {{ NEW_VERSION_SCRIPT }} {{ args }} 2>&1 | tee -a "$LOG_FILE"
     echo "Logs saved in $LOG_FILE"
 

script/Deploy.s.sol

@@ -10,6 +10,9 @@ import {StagedProposalProcessorSetup as SPPSetup} from "../src/StagedProposalPro
 
 import {PluginRepo} from "@aragon/osx/framework/plugin/repo/PluginRepo.sol";
 import {PluginRepoFactory} from "@aragon/osx/framework/plugin/repo/PluginRepoFactory.sol";
+import {
+    PlaceholderSetup
+} from "@aragon/osx/framework/plugin/repo/placeholder/PlaceholderSetup.sol";
 import {PermissionLib} from "@aragon/osx-commons-contracts/src/permission/PermissionLib.sol";
 import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
@@ -18,6 +21,8 @@ contract Deploy is BaseScript {
     error InvalidVersionBuild(uint8 build, uint8 latestBuild);
     error VersionPublishFailed();
 
+    PlaceholderSetup public placeholderSetup;
+
     function run() external {
         address pluginRepoFactory = vm.envAddress("PLUGIN_REPO_FACTORY_ADDRESS");
         address managementDao = vm.envAddress("MANAGEMENT_DAO_ADDRESS");
@@ -37,6 +42,20 @@ contract Deploy is BaseScript {
             revert InvalidVersionBuild(PluginSettings.VERSION_BUILD, uint8(latestBuild));
         }
 
+        // Fill builds 1..VERSION_BUILD-1 with PlaceholderSetup so build numbers stay
+        // aligned across networks (a fresh chain still ends up with build N == VERSION_BUILD).
+        if (PluginSettings.VERSION_BUILD > latestBuild + 1) {
+            placeholderSetup = new PlaceholderSetup();
+            for (uint8 i = uint8(latestBuild) + 1; i < PluginSettings.VERSION_BUILD; ++i) {
+                sppRepo.createVersion(
+                    PluginSettings.VERSION_RELEASE,
+                    address(placeholderSetup),
+                    bytes(PluginSettings.PLACEHOLDER_BUILD_METADATA),
+                    bytes(PluginSettings.RELEASE_METADATA)
+                );
+            }
+        }
+
         sppRepo.createVersion(
             PluginSettings.VERSION_RELEASE,
             address(sppSetup),
@@ -59,6 +78,9 @@ contract Deploy is BaseScript {
         console.log("- SPP PluginRepo:   ", address(sppRepo));
         console.log("- SPP PluginSetup:  ", address(sppSetup));
         console.log("- Implementation:   ", sppSetup.implementation());
+        if (address(placeholderSetup) != address(0)) {
+            console.log("- PlaceholderSetup: ", address(placeholderSetup));
+        }
         console.log(
             "- Version:          ",
             _versionString(PluginSettings.VERSION_RELEASE, PluginSettings.VERSION_BUILD)

script/NewVersion.s.sol

@@ -9,33 +9,112 @@ import {StagedProposalProcessor as SPP} from "../src/StagedProposalProcessor.sol
 import {StagedProposalProcessorSetup as SPPSetup} from "../src/StagedProposalProcessorSetup.sol";
 
 import {PluginRepo} from "@aragon/osx/framework/plugin/repo/PluginRepo.sol";
+import {Action} from "@aragon/osx-commons-contracts/src/executors/IExecutor.sol";
+import {IPluginSetup} from "@aragon/osx-commons-contracts/src/plugin/setup/IPluginSetup.sol";
 
-/// @notice Deploys a new SPPSetup implementation and prints the DAO proposal calldata.
-/// Submit the printed calldata as a management DAO proposal to publish the new version.
+/// @dev Minimal subset of the management DAO Multisig plugin ABI used by this script.
+/// Mirrors the 7-arg `createProposal` overload (selector `0xfbd56e41`); using this typed
+/// interface with `abi.encodeCall` makes the compiler enforce the signature match instead
+/// of trusting a string we hand to `abi.encodeWithSignature`.
+interface IManagementDaoMultisig {
+    function createProposal(
+        bytes calldata _metadata,
+        Action[] calldata _actions,
+        uint256 _allowFailureMap,
+        bool _approveProposal,
+        bool _tryExecution,
+        uint64 _startDate,
+        uint64 _endDate
+    ) external returns (uint256 proposalId);
+}
+
+/// @notice Deploys a new SPPSetup implementation and prints both the inner
+/// `createVersion` action and the outer management DAO multisig
+/// `createProposal` calldata. Submit the printed multisig calldata from any
+/// listed multisig member to publish this version.
 contract NewVersion is BaseScript {
     function run() external {
         sppRepo = PluginRepo(vm.envAddress("SPP_PLUGIN_REPO_ADDRESS"));
+        address managementDaoMultisig = vm.envAddress("MANAGEMENT_DAO_MULTISIG_ADDRESS");
+
+        // Reuse the previous build's plugin implementation. v1.2's bytecode is identical to v1.1's
+        SPP existingImpl = _readLatestImplementation();
 
         vm.startBroadcast(deployerPrivateKey);
-        sppSetup = new SPPSetup(new SPP());
+        sppSetup = new SPPSetup(existingImpl);
         vm.stopBroadcast();
 
-        console.log("- SPP PluginSetup:  ", address(sppSetup));
+        console.log("- SPP PluginSetup:           ", address(sppSetup));
         console.log(
-            "- Version:          ",
+            "- Version:                   ",
             _versionString(PluginSettings.VERSION_RELEASE, PluginSettings.VERSION_BUILD)
         );
-        console.log("\nDAO proposal to publish this version:");
-        console.log("  to:    ", address(sppRepo));
-        console.log("  value: ", uint256(0));
-        console.logBytes(
-            abi.encodeWithSelector(
-                sppRepo.createVersion.selector,
+
+        bytes memory createVersionData = abi.encodeCall(
+            sppRepo.createVersion,
+            (
                 PluginSettings.VERSION_RELEASE,
                 address(sppSetup),
-                PluginSettings.BUILD_METADATA,
-                PluginSettings.RELEASE_METADATA
+                bytes(PluginSettings.BUILD_METADATA),
+                bytes(PluginSettings.RELEASE_METADATA)
             )
         );
+
+        console.log("\nDAO action to publish this version:");
+        console.log("  to:    ", address(sppRepo));
+        console.log("  value: 0");
+        console.log("  data:  ");
+        console.logBytes(createVersionData);
+
+        // Wrap the action in a management DAO multisig proposal. The 7-arg
+        // `createProposal` is the multisig-specific overload; passing
+        // `_approveProposal=true` means the submitter also casts their vote
+        // in the same transaction.
+        Action[] memory actions = new Action[](1);
+        actions[0] = Action({to: address(sppRepo), value: 0, data: createVersionData});
+
+        bytes memory metadata = bytes(PluginSettings.PROPOSAL_METADATA);
+        uint64 endDate = uint64(vm.envOr("PROPOSAL_END_DATE", block.timestamp + 30 days));
+        bytes memory multisigCalldata = abi.encodeCall(
+            IManagementDaoMultisig.createProposal,
+            (
+                metadata,
+                actions,
+                uint256(0), // _allowFailureMap
+                true, // _approveProposal
+                false, // _tryExecution
+                uint64(0), // _startDate (0 = now, evaluated at submission time)
+                endDate
+            )
+        );
+
+        // The Multisig derives proposalId as
+        //   keccak256(abi.encode(chainid, block.number, multisig, keccak256(abi.encode(actions, metadata))))
+        // (see Multisig.createProposal -> Proposal._createProposalId in osx-commons).
+        // Submission block isn't known at script time, so we print the deterministic
+        // salt; the actual proposalId is also surfaced via the `ProposalCreated` event
+        // on the submission tx receipt.
+        bytes32 proposalSalt = keccak256(abi.encode(actions, metadata));
+
+        console.log("\nManagement DAO multisig proposal to publish this version:");
+        console.log("  to:    ", managementDaoMultisig);
+        console.log("  value: 0");
+        console.log("  data:  ");
+        console.logBytes(multisigCalldata);
+        console.log("\n  proposal metadata: ", PluginSettings.PROPOSAL_METADATA);
+        console.log("  defaults: allowFailureMap=0, approveProposal=true, tryExecution=false, startDate=0");
+        console.log("  endDate (unix):    ", uint256(endDate));
+        console.log("\n  proposal id (deterministic salt):");
+        console.logBytes32(proposalSalt);
+        console.log("  full id = keccak256(abi.encode(chainid, block.number @ submission, multisig, salt))");
+        console.log("  or read it from the `ProposalCreated` event on the submission tx receipt.");
+    }
+
+    function _readLatestImplementation() internal view returns (SPP) {
+        uint8 latestRelease = sppRepo.latestRelease();
+        uint16 latestBuild = uint16(sppRepo.buildCount(latestRelease));
+        PluginRepo.Tag memory latestTag = PluginRepo.Tag({release: latestRelease, build: latestBuild});
+        address latestSetup = sppRepo.getVersion(latestTag).pluginSetup;
+        return SPP(IPluginSetup(latestSetup).implementation());
     }
 }

script/new-version-proposal-metadata.json

@@ -0,0 +1,15 @@
+{
+    "title": "Publish StagedProposalProcessor v1.2",
+    "summary": "Publishes build 2 of the StagedProposalProcessor (release 1) on the SPP repo.",
+    "description": "Calls `createVersion` on the SPP plugin repo to publish v1.2 (release 1, build 2).\n\nBuild 2 is recompiled against an amended `osx-commons` `RuledCondition`. The installation parameters are unchanged versus build 1.\n\nExisting v1.1 installations can update in place via the setup's `prepareUpdate` flow, which deploys a fresh `SPPRuleCondition` seeded with the existing rules and migrates the `CREATE_PROPOSAL` and `UPDATE_RULES` permissions from the old helper to the new one.",
+    "resources": [
+        {
+            "name": "changelog",
+            "url": "https://github.com/aragon/staged-proposal-processor-plugin/blob/main/CHANGELOG.md"
+        },
+        {
+            "name": "audit report",
+            "url": "https://github.com/aragon/osx/tree/main/audits"
+        }
+    ]
+}

src/StagedProposalProcessorSetup.sol

@@ -20,7 +20,7 @@ import {
 /// @title StagedProposalProcessorSetup
 /// @author Aragon X - 2024
 /// @notice The setup contract of the `StagedProposalProcessor` plugin.
-/// @dev Release 1, Build 1
+/// @dev Release 1, Build 2
 contract StagedProposalProcessorSetup is PluginUpgradeableSetup {
     using ProxyLib for address;
 
@@ -91,14 +91,71 @@ contract StagedProposalProcessorSetup is PluginUpgradeableSetup {
     }
 
     /// @inheritdoc IPluginSetup
-    /// @dev The default implementation for the initial build 1 that reverts because no earlier build exists.
+    /// @dev v1.1 → v1.2: deploys a fresh `SPPRuleCondition` seeded with the existing rules and migrates
+    /// `CREATE_PROPOSAL_PERMISSION` (on the plugin) and `UPDATE_RULES_PERMISSION` (on the helper) from
+    /// the old condition to the new one. The plugin proxy itself is upgraded to the new implementation
+    /// by the `PluginSetupProcessor` automatically; no reinitializer is required because no new storage
+    /// is introduced in build 2. Existing rules are read from the old helper, so no caller-supplied data
+    /// is required — `_payload.data` is ignored.
     function prepareUpdate(
         address _dao,
         uint16 _fromBuild,
         SetupPayload calldata _payload
-    ) external pure virtual returns (bytes memory, PreparedSetupData memory) {
-        (_dao, _fromBuild, _payload);
-        revert InvalidUpdatePath({fromBuild: 0, thisBuild: 1});
+    ) external virtual override returns (bytes memory initData, PreparedSetupData memory preparedSetupData) {
+        if (_fromBuild != 1) {
+            revert InvalidUpdatePath({fromBuild: _fromBuild, thisBuild: 2});
+        }
+
+        address oldCondition = _payload.currentHelpers[0];
+        RuledCondition.Rule[] memory rules = SPPRuleCondition(oldCondition).getRules();
+
+        bytes memory conditionInitData = abi.encodeCall(
+            SPPRuleCondition.initialize,
+            (_dao, rules)
+        );
+        address newCondition = CLONES_SUPPORTED
+            ? CONDITION_IMPLEMENTATION.deployMinimalProxy(conditionInitData)
+            : CONDITION_IMPLEMENTATION.deployUUPSProxy(conditionInitData);
+
+        preparedSetupData.helpers = new address[](1);
+        preparedSetupData.helpers[0] = newCondition;
+
+        preparedSetupData.permissions = new PermissionLib.MultiTargetPermission[](4);
+
+        // Move CREATE_PROPOSAL_PERMISSION on the plugin from the old condition to the new one.
+        preparedSetupData.permissions[0] = PermissionLib.MultiTargetPermission({
+            operation: PermissionLib.Operation.Revoke,
+            where: _payload.plugin,
+            who: ANY_ADDR,
+            condition: oldCondition,
+            permissionId: Permissions.CREATE_PROPOSAL_PERMISSION_ID
+        });
+        preparedSetupData.permissions[1] = PermissionLib.MultiTargetPermission({
+            operation: PermissionLib.Operation.GrantWithCondition,
+            where: _payload.plugin,
+            who: ANY_ADDR,
+            condition: newCondition,
+            permissionId: Permissions.CREATE_PROPOSAL_PERMISSION_ID
+        });
+
+        // Move UPDATE_RULES_PERMISSION (DAO is the rule manager) from the old condition to the new one.
+        preparedSetupData.permissions[2] = PermissionLib.MultiTargetPermission({
+            operation: PermissionLib.Operation.Revoke,
+            where: oldCondition,
+            who: _dao,
+            condition: PermissionLib.NO_CONDITION,
+            permissionId: Permissions.UPDATE_RULES_PERMISSION_ID
+        });
+        preparedSetupData.permissions[3] = PermissionLib.MultiTargetPermission({
+            operation: PermissionLib.Operation.Grant,
+            where: newCondition,
+            who: _dao,
+            condition: PermissionLib.NO_CONDITION,
+            permissionId: Permissions.UPDATE_RULES_PERMISSION_ID
+        });
+
+        // initData stays empty — applyUpdate triggers the proxy implementation upgrade on its own.
+        initData = "";
     }
 
     /// @inheritdoc IPluginSetup

src/build-metadata.json

@@ -1,6 +1,6 @@
 {
   "ui": {},
-  "change": "Initial build.",
+  "change": "Recompiled against an amended osx-commons RuledCondition. Adds an in-place v1.1 -> v1.2 update path: the setup's prepareUpdate deploys a fresh SPPRuleCondition seeded with the existing rules and migrates CREATE_PROPOSAL / UPDATE_RULES permissions from the old helper to the new one. The setup interface for new installations is unchanged versus build 1.",
   "pluginSetup": {
     "prepareInstallation": {
       "description": "The information required for the installation of build 1.",