From 0b7fee3ad8046f23ef44ba43894b58060d193d11 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Mon, 14 Oct 2024 23:05:41 +0200 Subject: [PATCH 01/16] (groups): upgradeableRenounceableProxy for group policies --- src/groups/UpgradeableRenounceableProxy.sol | 49 +++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 src/groups/UpgradeableRenounceableProxy.sol diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol new file mode 100644 index 0000000..1cc1105 --- /dev/null +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity >=0.8.24; + +import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +contract UpgradeableRenounceableProxy is ERC1967Proxy { + // Errors + + error OnlyAdminError(); + + error BlockReceive(); + + // Modifier + + modifier onlyAdmin() { + if (msg.sender != ERC1967Utils.getAdmin()) { + revert OnlyAdminError(); + } + _; + } + + // Constructor + + constructor(address _implementation, bytes memory _data) ERC1967Proxy(_implementation, _data) { + // set the admin to the deployer + ERC1967Utils.changeAdmin(msg.sender); + } + + function implementation() external view returns (address) { + return _implementation(); + } + + function upgradeToAndCall(address _newImplementation, bytes memory _data) external onlyAdmin { + ERC1967Utils.upgradeToAndCall(_newImplementation, _data); + } + + /** + * @dev It renounces the ability to upgrade the contract, by setting the admin to 0x01. + */ + function renounceUpgradeability() external onlyAdmin { + ERC1967Utils.changeAdmin(address(0x01)); + } + + // Fallback function + + receive() external payable { + revert BlockReceive(); + } +} From ad5f87c2fb0ef15fd6329f32d207f0501aea3e2a Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 8 Nov 2024 16:24:46 +0000 Subject: [PATCH 02/16] Update src/groups/UpgradeableRenounceableProxy.sol Co-authored-by: Yevgeniy <35062472+roleengineer@users.noreply.github.com> --- src/groups/UpgradeableRenounceableProxy.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol index 1cc1105..ce91abd 100644 --- a/src/groups/UpgradeableRenounceableProxy.sol +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -2,7 +2,11 @@ pragma solidity >=0.8.24; import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; - +interface IUpgradeableRenounceableProxy { + function implementation() external view returns (address); + function upgradeToAndCall(address _newImplementation, bytes memory _data) external; + function renounceUpgradeability() external; +} contract UpgradeableRenounceableProxy is ERC1967Proxy { // Errors From be1657c4e3b0b9db0fe1d4fec205c77aa5d6765d Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 8 Nov 2024 16:24:54 +0000 Subject: [PATCH 03/16] Update src/groups/UpgradeableRenounceableProxy.sol Co-authored-by: Yevgeniy <35062472+roleengineer@users.noreply.github.com> --- src/groups/UpgradeableRenounceableProxy.sol | 42 ++++++++++++--------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol index ce91abd..1b95bc4 100644 --- a/src/groups/UpgradeableRenounceableProxy.sol +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -10,39 +10,47 @@ interface IUpgradeableRenounceableProxy { contract UpgradeableRenounceableProxy is ERC1967Proxy { // Errors - error OnlyAdminError(); - error BlockReceive(); + + // Constants - // Modifier - - modifier onlyAdmin() { - if (msg.sender != ERC1967Utils.getAdmin()) { - revert OnlyAdminError(); - } - _; - } + /// @dev Initial proxy admin. + address internal immutable ADMIN_INIT; // Constructor constructor(address _implementation, bytes memory _data) ERC1967Proxy(_implementation, _data) { // set the admin to the deployer ERC1967Utils.changeAdmin(msg.sender); + // set the admin as immutable + ADMIN_INIT = msg.sender; } function implementation() external view returns (address) { return _implementation(); } - function upgradeToAndCall(address _newImplementation, bytes memory _data) external onlyAdmin { - ERC1967Utils.upgradeToAndCall(_newImplementation, _data); + /// @dev Dispatch if caller is admin. + function _fallback() internal virtual override { + if (msg.sender == ADMIN_INIT && msg.sender == ERC1967Utils.getAdmin()) { + _dispatchAdmin(); + } else { + super._fallback(); + } } - /** - * @dev It renounces the ability to upgrade the contract, by setting the admin to 0x01. - */ - function renounceUpgradeability() external onlyAdmin { - ERC1967Utils.changeAdmin(address(0x01)); + /// @dev Upgrades to new implementation, renounces the ability to upgrade or moves to regular flow based on admin request. + function _dispatchAdmin() private { + if (msg.sig == IUpgradeableRenounceableProxy.upgradeToAndCall.selector) { + // upgrades to new implementation + (address newImplementation, bytes memory data) = abi.decode(msg.data[4:], (address, bytes)); + ERC1967Utils.upgradeToAndCall(newImplementation, data); + } else if (msg.sig == IUpgradeableRenounceableProxy.renounceUpgradeability.selector) { + // renounces the ability to upgrade the contract, by setting the admin to 0x01. + ERC1967Utils.changeAdmin(address(0x01)); + } else { + _delegate(_implementation()); + } } // Fallback function From e9837e2e120f8423b537c066df36db26ed3003ef Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 8 Nov 2024 17:22:54 +0000 Subject: [PATCH 04/16] (proxy): forge fmt --- src/groups/UpgradeableRenounceableProxy.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol index 1b95bc4..ffdc1f0 100644 --- a/src/groups/UpgradeableRenounceableProxy.sol +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -2,16 +2,18 @@ pragma solidity >=0.8.24; import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + interface IUpgradeableRenounceableProxy { function implementation() external view returns (address); function upgradeToAndCall(address _newImplementation, bytes memory _data) external; function renounceUpgradeability() external; } + contract UpgradeableRenounceableProxy is ERC1967Proxy { // Errors error BlockReceive(); - + // Constants /// @dev Initial proxy admin. From 1e104569b2f9aec80a32cb51d7d6263765f17f12 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Tue, 12 Nov 2024 18:31:22 +0000 Subject: [PATCH 05/16] Update src/groups/UpgradeableRenounceableProxy.sol Co-authored-by: Yevgeniy <35062472+roleengineer@users.noreply.github.com> --- src/groups/UpgradeableRenounceableProxy.sol | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol index ffdc1f0..37eb70e 100644 --- a/src/groups/UpgradeableRenounceableProxy.sol +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -28,12 +28,20 @@ contract UpgradeableRenounceableProxy is ERC1967Proxy { ADMIN_INIT = msg.sender; } - function implementation() external view returns (address) { - return _implementation(); - } - /// @dev Dispatch if caller is admin. + /// @dev Handles proxy function calls: attempts to dispatch to a specific + /// function or delegates all calls to the implementation contract. function _fallback() internal virtual override { + // staticcall implementation() returns the address + if (msg.sig == IUpgradeableRenounceableProxy.implementation.selector) { + bytes32 slot = ERC1967Utils.IMPLEMENTATION_SLOT; + assembly { + let implementation := sload(slot) + mstore(0, shr(12, shl(12, implementation))) + return(0, 0x20) + } + } + // dispatch if caller is admin, otherwise delegate to the implementation if (msg.sender == ADMIN_INIT && msg.sender == ERC1967Utils.getAdmin()) { _dispatchAdmin(); } else { From 8b637ad7611ff01da8cc95e480ca64b90db1118a Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Tue, 12 Nov 2024 18:36:35 +0000 Subject: [PATCH 06/16] (groups): forge fmt --- src/groups/UpgradeableRenounceableProxy.sol | 1 - test/groups/upgradeableRenouncableProxy.t.sol | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 test/groups/upgradeableRenouncableProxy.t.sol diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol index 37eb70e..ef50cbf 100644 --- a/src/groups/UpgradeableRenounceableProxy.sol +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -28,7 +28,6 @@ contract UpgradeableRenounceableProxy is ERC1967Proxy { ADMIN_INIT = msg.sender; } - /// @dev Handles proxy function calls: attempts to dispatch to a specific /// function or delegates all calls to the implementation contract. function _fallback() internal virtual override { diff --git a/test/groups/upgradeableRenouncableProxy.t.sol b/test/groups/upgradeableRenouncableProxy.t.sol new file mode 100644 index 0000000..3388958 --- /dev/null +++ b/test/groups/upgradeableRenouncableProxy.t.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity >=0.8.13; + +import {Test} from "forge-std/Test.sol"; +import {StdCheats} from "forge-std/StdCheats.sol"; +import "forge-std/console.sol"; +import "../../src/errors/Errors.sol"; +import "./setup.sol"; + +contract UpgradeableRenounceableProxyTest is Test, GroupSetup { + // Constructor + + constructor() GroupSetup() {} + + // Setup + + function setUp() public {} +} From b3a4dbef83722cb51ab5664828606ade4e349d8a Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Tue, 12 Nov 2024 19:16:49 +0000 Subject: [PATCH 07/16] (test/setup): rename HumanRegistration to AvatarCreation --- test/groups/setup.sol | 6 +++--- test/hub/PathTransferHub.t.sol | 6 +++--- test/hub/V1MintStatusUpdate.t.sol | 6 +++--- test/lift/ERC20Demurrage.t.sol | 6 +++--- test/lift/ERC20Inflationary.t.sol | 6 +++--- test/lift/ERC20Lift.t.sol | 6 +++--- test/names/NameRegistry.t.sol | 6 +++--- test/setup/{HumanRegistration.sol => AvatarCreation.sol} | 2 +- 8 files changed, 22 insertions(+), 22 deletions(-) rename test/setup/{HumanRegistration.sol => AvatarCreation.sol} (98%) diff --git a/test/groups/setup.sol b/test/groups/setup.sol index fb50a07..1c1cd43 100644 --- a/test/groups/setup.sol +++ b/test/groups/setup.sol @@ -2,12 +2,12 @@ pragma solidity >=0.8.13; import "../../src/groups/BaseMintPolicy.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "../setup/TimeCirclesSetup.sol"; import "../hub/MockDeployment.sol"; import "../hub/MockHub.sol"; -contract GroupSetup is TimeCirclesSetup, HumanRegistration { +contract GroupSetup is TimeCirclesSetup, AvatarCreation { // State variables MockDeployment public mockDeployment; @@ -16,7 +16,7 @@ contract GroupSetup is TimeCirclesSetup, HumanRegistration { // Constructor - constructor() HumanRegistration(40) {} + constructor() AvatarCreation(40) {} // Setup diff --git a/test/hub/PathTransferHub.t.sol b/test/hub/PathTransferHub.t.sol index 59e44a0..bea72cb 100644 --- a/test/hub/PathTransferHub.t.sol +++ b/test/hub/PathTransferHub.t.sol @@ -7,18 +7,18 @@ import "forge-std/console.sol"; import "../../src/hub/Hub.sol"; import "../../src/hub/TypeDefinitions.sol"; import "../setup/TimeCirclesSetup.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "../utils/Approximation.sol"; import "./MockPathTransferHub.sol"; -contract HubPathTransferTest is Test, TimeCirclesSetup, HumanRegistration, Approximation { +contract HubPathTransferTest is Test, TimeCirclesSetup, AvatarCreation, Approximation { // State variables MockPathTransferHub public mockHub; // Constructor - constructor() HumanRegistration(4) {} + constructor() AvatarCreation(4) {} // Setup diff --git a/test/hub/V1MintStatusUpdate.t.sol b/test/hub/V1MintStatusUpdate.t.sol index 75345a5..0074579 100644 --- a/test/hub/V1MintStatusUpdate.t.sol +++ b/test/hub/V1MintStatusUpdate.t.sol @@ -9,11 +9,11 @@ import "../../src/migration/IToken.sol"; import "../../src/migration/Migration.sol"; import "../../src/names/NameRegistry.sol"; import "../setup/TimeCirclesSetup.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "../migration/MockHub.sol"; import "./MockMigrationHub.sol"; -contract V1MintStatusUpdateTest is Test, TimeCirclesSetup, HumanRegistration { +contract V1MintStatusUpdateTest is Test, TimeCirclesSetup, AvatarCreation { // State variables MockMigrationHub public mockHub; @@ -24,7 +24,7 @@ contract V1MintStatusUpdateTest is Test, TimeCirclesSetup, HumanRegistration { // Constructor - constructor() HumanRegistration(2) {} + constructor() AvatarCreation(2) {} // Setup diff --git a/test/lift/ERC20Demurrage.t.sol b/test/lift/ERC20Demurrage.t.sol index 3c0180b..937b1f3 100644 --- a/test/lift/ERC20Demurrage.t.sol +++ b/test/lift/ERC20Demurrage.t.sol @@ -6,11 +6,11 @@ import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; import "../../src/circles/Demurrage.sol"; import "../setup/TimeCirclesSetup.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "../hub/MockDeployment.sol"; import "../hub/MockHub.sol"; -contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration { +contract ERC20LiftTest is Test, TimeCirclesSetup, AvatarCreation { // State variables MockDeployment public mockDeployment; @@ -20,7 +20,7 @@ contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration { // Constructor - constructor() HumanRegistration(2) {} + constructor() AvatarCreation(2) {} // Setup diff --git a/test/lift/ERC20Inflationary.t.sol b/test/lift/ERC20Inflationary.t.sol index 7705ee2..3cb07ca 100644 --- a/test/lift/ERC20Inflationary.t.sol +++ b/test/lift/ERC20Inflationary.t.sol @@ -6,12 +6,12 @@ import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; import "../../src/circles/Demurrage.sol"; import "../setup/TimeCirclesSetup.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "../hub/MockDeployment.sol"; import "../hub/MockHub.sol"; import "../utils/Approximation.sol"; -contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration, Approximation { +contract ERC20LiftTest is Test, TimeCirclesSetup, AvatarCreation, Approximation { // State variables MockDeployment public mockDeployment; @@ -21,7 +21,7 @@ contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration, Approximati // Constructor - constructor() HumanRegistration(2) {} + constructor() AvatarCreation(2) {} // Setup diff --git a/test/lift/ERC20Lift.t.sol b/test/lift/ERC20Lift.t.sol index 3ae2d89..ad6cfcd 100644 --- a/test/lift/ERC20Lift.t.sol +++ b/test/lift/ERC20Lift.t.sol @@ -6,11 +6,11 @@ import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; import "../../src/circles/Demurrage.sol"; import "../setup/TimeCirclesSetup.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "../hub/MockDeployment.sol"; import "../hub/MockHub.sol"; -contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration { +contract ERC20LiftTest is Test, TimeCirclesSetup, AvatarCreation { // State variables MockDeployment public mockDeployment; @@ -18,7 +18,7 @@ contract ERC20LiftTest is Test, TimeCirclesSetup, HumanRegistration { // Constructor - constructor() HumanRegistration(2) {} + constructor() AvatarCreation(2) {} // Setup diff --git a/test/names/NameRegistry.t.sol b/test/names/NameRegistry.t.sol index 90f4d95..46ea8ad 100644 --- a/test/names/NameRegistry.t.sol +++ b/test/names/NameRegistry.t.sol @@ -4,10 +4,10 @@ pragma solidity >=0.8.24; import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; -import "../setup/HumanRegistration.sol"; +import "../setup/AvatarCreation.sol"; import "./MockNameRegistry.sol"; -contract NamesTest is Test, HumanRegistration { +contract NamesTest is Test, AvatarCreation { // Constants // IPFS hash for Ubuntu 20.04, random CIDv0 @@ -22,7 +22,7 @@ contract NamesTest is Test, HumanRegistration { // Constructor - constructor() HumanRegistration(4) {} + constructor() AvatarCreation(4) {} // Setup diff --git a/test/setup/HumanRegistration.sol b/test/setup/AvatarCreation.sol similarity index 98% rename from test/setup/HumanRegistration.sol rename to test/setup/AvatarCreation.sol index de359a4..61db285 100644 --- a/test/setup/HumanRegistration.sol +++ b/test/setup/AvatarCreation.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.13; import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; -contract HumanRegistration is Test { +contract AvatarCreation is Test { // Constants uint256 public immutable N; From 1ad0261858b0a2eb5cfc1fa17a35729e8f470173 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Thu, 14 Nov 2024 21:38:17 +0000 Subject: [PATCH 08/16] (test/groups): starting, but out of context and not seeing why compiler complains when importing proxy --- test/groups/compositeMintGroups.sol | 2 +- test/groups/createGroups.t.sol | 2 +- test/groups/{setup.sol => groupSetup.sol} | 0 test/groups/mintGroupCircles.t.sol | 2 +- test/groups/upgradeableRenouncableProxy.t.sol | 20 +++++++++++++++++-- 5 files changed, 21 insertions(+), 5 deletions(-) rename test/groups/{setup.sol => groupSetup.sol} (100%) diff --git a/test/groups/compositeMintGroups.sol b/test/groups/compositeMintGroups.sol index dc03282..c79ba2b 100644 --- a/test/groups/compositeMintGroups.sol +++ b/test/groups/compositeMintGroups.sol @@ -5,7 +5,7 @@ import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; import "../../src/errors/Errors.sol"; -import "./setup.sol"; +import "./groupSetup.sol"; contract CompositeMintGroupsTest is Test, GroupSetup, IHubErrors { // State variables diff --git a/test/groups/createGroups.t.sol b/test/groups/createGroups.t.sol index e73b967..78540a6 100644 --- a/test/groups/createGroups.t.sol +++ b/test/groups/createGroups.t.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.13; import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; -import "./setup.sol"; +import "./groupSetup.sol"; contract GroupMintTest is Test, GroupSetup { // Constructor diff --git a/test/groups/setup.sol b/test/groups/groupSetup.sol similarity index 100% rename from test/groups/setup.sol rename to test/groups/groupSetup.sol diff --git a/test/groups/mintGroupCircles.t.sol b/test/groups/mintGroupCircles.t.sol index c72bc25..22f3d2e 100644 --- a/test/groups/mintGroupCircles.t.sol +++ b/test/groups/mintGroupCircles.t.sol @@ -5,7 +5,7 @@ import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; import "../../src/errors/Errors.sol"; -import "./setup.sol"; +import "./groupSetup.sol"; contract MintGroupCirclesTest is Test, GroupSetup, IHubErrors { // State variables diff --git a/test/groups/upgradeableRenouncableProxy.t.sol b/test/groups/upgradeableRenouncableProxy.t.sol index 3388958..c76572a 100644 --- a/test/groups/upgradeableRenouncableProxy.t.sol +++ b/test/groups/upgradeableRenouncableProxy.t.sol @@ -4,15 +4,31 @@ pragma solidity >=0.8.13; import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; +import "../../src/groups/UpgradeableRenounceableProxy.sol"; import "../../src/errors/Errors.sol"; -import "./setup.sol"; +import "./groupSetup.sol"; contract UpgradeableRenounceableProxyTest is Test, GroupSetup { + // State variables + address group; + + // UpgradeableRenounceableProxy public policyProxy; + // Constructor constructor() GroupSetup() {} // Setup - function setUp() public {} + function setUp() public { + groupSetup(); + + group = addresses[39]; + } + + // // Tests + + // function testProxyGroup() public { + + // } } From 8a3b7fc8391db35e2d5ddd930e3fdde015244acd Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 15 Nov 2024 17:35:10 +0000 Subject: [PATCH 09/16] (groups): finally! found the obtuse error on conflicting import errors --- src/groups/UpgradeableRenounceableProxy.sol | 3 ++- test/groups/upgradeableRenouncableProxy.t.sol | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/groups/UpgradeableRenounceableProxy.sol b/src/groups/UpgradeableRenounceableProxy.sol index ef50cbf..244b7b6 100644 --- a/src/groups/UpgradeableRenounceableProxy.sol +++ b/src/groups/UpgradeableRenounceableProxy.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: AGPL-3.0-only pragma solidity >=0.8.24; -import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol"; interface IUpgradeableRenounceableProxy { function implementation() external view returns (address); diff --git a/test/groups/upgradeableRenouncableProxy.t.sol b/test/groups/upgradeableRenouncableProxy.t.sol index c76572a..6da72da 100644 --- a/test/groups/upgradeableRenouncableProxy.t.sol +++ b/test/groups/upgradeableRenouncableProxy.t.sol @@ -11,8 +11,6 @@ import "./groupSetup.sol"; contract UpgradeableRenounceableProxyTest is Test, GroupSetup { // State variables address group; - - // UpgradeableRenounceableProxy public policyProxy; // Constructor From 1443a508eef762860f77b594dbfee8b448fd893f Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 15 Nov 2024 19:04:20 +0000 Subject: [PATCH 10/16] (test/groups): creation test with proxy --- test/groups/upgradeableRenouncableProxy.t.sol | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/test/groups/upgradeableRenouncableProxy.t.sol b/test/groups/upgradeableRenouncableProxy.t.sol index 6da72da..ddd04c6 100644 --- a/test/groups/upgradeableRenouncableProxy.t.sol +++ b/test/groups/upgradeableRenouncableProxy.t.sol @@ -10,7 +10,7 @@ import "./groupSetup.sol"; contract UpgradeableRenounceableProxyTest is Test, GroupSetup { // State variables - address group; + address public group; // Constructor @@ -19,14 +19,20 @@ contract UpgradeableRenounceableProxyTest is Test, GroupSetup { // Setup function setUp() public { + // first 35 addresses are registered as human + // in mock deployment, with 14 days of mint groupSetup(); - group = addresses[39]; + group = addresses[36]; } - // // Tests + // Tests - // function testProxyGroup() public { - - // } + function testRegisterGroupWithProxyPolicy() public { + // create a proxy deployment with the mint policy as implementation + vm.startPrank(group); + UpgradeableRenounceableProxy proxy = new UpgradeableRenounceableProxy(mintPolicy, ""); + hub.registerGroup(address(proxy), "ProxyPolicyGroup", "PPG", bytes32(0)); + vm.stopPrank(); + } } From 23a48d4d4108191da7c3909940c2935b54fb6166 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 15 Nov 2024 19:45:40 +0000 Subject: [PATCH 11/16] (test/groups): test minting over proxy policy --- .../createUpgradeableRenouncableProxy.t.sol | 79 +++++++++++++++++++ test/groups/upgradeableRenouncableProxy.t.sol | 38 --------- 2 files changed, 79 insertions(+), 38 deletions(-) create mode 100644 test/groups/createUpgradeableRenouncableProxy.t.sol delete mode 100644 test/groups/upgradeableRenouncableProxy.t.sol diff --git a/test/groups/createUpgradeableRenouncableProxy.t.sol b/test/groups/createUpgradeableRenouncableProxy.t.sol new file mode 100644 index 0000000..7ea63f5 --- /dev/null +++ b/test/groups/createUpgradeableRenouncableProxy.t.sol @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity >=0.8.13; + +import {Test} from "forge-std/Test.sol"; +import {StdCheats} from "forge-std/StdCheats.sol"; +import "forge-std/console.sol"; +import "../../src/groups/UpgradeableRenounceableProxy.sol"; +import "../../src/errors/Errors.sol"; +import "./groupSetup.sol"; + +contract UpgradeableRenounceableProxyTest is Test, GroupSetup { + // State variables + address public group; + + // Constructor + + constructor() GroupSetup() {} + + // Setup + + function setUp() public { + // first 35 addresses are registered as human + // in mock deployment, with 14 days of mint + groupSetup(); + + // 36: Kevin + group = addresses[36]; + } + + // Tests + + function testRegisterGroupWithProxyPolicy() public { + _createGroupWithProxyPolicy(); + } + + function testMintGroupWithProxyPolicy() public { + UpgradeableRenounceableProxy proxy = _createGroupWithProxyPolicy(); + + address alice = addresses[0]; + + // group must trust alice + vm.prank(group); + hub.trust(alice, INDEFINITE_FUTURE); + + // test minting to group + _testGroupMintOwnCollateral(alice, group, 1 * CRC); + } + + // Internal functions + + function _createGroupWithProxyPolicy() internal returns (UpgradeableRenounceableProxy) { + // create a proxy deployment with the mint policy as implementation + vm.startPrank(group); + UpgradeableRenounceableProxy proxy = new UpgradeableRenounceableProxy(mintPolicy, ""); + hub.registerGroup(address(proxy), "ProxyPolicyGroup", "PPG", bytes32(0)); + vm.stopPrank(); + + return proxy; + } + + function _testGroupMintOwnCollateral(address _minter, address _group, uint256 _amount) internal { + uint256 tokenIdGroup = uint256(uint160(_group)); + + address[] memory collateral = new address[](1); + uint256[] memory amounts = new uint256[](1); + collateral[0] = _minter; + amounts[0] = _amount; + + // check balance of group before mint + uint256 balanceBefore = hub.balanceOf(_minter, tokenIdGroup); + + vm.prank(_minter); + hub.groupMint(_group, collateral, amounts, ""); + + // check balance of group after mint + uint256 balanceAfter = hub.balanceOf(_minter, tokenIdGroup); + assertEq(balanceAfter, balanceBefore + _amount); + } +} diff --git a/test/groups/upgradeableRenouncableProxy.t.sol b/test/groups/upgradeableRenouncableProxy.t.sol deleted file mode 100644 index ddd04c6..0000000 --- a/test/groups/upgradeableRenouncableProxy.t.sol +++ /dev/null @@ -1,38 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-only -pragma solidity >=0.8.13; - -import {Test} from "forge-std/Test.sol"; -import {StdCheats} from "forge-std/StdCheats.sol"; -import "forge-std/console.sol"; -import "../../src/groups/UpgradeableRenounceableProxy.sol"; -import "../../src/errors/Errors.sol"; -import "./groupSetup.sol"; - -contract UpgradeableRenounceableProxyTest is Test, GroupSetup { - // State variables - address public group; - - // Constructor - - constructor() GroupSetup() {} - - // Setup - - function setUp() public { - // first 35 addresses are registered as human - // in mock deployment, with 14 days of mint - groupSetup(); - - group = addresses[36]; - } - - // Tests - - function testRegisterGroupWithProxyPolicy() public { - // create a proxy deployment with the mint policy as implementation - vm.startPrank(group); - UpgradeableRenounceableProxy proxy = new UpgradeableRenounceableProxy(mintPolicy, ""); - hub.registerGroup(address(proxy), "ProxyPolicyGroup", "PPG", bytes32(0)); - vm.stopPrank(); - } -} From 7471971d441fcd8051e4d01ed508db821d253636 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 15 Nov 2024 21:29:46 +0000 Subject: [PATCH 12/16] (test/groups): test upgrade implementation --- ...erationsUpgradeableRenounceableProxy.t.sol | 72 +++++++++++++++++++ ...sePolicyUpgradeableRenouncableProxy.t.sol} | 8 +-- 2 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol rename test/groups/{createUpgradeableRenouncableProxy.t.sol => upgradeableProxy/usePolicyUpgradeableRenouncableProxy.t.sol} (91%) diff --git a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol new file mode 100644 index 0000000..2717d30 --- /dev/null +++ b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity >=0.8.13; + +import {Test} from "forge-std/Test.sol"; +import {StdCheats} from "forge-std/StdCheats.sol"; +import "forge-std/console.sol"; +import "../../../src/groups/UpgradeableRenounceableProxy.sol"; +import "../../../src/errors/Errors.sol"; +import "../groupSetup.sol"; + +contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { + // State variables + + address public group; + IUpgradeableRenounceableProxy public proxy; + + // Constructor + + constructor() GroupSetup() {} + + // Setup + + function setUp() public { + // first 35 addresses are registered as human + // in mock deployment, with 14 days of mint + groupSetup(); + + // 36: Kevin + group = addresses[36]; + + // create a proxy deployment with the mint policy as implementation + vm.startPrank(group); + proxy = IUpgradeableRenounceableProxy(address(new UpgradeableRenounceableProxy(mintPolicy, ""))); + hub.registerGroup(address(proxy), "ProxyPolicyGroup", "PPG", bytes32(0)); + vm.stopPrank(); + + for (uint256 i = 0; i < 5; i++) { + vm.prank(group); + hub.trust(addresses[i], INDEFINITE_FUTURE); + } + } + + // Tests + + function testGetImplementation() public { + address implementation = proxy.implementation(); + assertEq(implementation, mintPolicy); + } + + /* todo: - test getting admin from proxy + * - test admin cannot be changed + * - test noone else can call upgradeToAndCall + * - test upgradeToAndCall with call data + * - test renouncing admin + */ + + function testUpgradeToAndCall() public { + address originalImplementation = proxy.implementation(); + assertEq(originalImplementation, mintPolicy); + + // deploy a new copy of base mint policy + address newMintPolicy = address(new MintPolicy()); + + // upgrade the proxy to the new implementation + vm.prank(group); + proxy.upgradeToAndCall(newMintPolicy, ""); + + // check that the implementation has changed + address newImplementation = proxy.implementation(); + assertEq(newImplementation, newMintPolicy); + } +} diff --git a/test/groups/createUpgradeableRenouncableProxy.t.sol b/test/groups/upgradeableProxy/usePolicyUpgradeableRenouncableProxy.t.sol similarity index 91% rename from test/groups/createUpgradeableRenouncableProxy.t.sol rename to test/groups/upgradeableProxy/usePolicyUpgradeableRenouncableProxy.t.sol index 7ea63f5..99a3e6e 100644 --- a/test/groups/createUpgradeableRenouncableProxy.t.sol +++ b/test/groups/upgradeableProxy/usePolicyUpgradeableRenouncableProxy.t.sol @@ -4,11 +4,11 @@ pragma solidity >=0.8.13; import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; import "forge-std/console.sol"; -import "../../src/groups/UpgradeableRenounceableProxy.sol"; -import "../../src/errors/Errors.sol"; -import "./groupSetup.sol"; +import "../../../src/groups/UpgradeableRenounceableProxy.sol"; +import "../../../src/errors/Errors.sol"; +import "../groupSetup.sol"; -contract UpgradeableRenounceableProxyTest is Test, GroupSetup { +contract usePolicyUpgradeableRenounceableProxyTest is Test, GroupSetup { // State variables address public group; From 0e55cb36f96c31b613adfcd5213ee56698494e03 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 15 Nov 2024 21:38:42 +0000 Subject: [PATCH 13/16] (test/groups): also test mint works with upgraded implementation --- ...erationsUpgradeableRenounceableProxy.t.sol | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol index 2717d30..ba76fca 100644 --- a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol +++ b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol @@ -68,5 +68,30 @@ contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { // check that the implementation has changed address newImplementation = proxy.implementation(); assertEq(newImplementation, newMintPolicy); + + // test minting to group with new policy + _testGroupMintOwnCollateral(addresses[0], group, 1 * CRC); + } + + // Internal functions + + // todo: this is a duplicate; test helpers can be better structured + function _testGroupMintOwnCollateral(address _minter, address _group, uint256 _amount) internal { + uint256 tokenIdGroup = uint256(uint160(_group)); + + address[] memory collateral = new address[](1); + uint256[] memory amounts = new uint256[](1); + collateral[0] = _minter; + amounts[0] = _amount; + + // check balance of group before mint + uint256 balanceBefore = hub.balanceOf(_minter, tokenIdGroup); + + vm.prank(_minter); + hub.groupMint(_group, collateral, amounts, ""); + + // check balance of group after mint + uint256 balanceAfter = hub.balanceOf(_minter, tokenIdGroup); + assertEq(balanceAfter, balanceBefore + _amount); } } From d32aa9cb688e64aa22ca3470df4a4f09849a22b3 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Fri, 15 Nov 2024 22:20:30 +0000 Subject: [PATCH 14/16] (test/groups): test upgrade and renounce --- ...erationsUpgradeableRenounceableProxy.t.sol | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol index ba76fca..0c35c4d 100644 --- a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol +++ b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol @@ -3,12 +3,17 @@ pragma solidity >=0.8.13; import {Test} from "forge-std/Test.sol"; import {StdCheats} from "forge-std/StdCheats.sol"; +import {StorageSlot} from "@openzeppelin/contracts/utils/StorageSlot.sol"; import "forge-std/console.sol"; import "../../../src/groups/UpgradeableRenounceableProxy.sol"; import "../../../src/errors/Errors.sol"; import "../groupSetup.sol"; contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { + // Constants + + bytes32 internal constant ADMIN_SLOT = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + // State variables address public group; @@ -51,7 +56,8 @@ contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { * - test admin cannot be changed * - test noone else can call upgradeToAndCall * - test upgradeToAndCall with call data - * - test renouncing admin + * - test renouncing admin (DONE) + * - test accessibility of interface functions from non-Admin callers */ function testUpgradeToAndCall() public { @@ -73,6 +79,24 @@ contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { _testGroupMintOwnCollateral(addresses[0], group, 1 * CRC); } + function testRenounceAdmin() public { + // todo: it's not trivial (or impossible?) to read the ADMIN_SLOT from the proxy from a test contract + // this can only be read over the RPC? + // So for now, just test that after renouncing the admin, the group is no longer able to upgrade + // To properly test this, we need to mock the proxy to have an admin() func + // But we can also see this in the test trace, so maybe not necessary + + // renounce admin + vm.prank(group); + proxy.renounceUpgradeability(); + + // expect revert when trying to upgrade to implementation 0xdead + vm.startPrank(group); + vm.expectRevert(); + proxy.upgradeToAndCall(address(0xdead), ""); + vm.stopPrank(); + } + // Internal functions // todo: this is a duplicate; test helpers can be better structured From e63bc6c99408242f2952d79ed35372a5171a9884 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Mon, 18 Nov 2024 17:00:05 +0000 Subject: [PATCH 15/16] Update test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol Co-authored-by: Yevgeniy <35062472+roleengineer@users.noreply.github.com> --- ...minOperationsUpgradeableRenounceableProxy.t.sol | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol index 0c35c4d..216cc9b 100644 --- a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol +++ b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol @@ -80,16 +80,18 @@ contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { } function testRenounceAdmin() public { - // todo: it's not trivial (or impossible?) to read the ADMIN_SLOT from the proxy from a test contract - // this can only be read over the RPC? - // So for now, just test that after renouncing the admin, the group is no longer able to upgrade - // To properly test this, we need to mock the proxy to have an admin() func - // But we can also see this in the test trace, so maybe not necessary - + // current admin + address admin = address(uint160(uint256(vm.load(address(proxy), ADMIN_SLOT)))); + assertEq(admin, group); + // renounce admin vm.prank(group); proxy.renounceUpgradeability(); + // renounced admin + admin = address(uint160(uint256(vm.load(address(proxy), ADMIN_SLOT)))); + assertEq(admin, address(0x1)); + // expect revert when trying to upgrade to implementation 0xdead vm.startPrank(group); vm.expectRevert(); From 723ea360f744c35bd5a9350e9049962580cc10e7 Mon Sep 17 00:00:00 2001 From: Benjamin Bollen Date: Mon, 18 Nov 2024 17:01:59 +0000 Subject: [PATCH 16/16] (test/groups): forge fmt --- .../adminOperationsUpgradeableRenounceableProxy.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol index 216cc9b..5724938 100644 --- a/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol +++ b/test/groups/upgradeableProxy/adminOperationsUpgradeableRenounceableProxy.t.sol @@ -83,7 +83,7 @@ contract adminOperationsUpgradeableRenounceableProxy is Test, GroupSetup { // current admin address admin = address(uint160(uint256(vm.load(address(proxy), ADMIN_SLOT)))); assertEq(admin, group); - + // renounce admin vm.prank(group); proxy.renounceUpgradeability();