diff --git a/packages/contracts-ethers/package.json b/packages/contracts-ethers/package.json index d6e5dcdf4..bebf7e6b5 100644 --- a/packages/contracts-ethers/package.json +++ b/packages/contracts-ethers/package.json @@ -1,6 +1,6 @@ { "name": "@aragon/osx-ethers", - "version": "1.3.1-rc0", + "version": "1.4.0-rc0", "description": "The Aragon OSx contract definitions for ethers.js", "main": "dist/bundle-cjs.js", "module": "dist/bundle-esm.js", diff --git a/packages/contracts/CHANGELOG.md b/packages/contracts/CHANGELOG.md index dcbf0d0b9..4fe77298d 100644 --- a/packages/contracts/CHANGELOG.md +++ b/packages/contracts/CHANGELOG.md @@ -5,6 +5,23 @@ 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.4.0-rc0 + +### Added + +- Added the `FunctionRemoved` error to `DAO`. + +### Changed + +- Renamed the `signatureValidator` variable in `DAO` to `__removed0`. +- Use the DAOs permission manager functionality to validate signatures. + +### Removed + +- Removed the `SignatureValidatorSet` event from `IDAO`. +- Removed unused `ERC1271Mock` contract. +- Removed the `setSignatureValidator` function and `signatureValidator` variable in `DAO`. In places, where the function must remain to not alter the `IDAO` interface ID, it will revert and explanatory notes are put in place.. + ## v1.3.1-rc0 ### Added diff --git a/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md b/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md index 9dff4e774..7ca51869c 100644 --- a/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md +++ b/packages/contracts/docs/osx/01-how-it-works/01-core/01-dao/index.md @@ -50,7 +50,7 @@ Currently, externally owned accounts (EOAs) can sign messages with their associa An exemplary use case is a decentralized exchange with an off-chain order book, where buy/sell orders are signed messages. To accept such a request, both, the external service provider and caller need to follow a standard with which the signed message of the caller can be validated. -By supporting the [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) standard, your DAO can validate signatures via its `isValidSignature` function that forwards the call to a signature validator contract. The signature validator can be set with `setSignatureValidator` function. +By supporting the [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) standard, your DAO can validate signatures via its `isValidSignature` function that forwards the call to a signature validator contract. diff --git a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md index 025328536..2090809fc 100644 --- a/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md +++ b/packages/contracts/docs/osx/01-how-it-works/01-core/02-permissions/index.md @@ -145,7 +145,6 @@ The following functions in the DAO are permissioned: | `_authorizeUpgrade` | `UPGRADE_DAO_PERMISSION_ID` | Required to upgrade the DAO (via the [UUPS](https://eips.ethereum.org/EIPS/eip-1822)). | | `setMetadata` | `SET_METADATA_PERMISSION_ID` | Required to set the DAO’s metadata and [DAOstar.one DAO URI](https://eips.ethereum.org/EIPS/eip-4824). | | `setTrustedForwarder` | `SET_TRUSTED_FORWARDER_PERMISSION_ID` | Required to set the DAO’s trusted forwarder for meta transactions. | -| `setSignatureValidator` | `SET_SIGNATURE_VALIDATOR_PERMISSION_ID` | Required to set the DAO’s signature validator contract (see ERC-1271). | | `registerStandardCallback` | `REGISTER_STANDARD_CALLBACK_PERMISSION_ID` | Required to register a standard callback for an [ERC-165](https://eips.ethereum.org/EIPS/eip-165) interface ID. | Plugins installed on the DAO might introduce other permissions and associated permission identifiers. diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 8608f9e27..0af486dba 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -1,6 +1,6 @@ { "name": "@aragon/osx-artifacts", - "version": "1.3.1-rc0", + "version": "1.4.0-rc0", "description": "The Aragon OSx Solidity contracts", "main": "dist/bundle-cjs.js", "module": "dist/bundle-esm.js", @@ -42,6 +42,7 @@ "devDependencies": { "@aragon/osx-ethers-v1.2.0": "npm:@aragon/osx-ethers@1.2.0", "@aragon/osx-v1.0.1": "npm:@aragon/osx@1.0.1", + "@aragon/osx-v1.3.0-rc0.2": "npm:@aragon/osx@1.3.0-rc0.2", "@defi-wonderland/smock": "^2.3.4", "@ensdomains/ens-contracts": "0.0.11", "@nomicfoundation/hardhat-chai-matchers": "^1.0.5", diff --git a/packages/contracts/scripts/osx-versions-aliases.ts b/packages/contracts/scripts/osx-versions-aliases.ts index 9c3601aec..03acd7956 100644 --- a/packages/contracts/scripts/osx-versions-aliases.ts +++ b/packages/contracts/scripts/osx-versions-aliases.ts @@ -5,4 +5,7 @@ * * To add a new version alias, append it to this array. */ -export const OSX_VERSION_ALIASES = ['@aragon/osx-v1.0.1/']; +export const OSX_VERSION_ALIASES = [ + '@aragon/osx-v1.0.1/', + '@aragon/osx-v1.3.0-rc0.2/', +]; diff --git a/packages/contracts/src/core/dao/DAO.sol b/packages/contracts/src/core/dao/DAO.sol index 7a10df655..2f119af6c 100644 --- a/packages/contracts/src/core/dao/DAO.sol +++ b/packages/contracts/src/core/dao/DAO.sol @@ -52,14 +52,14 @@ contract DAO is bytes32 public constant SET_TRUSTED_FORWARDER_PERMISSION_ID = keccak256("SET_TRUSTED_FORWARDER_PERMISSION"); - /// @notice The ID of the permission required to call the `setSignatureValidator` function. - bytes32 public constant SET_SIGNATURE_VALIDATOR_PERMISSION_ID = - keccak256("SET_SIGNATURE_VALIDATOR_PERMISSION"); - /// @notice The ID of the permission required to call the `registerStandardCallback` function. bytes32 public constant REGISTER_STANDARD_CALLBACK_PERMISSION_ID = keccak256("REGISTER_STANDARD_CALLBACK_PERMISSION"); + /// @notice The ID of the permission required to validate [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signatures. + bytes32 public constant VALIDATE_SIGNATURE_PERMISSION_ID = + keccak256("VALIDATE_SIGNATURE_PERMISSION"); + /// @notice The internal constant storing the maximal action array length. uint256 internal constant MAX_ACTIONS = 256; @@ -69,9 +69,10 @@ contract DAO is /// @notice The second out of two values to which the `_reentrancyStatus` state variable (used by the `nonReentrant` modifier) can be set inidicating that a function was entered. uint256 private constant _ENTERED = 2; - /// @notice The [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract. - /// @dev Added in v1.0.0. - IERC1271 public signatureValidator; + /// @notice Removed variable that is left here to maintain the storage layout. + /// @dev Introduced in v1.0.0. Removed in v1.4.0. + /// @custom:oz-renamed-from signatureValidator + address private __removed0; /// @notice The address of the trusted forwarder verifying meta transactions. /// @dev Added in v1.0.0. @@ -109,6 +110,9 @@ contract DAO is /// @notice Thrown if an upgrade is not supported from a specific protocol version . error ProtocolVersionUpgradeNotSupported(uint8[3] protocolVersion); + /// @notice Thrown when a function is removed but left to not corrupt the interface ID. + error FunctionRemoved(); + /// @notice Emitted when a new DAO URI is set. /// @param daoURI The new URI. event NewURI(string daoURI); @@ -193,7 +197,6 @@ contract DAO is _permissionId == UPGRADE_DAO_PERMISSION_ID || _permissionId == SET_METADATA_PERMISSION_ID || _permissionId == SET_TRUSTED_FORWARDER_PERMISSION_ID || - _permissionId == SET_SIGNATURE_VALIDATOR_PERMISSION_ID || _permissionId == REGISTER_STANDARD_CALLBACK_PERMISSION_ID; } @@ -320,25 +323,30 @@ contract DAO is } /// @inheritdoc IDAO - function setSignatureValidator( - address _signatureValidator - ) external override auth(SET_SIGNATURE_VALIDATOR_PERMISSION_ID) { - signatureValidator = IERC1271(_signatureValidator); - - emit SignatureValidatorSet({signatureValidator: _signatureValidator}); + function setSignatureValidator(address) external pure override { + revert FunctionRemoved(); } /// @inheritdoc IDAO + /// @dev Relays the validation logic determining who is allowed to sign on behalf of the DAO to its permission manager. + /// Caller specific bypassing can be set direct granting (i.e., `grant({_where: dao, _who: specificErc1271Caller, _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID})`). + /// Caller specific signature validation logic can be set by granting with a `PermissionCondition` (i.e., `grantWithCondition({_where: dao, _who: specificErc1271Caller, _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID, _condition: yourConditionImplementation})`) + /// Generic signature validation logic can be set for all calling contracts by granting with a `PermissionCondition` to `PermissionManager.ANY_ADDR()` (i.e., `grantWithCondition({_where: dao, _who: PermissionManager.ANY_ADDR(), _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID, _condition: yourConditionImplementation})`). function isValidSignature( bytes32 _hash, bytes memory _signature ) external view override(IDAO, IERC1271) returns (bytes4) { - if (address(signatureValidator) == address(0)) { - // Return the invalid magic number - return bytes4(0); + if ( + isGranted({ + _where: address(this), + _who: msg.sender, + _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID, + _data: abi.encode(_hash, _signature) + }) + ) { + return 0x1626ba7e; // `type(IERC1271).interfaceId` = bytes4(keccak256("isValidSignature(bytes32,bytes)")` } - // Forward the call to the set signature validator contract - return signatureValidator.isValidSignature(_hash, _signature); + return 0xffffffff; // `bytes4(uint32(type(uint32).max-1))` } /// @notice Emits the `NativeTokenDeposited` event to track native token deposits that weren't made via the deposit method. diff --git a/packages/contracts/src/core/dao/IDAO.sol b/packages/contracts/src/core/dao/IDAO.sol index e50fad31e..5b6c0d233 100644 --- a/packages/contracts/src/core/dao/IDAO.sol +++ b/packages/contracts/src/core/dao/IDAO.sol @@ -112,15 +112,7 @@ interface IDAO { /// @param forwarder the new forwarder address. event TrustedForwarderSet(address forwarder); - /// @notice Setter for the [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract. - /// @param _signatureValidator The address of the signature validator. - function setSignatureValidator(address _signatureValidator) external; - - /// @notice Emitted when the signature validator address is updated. - /// @param signatureValidator The address of the signature validator. - event SignatureValidatorSet(address signatureValidator); - - /// @notice Checks whether a signature is valid for the provided hash by forwarding the call to the set [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271) signature validator contract. + /// @notice Checks whether a signature is valid for a provided hash according to [ERC-1271](https://eips.ethereum.org/EIPS/eip-1271). /// @param _hash The hash of the data to be signed. /// @param _signature The signature byte array associated with `_hash`. /// @return Returns the `bytes4` magic value `0x1626ba7e` if the signature is valid. @@ -135,4 +127,8 @@ interface IDAO { bytes4 _callbackSelector, bytes4 _magicNumber ) external; + + /// @notice Removed function being left here to not corrupt the IDAO interface ID. Any call will revert. + /// @dev Introduced in v1.0.0. Removed in v1.4.0. + function setSignatureValidator(address) external; } diff --git a/packages/contracts/src/core/permission/PermissionManager.sol b/packages/contracts/src/core/permission/PermissionManager.sol index 1a05a3124..57236fd61 100644 --- a/packages/contracts/src/core/permission/PermissionManager.sol +++ b/packages/contracts/src/core/permission/PermissionManager.sol @@ -60,7 +60,6 @@ abstract contract PermissionManager is Initializable { error ConditionInterfacNotSupported(IPermissionCondition condition); /// @notice Thrown for `ROOT_PERMISSION_ID` or `EXECUTE_PERMISSION_ID` permission grants where `who` or `where` is `ANY_ADDR`. - error PermissionsForAnyAddressDisallowed(); /// @notice Thrown for permission grants where `who` and `where` are both `ANY_ADDR`. diff --git a/packages/contracts/src/framework/dao/DAOFactory.sol b/packages/contracts/src/framework/dao/DAOFactory.sol index d9d6a5eb2..a82a8635b 100644 --- a/packages/contracts/src/framework/dao/DAOFactory.sol +++ b/packages/contracts/src/framework/dao/DAOFactory.sol @@ -169,7 +169,7 @@ contract DAOFactory is ERC165, ProtocolVersion { function _setDAOPermissions(DAO _dao) internal { // set permissionIds on the dao itself. PermissionLib.SingleTargetPermission[] - memory items = new PermissionLib.SingleTargetPermission[](6); + memory items = new PermissionLib.SingleTargetPermission[](5); // Grant DAO all the permissions required items[0] = PermissionLib.SingleTargetPermission( @@ -183,21 +183,16 @@ contract DAOFactory is ERC165, ProtocolVersion { _dao.UPGRADE_DAO_PERMISSION_ID() ); items[2] = PermissionLib.SingleTargetPermission( - PermissionLib.Operation.Grant, - address(_dao), - _dao.SET_SIGNATURE_VALIDATOR_PERMISSION_ID() - ); - items[3] = PermissionLib.SingleTargetPermission( PermissionLib.Operation.Grant, address(_dao), _dao.SET_TRUSTED_FORWARDER_PERMISSION_ID() ); - items[4] = PermissionLib.SingleTargetPermission( + items[3] = PermissionLib.SingleTargetPermission( PermissionLib.Operation.Grant, address(_dao), _dao.SET_METADATA_PERMISSION_ID() ); - items[5] = PermissionLib.SingleTargetPermission( + items[4] = PermissionLib.SingleTargetPermission( PermissionLib.Operation.Grant, address(_dao), _dao.REGISTER_STANDARD_CALLBACK_PERMISSION_ID() diff --git a/packages/contracts/src/package.json b/packages/contracts/src/package.json index 1093bcce6..1ef73d391 100644 --- a/packages/contracts/src/package.json +++ b/packages/contracts/src/package.json @@ -1,6 +1,6 @@ { "name": "@aragon/osx", - "version": "1.3.1-rc0", + "version": "1.4.0-rc0", "description": "The Aragon OSx Solidity contracts", "publishConfig": { "access": "public" diff --git a/packages/contracts/src/test/Migration.sol b/packages/contracts/src/test/Migration.sol index d5fdae2b2..15bf7188f 100644 --- a/packages/contracts/src/test/Migration.sol +++ b/packages/contracts/src/test/Migration.sol @@ -20,14 +20,30 @@ pragma solidity 0.8.17; */ import {DAO as DAO_v1_0_0} from "@aragon/osx-v1.0.1/core/dao/DAO.sol"; +import {DAO as DAO_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol"; import {DAORegistry as DAORegistry_v1_0_0} from "@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol"; +import {DAORegistry as DAORegistry_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/dao/DAORegistry.sol"; + import {PluginRepo as PluginRepo_v1_0_0} from "@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol"; +import {PluginRepo as PluginRepo_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepo.sol"; + import {PluginRepoRegistry as PluginRepoRegistry_v1_0_0} from "@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol"; +import {PluginRepoRegistry as PluginRepoRegistry_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepoRegistry.sol"; + import {ENSSubdomainRegistrar as ENSSubdomainRegistrar_v1_0_0} from "@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol"; +import {ENSSubdomainRegistrar as ENSSubdomainRegistrar_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/framework/utils/ens/ENSSubdomainRegistrar.sol"; import {TokenVoting as TokenVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol"; +import {TokenVoting as TokenVoting_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/token/TokenVoting.sol"; + import {AddresslistVoting as AddresslistVoting_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol"; +import {AddresslistVoting as AddresslistVoting_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol"; + import {Multisig as Multisig_v1_0_0} from "@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol"; +import {Multisig as Multisig_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/governance/multisig/Multisig.sol"; import {MerkleMinter as MerkleMinter_v1_0_0} from "@aragon/osx-v1.0.1/plugins/token/MerkleMinter.sol"; +import {MerkleMinter as MerkleMinter_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleMinter.sol"; + import {MerkleDistributor as MerkleDistributor_v1_0_0} from "@aragon/osx-v1.0.1/plugins/token/MerkleDistributor.sol"; +import {MerkleDistributor as MerkleDistributor_v1_3_0} from "@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleDistributor.sol"; diff --git a/packages/contracts/src/test/dao/ERC1271Mock.sol b/packages/contracts/src/test/dao/ERC1271Mock.sol deleted file mode 100644 index cc39cfd91..000000000 --- a/packages/contracts/src/test/dao/ERC1271Mock.sol +++ /dev/null @@ -1,9 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later - -pragma solidity 0.8.17; - -contract ERC1271Mock { - function isValidSignature(bytes32, bytes memory) public pure returns (bytes4) { - return 0x41424344; - } -} diff --git a/packages/contracts/src/test/permission/PermissionConditionMock.sol b/packages/contracts/src/test/permission/PermissionConditionMock.sol index 21b70db87..c9c27a275 100644 --- a/packages/contracts/src/test/permission/PermissionConditionMock.sol +++ b/packages/contracts/src/test/permission/PermissionConditionMock.sol @@ -5,18 +5,23 @@ pragma solidity 0.8.17; import "../../core/permission/PermissionCondition.sol"; contract PermissionConditionMock is PermissionCondition { - bool internal _hasPermissionsResult = true; + bool public answer; - function isGranted( - address /* _where */, - address /* _who */, - bytes32 /* _permissionId */, - bytes memory /* _data */ - ) external view returns (bool) { - return _hasPermissionsResult; + constructor() { + answer = true; + } + + function setAnswer(bool _answer) external { + answer = _answer; } - function setWillPerform(bool _result) external { - _hasPermissionsResult = _result; + function isGranted( + address _where, + address _who, + bytes32 _permissionId, + bytes memory _data + ) external view returns (bool) { + (_where, _who, _permissionId, _data); + return answer; } } diff --git a/packages/contracts/src/utils/protocol/ProtocolVersion.sol b/packages/contracts/src/utils/protocol/ProtocolVersion.sol index f1fed8c48..8454d5043 100644 --- a/packages/contracts/src/utils/protocol/ProtocolVersion.sol +++ b/packages/contracts/src/utils/protocol/ProtocolVersion.sol @@ -13,6 +13,6 @@ abstract contract ProtocolVersion is IProtocolVersion { /// @inheritdoc IProtocolVersion function protocolVersion() public pure returns (uint8[3] memory) { - return [1, 3, 0]; + return [1, 4, 0]; } } diff --git a/packages/contracts/test/core/dao/dao.ts b/packages/contracts/test/core/dao/dao.ts index b92f84f66..63b128cc6 100644 --- a/packages/contracts/test/core/dao/dao.ts +++ b/packages/contracts/test/core/dao/dao.ts @@ -11,7 +11,6 @@ import { TestERC721__factory, TestERC1155, TestERC1155__factory, - ERC1271Mock__factory, GasConsumer__factory, DAO__factory, IDAO__factory, @@ -21,8 +20,11 @@ import { IERC1271__factory, IEIP4824__factory, IProtocolVersion__factory, + PermissionConditionMock__factory, + PermissionConditionMock, } from '../../../typechain'; import {DAO__factory as DAO_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol'; +import {DAO__factory as DAO_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol'; import { getProtocolVersion, @@ -52,6 +54,8 @@ import { CURRENT_PROTOCOL_VERSION, IMPLICIT_INITIAL_PROTOCOL_VERSION, } from '../../test-utils/protocol-version'; +import {ANY_ADDR} from '../permission/permission-manager'; +import {defaultAbiCoder} from 'ethers/lib/utils'; chai.use(smock.matchers); @@ -86,8 +90,8 @@ export const PERMISSION_IDS = { UPGRADE_DAO_PERMISSION_ID: UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID, SET_METADATA_PERMISSION_ID: ethers.utils.id('SET_METADATA_PERMISSION'), EXECUTE_PERMISSION_ID: ethers.utils.id('EXECUTE_PERMISSION'), - SET_SIGNATURE_VALIDATOR_PERMISSION_ID: ethers.utils.id( - 'SET_SIGNATURE_VALIDATOR_PERMISSION' + VALIDATE_SIGNATURE_PERMISSION_ID: ethers.utils.id( + 'VALIDATE_SIGNATURE_PERMISSION' ), SET_TRUSTED_FORWARDER_PERMISSION_ID: ethers.utils.id( 'SET_TRUSTED_FORWARDER_PERMISSION' @@ -98,6 +102,9 @@ export const PERMISSION_IDS = { ), }; +export const VALID_ERC1271_SIGNATURE = '0x1626ba7e'; +export const INVALID_ERC1271_SIGNATURE = '0xffffffff'; + describe('DAO', function () { let signers: SignerWithAddress[]; let ownerAddress: string; @@ -136,11 +143,6 @@ describe('DAO', function () { ownerAddress, PERMISSION_IDS.UPGRADE_DAO_PERMISSION_ID ), - dao.grant( - dao.address, - ownerAddress, - PERMISSION_IDS.SET_SIGNATURE_VALIDATOR_PERMISSION_ID - ), dao.grant( dao.address, ownerAddress, @@ -383,6 +385,33 @@ describe('DAO', function () { ); expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new DAO_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_DAO_PERMISSION_ID + ); + expect(toImplementation).to.not.equal(fromImplementation); + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 3, 0]); + expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('ERC-165', async () => { @@ -1190,98 +1219,234 @@ describe('DAO', function () { }); describe('ERC1271', async () => { - it('should register the interfaceId', async () => { + let signer: SignerWithAddress; + let caller: SignerWithAddress; + let otherCaller: SignerWithAddress; + + let message: string; + let hash: string; + let signature: string; + + let mockConditionFactory: PermissionConditionMock__factory; + + beforeEach(async () => { + caller = signers[0]; + signer = signers[1]; + otherCaller = signers[2]; + + mockConditionFactory = new PermissionConditionMock__factory(caller); + + message = 'The message!'; + hash = ethers.utils.hashMessage(message); + signature = await signer.signMessage(message); + }); + + it('treats signatures as invalid by default if no permission is set', async () => { expect( - await dao.supportsInterface( - getInterfaceID(IERC1271__factory.createInterface()) - ) - ).to.be.eq(true); + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(INVALID_ERC1271_SIGNATURE); }); - it('should return 0 if no validator is set', async () => { + it('allows caller-specific signature validation bypassing', async () => { + // Grant the permission to validate signatures to the caller without a condition + await dao.grant( + dao.address, + caller.address, + PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID + ); + + // The caller can validate signatures now. + expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not + .be.reverted; + + // Because the caller is allowed unconditionally, the signature is always valid. expect( - await dao.isValidSignature(ethers.utils.keccak256('0x00'), '0x00') - ).to.be.eq('0x00000000'); + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + + // Because the other caller is not allowed, the signature is always invalid. + expect( + await dao.connect(otherCaller).isValidSignature(hash, signature) + ).to.equal(INVALID_ERC1271_SIGNATURE); }); - it('should allow only `SET_SIGNATURE_VALIDATOR_PERMISSION_ID` to set validator', async () => { - await expect( - dao - .connect(signers[2]) - .setSignatureValidator(ethers.Wallet.createRandom().address) - ) + it('allows caller-specific signature validation conditions', async () => { + // Try to call with caller but caller has no permission + expect(await dao.connect(caller).isValidSignature(hash, signature)) .to.be.revertedWithCustomError(dao, 'Unauthorized') - .withArgs( - dao.address, - signers[2].address, - PERMISSION_IDS.SET_SIGNATURE_VALIDATOR_PERMISSION_ID - ); - }); + .withArgs(); - it('should set validator and emits event', async () => { - const validatorAddress = ethers.Wallet.createRandom().address; - const tx = await dao.setSignatureValidator(validatorAddress); + // Deploy a mock condition + const mockCondition = await mockConditionFactory.deploy(); - expect(await dao.signatureValidator()).to.be.eq(validatorAddress); + // Grant the permission to validate signatures to the caller + await dao.grantWithCondition( + dao.address, + caller.address, + PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID, + mockCondition.address + ); - await expect(tx) - .to.emit(dao, EVENTS.SignatureValidatorSet) - .withArgs(validatorAddress); - }); + // The caller can validate signatures now. + expect(await dao.connect(caller).isValidSignature(hash, signature)).to.not + .be.reverted; + + // Check that the mock condition will answer true. + expect(await mockCondition.answer()).to.be.true; - it('should call the signature validator', async () => { - const ERC1271MockFactory = await smock.mock('ERC1271Mock'); - const erc1271Mock = await ERC1271MockFactory.deploy(); + // Check that the signature is valid in this case. + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + + // Set the mock condition to answer false. + await mockCondition.setAnswer(false); - await dao.setSignatureValidator(erc1271Mock.address); - await dao.isValidSignature(ethers.utils.keccak256('0x00'), '0x00'); - expect(erc1271Mock.isValidSignature).has.been.callCount(1); + // Check that the mock condition will answer false. + expect(await mockCondition.answer()).to.be.false; + + // Check that the signature is invalid in this case. + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(INVALID_ERC1271_SIGNATURE); }); - it('should return the validators response', async () => { - const ERC1271MockFactory = new ERC1271Mock__factory(signers[0]); - const erc1271Mock = await ERC1271MockFactory.deploy(); + it('allows generic signature validation by granting to ANY_ADDR', async () => { + // Deploy a mock condition + const mockCondition = await mockConditionFactory.deploy(); + + // Grant the permission to validate signatures to the ANY caller conditionally (granting it unconditionally is not possible in combination with `_who: ANY_ADDR`) + await dao.grantWithCondition( + dao.address, + ANY_ADDR, + PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID, + mockCondition.address + ); + + // Check that the mock condition will answer true. + expect(await mockCondition.answer()).to.be.true; - await dao.setSignatureValidator(erc1271Mock.address); + // Any caller can validate signatures using this condition now. expect( - await dao.isValidSignature(ethers.utils.keccak256('0x00'), '0x00') - ).to.be.eq('0x41424344'); + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + expect( + await dao.connect(otherCaller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + + // Set the mock condition to answer false. + await mockCondition.setAnswer(false); + + // Check that the mock condition will answer false. + expect(await mockCondition.answer()).to.be.false; + + // Check that the signature is invalid in this case for every caller. + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(INVALID_ERC1271_SIGNATURE); + expect( + await dao.connect(otherCaller).isValidSignature(hash, signature) + ).to.equal(INVALID_ERC1271_SIGNATURE); }); - describe('ERC4824 - daoURI', async () => { - it('should set a new URI', async () => { - const newURI = 'https://new.example.com'; - expect(await dao.daoURI()).not.to.be.eq(newURI); - await dao.setDaoURI(newURI); - expect(await dao.daoURI()).to.be.eq(newURI); - }); + context( + 'A caller-specfic and a generic condition are both set', + async () => { + let specificMockCondition: PermissionConditionMock; + let genericMockCondition: PermissionConditionMock; - it('should emit DaoURIUpdated', async () => { - const newURI = 'https://new.example.com'; - await expect(dao.setDaoURI(newURI)) - .to.emit(dao, DAO_EVENTS.NEW_URI) - .withArgs(newURI); - }); + beforeEach(async () => { + // Setup the specfic condition for a specific caller + specificMockCondition = await mockConditionFactory.deploy(); + await dao.grantWithCondition( + dao.address, + caller.address, + PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID, + specificMockCondition.address + ); + + // Setup the generic condition for ANY caller + genericMockCondition = await mockConditionFactory.deploy(); + await dao.grantWithCondition( + dao.address, + ANY_ADDR, + PERMISSION_IDS.VALIDATE_SIGNATURE_PERMISSION_ID, + genericMockCondition.address + ); + }); + + it('returns valid if both conditions are met', async () => { + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + }); + + it('returns valid if only the specific condition is met', async () => { + await genericMockCondition.setAnswer(false); + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + }); + + it('returns valid if only the generic condition is met', async () => { + await specificMockCondition.setAnswer(false); + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(VALID_ERC1271_SIGNATURE); + }); - it('should revert if the sender lacks the permission to update the URI', async () => { - await dao.revoke( + it('returns invalid if both conditions are not met', async () => { + await specificMockCondition.setAnswer(false); + await genericMockCondition.setAnswer(false); + expect( + await dao.connect(caller).isValidSignature(hash, signature) + ).to.equal(INVALID_ERC1271_SIGNATURE); + }); + } + ); + + it('should revert if `setSignatureValidator` is called', async () => { + await expect( + dao + .connect(caller) + .setSignatureValidator(ethers.Wallet.createRandom().address) + ).to.be.revertedWithCustomError(dao, 'FunctionRemoved'); + }); + }); + + describe('ERC4824 - daoURI', async () => { + it('should set a new URI', async () => { + const newURI = 'https://new.example.com'; + expect(await dao.daoURI()).not.to.be.eq(newURI); + await dao.setDaoURI(newURI); + expect(await dao.daoURI()).to.be.eq(newURI); + }); + + it('should emit DaoURIUpdated', async () => { + const newURI = 'https://new.example.com'; + await expect(dao.setDaoURI(newURI)) + .to.emit(dao, DAO_EVENTS.NEW_URI) + .withArgs(newURI); + }); + + it('should revert if the sender lacks the permission to update the URI', async () => { + await dao.revoke( + dao.address, + ownerAddress, + PERMISSION_IDS.SET_METADATA_PERMISSION_ID + ); + + await expect(dao.setDaoURI('https://new.example.com')) + .to.be.revertedWithCustomError(dao, 'Unauthorized') + .withArgs( dao.address, ownerAddress, PERMISSION_IDS.SET_METADATA_PERMISSION_ID ); + }); - await expect(dao.setDaoURI('https://new.example.com')) - .to.be.revertedWithCustomError(dao, 'Unauthorized') - .withArgs( - dao.address, - ownerAddress, - PERMISSION_IDS.SET_METADATA_PERMISSION_ID - ); - }); - - it('should return the DAO URI', async () => { - expect(await dao.daoURI()).to.be.eq(daoExampleURI); - }); + it('should return the DAO URI', async () => { + expect(await dao.daoURI()).to.be.eq(daoExampleURI); }); }); }); diff --git a/packages/contracts/test/core/permission/permission-manager.ts b/packages/contracts/test/core/permission/permission-manager.ts index 8d50a4374..2034a8a9e 100644 --- a/packages/contracts/test/core/permission/permission-manager.ts +++ b/packages/contracts/test/core/permission/permission-manager.ts @@ -27,9 +27,9 @@ const UNSET_FLAG = ethers.utils.getAddress( const ALLOW_FLAG = ethers.utils.getAddress( '0x0000000000000000000000000000000000000002' ); +export const ANY_ADDR = '0xffffffffffffffffffffffffffffffffffffffff'; const addressZero = ethers.constants.AddressZero; -const ANY_ADDR = '0xffffffffffffffffffffffffffffffffffffffff'; let conditionMock: PermissionConditionMock; @@ -816,7 +816,7 @@ describe('Core: PermissionManager', function () { ) ).to.be.equal(true); - await permissionCondition.setWillPerform(false); + await permissionCondition.setAnswer(false); expect( await pm.callStatic.isGranted( pm.address, diff --git a/packages/contracts/test/framework/dao/dao-factory.ts b/packages/contracts/test/framework/dao/dao-factory.ts index cb5a9cef0..e11af5d77 100644 --- a/packages/contracts/test/framework/dao/dao-factory.ts +++ b/packages/contracts/test/framework/dao/dao-factory.ts @@ -421,14 +421,6 @@ describe('DAOFactory: ', function () { ALLOW_FLAG ) .to.emit(daoContract, EVENTS.Granted) - .withArgs( - SET_SIGNATURE_VALIDATOR_PERMISSION_ID, - daoFactory.address, - dao, - dao, - ALLOW_FLAG - ) - .to.emit(daoContract, EVENTS.Granted) .withArgs( SET_TRUSTED_FORWARDER_PERMISSION_ID, daoFactory.address, diff --git a/packages/contracts/test/framework/dao/dao-registry.ts b/packages/contracts/test/framework/dao/dao-registry.ts index c7cf860d5..53d59767a 100644 --- a/packages/contracts/test/framework/dao/dao-registry.ts +++ b/packages/contracts/test/framework/dao/dao-registry.ts @@ -10,6 +10,7 @@ import { ENSSubdomainRegistrar, } from '../../../typechain'; import {DAORegistry__factory as DAORegistry_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/dao/DAORegistry.sol'; +import {DAORegistry__factory as DAORegistry_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/dao/DAORegistry.sol'; import {deployNewDAO} from '../../test-utils/dao'; import {deployENSSubdomainRegistrar} from '../../test-utils/ens'; @@ -295,7 +296,6 @@ describe('DAORegistry', function () { UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID, managingDao ); - expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version const fromProtocolVersion = await getProtocolVersion( @@ -311,5 +311,35 @@ describe('DAORegistry', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new DAORegistry_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID, + managingDao + ); + expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.3.0 to the current version + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); }); diff --git a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts index 0dae6bcff..cd4ed9a3c 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo-registry.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo-registry.ts @@ -11,6 +11,7 @@ import { PluginRepoRegistry__factory, } from '../../../typechain'; import {PluginRepoRegistry__factory as PluginRepoRegistry_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepoRegistry.sol'; +import {PluginRepoRegistry__factory as PluginRepoRegistry_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepoRegistry.sol'; import {deployNewDAO} from '../../test-utils/dao'; import {deployNewPluginRepo} from '../../test-utils/repo'; @@ -306,7 +307,6 @@ describe('PluginRepoRegistry', function () { UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID, managingDAO ); - expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.0.0 to the current version const fromProtocolVersion = await getProtocolVersion( @@ -322,5 +322,37 @@ describe('PluginRepoRegistry', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new PluginRepoRegistry_V1_3_0__factory( + signers[0] + ); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRY_PERMISSION_ID, + managingDAO + ); + expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.3.0 to the current version + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); }); diff --git a/packages/contracts/test/framework/plugin/plugin-repo.ts b/packages/contracts/test/framework/plugin/plugin-repo.ts index 5853ef965..601cce3f4 100644 --- a/packages/contracts/test/framework/plugin/plugin-repo.ts +++ b/packages/contracts/test/framework/plugin/plugin-repo.ts @@ -17,6 +17,7 @@ import { IProtocolVersion__factory, } from '../../../typechain'; import {PluginRepo__factory as PluginRepo_V1_0_0__factory} from '../../../typechain/@aragon/osx-v1.0.1/framework/plugin/repo/PluginRepo.sol'; +import {PluginRepo__factory as PluginRepo_V1_3_0__factory} from '../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/plugin/repo/PluginRepo.sol'; import { getProtocolVersion, @@ -133,6 +134,33 @@ describe('PluginRepo', function () { ); expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new PluginRepo_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REPO_PERMISSION_ID + ); + expect(toImplementation).to.not.equal(fromImplementation); + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.not.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal([1, 3, 0]); + expect(toProtocolVersion).to.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('ERC-165', async () => { diff --git a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts index 0997d6985..3b4ea2003 100644 --- a/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts +++ b/packages/contracts/test/framework/utils/ens/ens-subdomain-registry.ts @@ -13,6 +13,7 @@ import { ENSSubdomainRegistrar__factory, } from '../../../../typechain'; import {ENSSubdomainRegistrar__factory as ENSSubdomainRegistrar_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/framework/utils/ens/ENSSubdomainRegistrar.sol'; +import {ENSSubdomainRegistrar__factory as ENSSubdomainRegistrar_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/framework/utils/ens/ENSSubdomainRegistrar.sol'; import {deployWithProxy} from '../../../test-utils/proxy'; import {deployNewDAO} from '../../../test-utils/dao'; @@ -351,6 +352,38 @@ describe('ENSSubdomainRegistrar', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new ENSSubdomainRegistrar_V1_3_0__factory( + signers[0] + ); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_REGISTRAR_PERMISSION_ID, + managingDao + ); + expect(toImplementation).to.equal(fromImplementation); // The implementation was not changed from 1.3.0 to the current version + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); function expectedReverts() { diff --git a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts index 3be84438f..b8984ea45 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/addresslist/addresslist-voting.ts @@ -16,6 +16,8 @@ import { IProposal__factory, } from '../../../../../typechain'; import {AddresslistVoting__factory as AddresslistVoting_V1_0_0__factory} from '../../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol'; +import {AddresslistVoting__factory as AddresslistVoting_V1_3_0__factory} from '../../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/addresslist/AddresslistVoting.sol'; + import { ProposalCreatedEvent, ProposalExecutedEvent, @@ -191,6 +193,36 @@ describe('AddresslistVoting', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new AddresslistVoting_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + expect(toImplementation).to.equal(fromImplementation); + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('plugin interface: ', async () => { diff --git a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts index 71d854eea..54e2e852a 100644 --- a/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts +++ b/packages/contracts/test/plugins/governance/majority-voting/token/token-voting.ts @@ -17,6 +17,8 @@ import { TokenVoting__factory, } from '../../../../../typechain'; import {TokenVoting__factory as TokenVoting_V1_0_0__factory} from '../../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/majority-voting/token/TokenVoting.sol'; +import {TokenVoting__factory as TokenVoting_V1_3_0__factory} from '../../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/governance/majority-voting/token/TokenVoting.sol'; + import { ProposalCreatedEvent, ProposalExecutedEvent, @@ -258,6 +260,36 @@ describe('TokenVoting', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new TokenVoting_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + expect(toImplementation).to.equal(fromImplementation); + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('plugin interface: ', async () => { diff --git a/packages/contracts/test/plugins/governance/multisig/multisig.ts b/packages/contracts/test/plugins/governance/multisig/multisig.ts index fb63f0c1b..fb9783bf8 100644 --- a/packages/contracts/test/plugins/governance/multisig/multisig.ts +++ b/packages/contracts/test/plugins/governance/multisig/multisig.ts @@ -16,6 +16,8 @@ import { Multisig__factory, } from '../../../../typechain'; import {Multisig__factory as Multisig_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/governance/multisig/Multisig.sol'; +import {Multisig__factory as Multisig_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/governance/multisig/Multisig.sol'; + import { ApprovedEvent, ProposalExecutedEvent, @@ -258,6 +260,36 @@ describe('Multisig', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new Multisig_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + expect(toImplementation).to.equal(fromImplementation); + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('plugin interface: ', async () => { diff --git a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts index f7359c39e..5b34c1821 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-distributor.ts @@ -16,6 +16,7 @@ import { MerkleDistributor__factory, } from '../../../../typechain'; import {MerkleDistributor__factory as MerkleDistributor_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/token/MerkleDistributor.sol'; +import {MerkleDistributor__factory as MerkleDistributor_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleDistributor.sol'; import {deployWithProxy} from '../../../test-utils/proxy'; import BalanceTree from './src/balance-tree'; @@ -141,6 +142,36 @@ describe('MerkleDistributor', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new MerkleDistributor_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + dao + ); + expect(toImplementation).to.equal(fromImplementation); // The build did not change + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('general', () => { diff --git a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts index cb462db30..026c3cd37 100644 --- a/packages/contracts/test/plugins/token/distribution/merkle-minter.ts +++ b/packages/contracts/test/plugins/token/distribution/merkle-minter.ts @@ -18,6 +18,7 @@ import { GovernanceERC20__factory, } from '../../../../typechain'; import {MerkleMinter__factory as MerkleMinter_V1_0_0__factory} from '../../../../typechain/@aragon/osx-v1.0.1/plugins/token/MerkleMinter.sol'; +import {MerkleMinter__factory as MerkleMinter_V1_3_0__factory} from '../../../../typechain/@aragon/osx-v1.3.0-rc0.2/plugins/token/MerkleMinter.sol'; import BalanceTree from './src/balance-tree'; import {deployNewDAO} from '../../../test-utils/dao'; @@ -151,6 +152,36 @@ describe('MerkleMinter', function () { ); expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); }); + + it('from v1.3.0', async () => { + legacyContractFactory = new MerkleMinter_V1_3_0__factory(signers[0]); + + const {fromImplementation, toImplementation} = + await deployAndUpgradeFromToCheck( + signers[0], + signers[1], + initArgs, + 'initialize', + legacyContractFactory, + currentContractFactory, + UPGRADE_PERMISSIONS.UPGRADE_PLUGIN_PERMISSION_ID, + managingDao + ); + expect(toImplementation).to.equal(fromImplementation); // The build did not change + + const fromProtocolVersion = await getProtocolVersion( + legacyContractFactory.attach(fromImplementation) + ); + const toProtocolVersion = await getProtocolVersion( + currentContractFactory.attach(toImplementation) + ); + + expect(fromProtocolVersion).to.deep.equal(toProtocolVersion); // The contracts inherited from OSx did not change from 1.3.0 to the current version + expect(fromProtocolVersion).to.deep.equal( + IMPLICIT_INITIAL_PROTOCOL_VERSION + ); + expect(toProtocolVersion).to.not.deep.equal(CURRENT_PROTOCOL_VERSION); + }); }); describe('plugin interface: ', async () => { diff --git a/packages/contracts/test/test-utils/protocol-version.ts b/packages/contracts/test/test-utils/protocol-version.ts index fe6b20723..28eca6de1 100644 --- a/packages/contracts/test/test-utils/protocol-version.ts +++ b/packages/contracts/test/test-utils/protocol-version.ts @@ -1,5 +1,5 @@ // The current protocol version number as specified by the `getProtocolVersion()` function in `ProtocolVersion.sol`. -export const CURRENT_PROTOCOL_VERSION: [number, number, number] = [1, 3, 0]; +export const CURRENT_PROTOCOL_VERSION: [number, number, number] = [1, 4, 0]; // The protocol version number of contracts not having a `getProtocolVersion()` function because they don't inherit from `ProtocolVersion.sol` yet. export const IMPLICIT_INITIAL_PROTOCOL_VERSION: [number, number, number] = [ diff --git a/packages/contracts/test/upgrade/dao.ts b/packages/contracts/test/upgrade/dao.ts index 406841ea0..b0d40269c 100644 --- a/packages/contracts/test/upgrade/dao.ts +++ b/packages/contracts/test/upgrade/dao.ts @@ -7,7 +7,12 @@ import { DAO__factory as DAO_V1_0_0__factory, } from '../../typechain/@aragon/osx-v1.0.1/core/dao/DAO.sol'; -import {DAO, DAO__factory, ProtocolVersion__factory} from '../../typechain'; +import { + DAO as DAO_V1_3_0, + DAO__factory as DAO_V1_3_0__factory, +} from '../../typechain/@aragon/osx-v1.3.0-rc0.2/core/dao/DAO.sol'; + +import {DAO, ProtocolVersion__factory} from '../../typechain'; import {daoExampleURI, ZERO_BYTES32} from '../test-utils/dao'; import {deployWithProxy} from '../test-utils/proxy'; @@ -19,12 +24,10 @@ import {UpgradedEvent} from '../../typechain/DAO'; import {IMPLICIT_INITIAL_PROTOCOL_VERSION} from '../test-utils/protocol-version'; let signers: SignerWithAddress[]; -let DAO_old: DAO_V1_0_0__factory; -let DAO_Current: DAO__factory; let daoV100Proxy: DAO_V1_0_0; -let daoV100Implementation: string; -let daoCurrentImplementaion: DAO; +let daoV100Implementation: DAO_V1_0_0; +let daoV130Implementation: DAO_V1_3_0; const EMPTY_DATA = '0x'; @@ -39,18 +42,16 @@ describe('DAO Upgrade', function () { before(async function () { signers = await ethers.getSigners(); - // We don't use the typchain here but directly grab the artifacts. This will be changed in an upcoming PR again. - DAO_old = new DAO_V1_0_0__factory(signers[0]); - DAO_Current = new DAO__factory(signers[0]); - // Deploy the v1.3.0 implementation - daoCurrentImplementaion = await DAO_Current.deploy(); + daoV130Implementation = await new DAO_V1_3_0__factory(signers[0]).deploy(); }); context(`Re-entrancy`, function () { context(`v1.0.0 to v1.3.0`, function () { beforeEach(async function () { - daoV100Proxy = await deployWithProxy(DAO_old); + daoV100Proxy = await deployWithProxy( + new DAO_V1_0_0__factory(signers[0]) + ); await daoV100Proxy.initialize( DUMMY_METADATA, signers[0].address, @@ -59,8 +60,8 @@ describe('DAO Upgrade', function () { ); // Store the v1.0.0 implementation - daoV100Implementation = await readImplementationValueFromSlot( - daoV100Proxy.address + daoV100Implementation = new DAO_V1_0_0__factory(signers[0]).attach( + await readImplementationValueFromSlot(daoV100Proxy.address) ); // Grant the upgrade permission @@ -74,8 +75,8 @@ describe('DAO Upgrade', function () { it('does not corrupt the DAO storage', async () => { // Upgrade and call `initializeFrom`. const upgradeTx = await daoV100Proxy.upgradeToAndCall( - daoCurrentImplementaion.address, - DAO_Current.interface.encodeFunctionData('initializeFrom', [ + daoV130Implementation.address, + daoV130Implementation.interface.encodeFunctionData('initializeFrom', [ IMPLICIT_INITIAL_PROTOCOL_VERSION, EMPTY_DATA, ]) @@ -85,7 +86,7 @@ describe('DAO Upgrade', function () { const implementationAfterUpgrade = await readImplementationValueFromSlot(daoV100Proxy.address); expect(implementationAfterUpgrade).to.equal( - daoCurrentImplementaion.address + daoV130Implementation.address ); expect(implementationAfterUpgrade).to.not.equal(daoV100Implementation); @@ -93,11 +94,11 @@ describe('DAO Upgrade', function () { const emittedImplementation = ( await findEventTopicLog( upgradeTx, - DAO_old.interface, + daoV130Implementation.interface, 'Upgraded' ) ).args.implementation; - expect(emittedImplementation).to.equal(daoCurrentImplementaion.address); + expect(emittedImplementation).to.equal(daoV130Implementation.address); // Check that storage is not corrupted. expect(await daoV100Proxy.callStatic.daoURI()).to.equal(daoExampleURI); @@ -140,8 +141,8 @@ describe('DAO Upgrade', function () { // Upgrade and call `initializeFrom`. await daoV100Proxy.upgradeToAndCall( - daoCurrentImplementaion.address, - DAO_Current.interface.encodeFunctionData('initializeFrom', [ + daoV130Implementation.address, + daoV130Implementation.interface.encodeFunctionData('initializeFrom', [ IMPLICIT_INITIAL_PROTOCOL_VERSION, EMPTY_DATA, ]) @@ -151,7 +152,7 @@ describe('DAO Upgrade', function () { const implementationAfterUpgrade = await readImplementationValueFromSlot(daoV100Proxy.address); expect(implementationAfterUpgrade).to.equal( - daoCurrentImplementaion.address + daoV130Implementation.address ); expect(implementationAfterUpgrade).to.not.equal(daoV100Implementation); @@ -220,8 +221,8 @@ describe('DAO Upgrade', function () { // Upgrade and call `initializeFrom`. await daoV100Proxy.upgradeToAndCall( - daoCurrentImplementaion.address, - DAO_Current.interface.encodeFunctionData('initializeFrom', [ + daoV130Implementation.address, + daoV130Implementation.interface.encodeFunctionData('initializeFrom', [ IMPLICIT_INITIAL_PROTOCOL_VERSION, EMPTY_DATA, ]) @@ -231,7 +232,7 @@ describe('DAO Upgrade', function () { const implementationAfterUpgrade = await readImplementationValueFromSlot(daoV100Proxy.address); expect(implementationAfterUpgrade).to.equal( - daoCurrentImplementaion.address + daoV130Implementation.address ); expect(implementationAfterUpgrade).to.not.equal(daoV100Implementation); @@ -262,7 +263,9 @@ describe('DAO Upgrade', function () { context(`Protocol Version`, function () { beforeEach(async function () { // prepare v1.0.0 - daoV100Proxy = await deployWithProxy(DAO_old); + daoV100Proxy = await deployWithProxy( + new DAO_V1_0_0__factory(signers[0]) + ); await daoV100Proxy.initialize( DUMMY_METADATA, signers[0].address, @@ -280,7 +283,9 @@ describe('DAO Upgrade', function () { it('fails to call protocolVersion on versions prior to v1.3.0 and succeeds from v1.3.0 onwards', async () => { // deploy the different versions - const daoCurrentProxy = await deployWithProxy(DAO_Current); + const daoCurrentProxy = await deployWithProxy( + new DAO_V1_3_0__factory(signers[0]) + ); await daoCurrentProxy.initialize( DUMMY_METADATA, signers[0].address, @@ -325,8 +330,8 @@ describe('DAO Upgrade', function () { // Upgrade and call `initializeFrom`. await daoV100Proxy.upgradeToAndCall( - daoCurrentImplementaion.address, - DAO_Current.interface.encodeFunctionData('initializeFrom', [ + daoV130Implementation.address, + daoV130Implementation.interface.encodeFunctionData('initializeFrom', [ IMPLICIT_INITIAL_PROTOCOL_VERSION, EMPTY_DATA, ]) @@ -343,14 +348,16 @@ describe('DAO Upgrade', function () { it('returns the correct protocol version after upgrade', async () => { // Upgrade and call `initializeFrom`. await daoV100Proxy.upgradeToAndCall( - daoCurrentImplementaion.address, - DAO_Current.interface.encodeFunctionData('initializeFrom', [ + daoV130Implementation.address, + daoV130Implementation.interface.encodeFunctionData('initializeFrom', [ IMPLICIT_INITIAL_PROTOCOL_VERSION, EMPTY_DATA, ]) ); - const daoV130 = DAO_Current.attach(daoV100Proxy.address); + const daoV130 = new DAO_V1_3_0__factory(signers[0]).attach( + daoV100Proxy.address + ); expect(await daoV130.protocolVersion()).to.be.deep.eq([1, 3, 0]); }); }); diff --git a/yarn.lock b/yarn.lock index ff9447a8e..5250a5dde 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14,6 +14,15 @@ resolved "https://registry.yarnpkg.com/@aragon/osx/-/osx-1.0.1.tgz#b758ba87db93a46a8ddabfaefc99ac8e44c46c78" integrity sha512-TiP5/1AGv/hth+V8PoDVFlwMzmLazYxzp//jiepAZ0WJkx9EnQNYafo+M7+pjAqRPG005liQjmFZNiK6ARLULg== +"@aragon/osx-v1.3.0-rc0.2@npm:@aragon/osx@1.3.0-rc0.2": + version "1.3.0-rc0.2" + resolved "https://registry.yarnpkg.com/@aragon/osx/-/osx-1.3.0-rc0.2.tgz#6922052bc7eaa180e143478b56292cc1e1245adf" + integrity sha512-IQTOPXgISEOGhcf2LeE4iz+TF4YmNzYjga9UgaIEYpcMkVKN1LUdJgvrF6VxGeGWiDjRTb0nNijtdmeOw4ungw== + dependencies: + "@ensdomains/ens-contracts" "0.0.11" + "@openzeppelin/contracts" "4.8.1" + "@openzeppelin/contracts-upgradeable" "4.8.1" + "@babel/code-frame@7.12.11": version "7.12.11" resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.12.11.tgz#f4ad435aa263db935b8f10f2c552d23fb716a63f"