Add upgradable tokens and proxy

contracts/Baal.sol

@@ -15,7 +15,7 @@ import "@gnosis.pm/zodiac/contracts/core/Module.sol";
 import "@gnosis.pm/safe-contracts/contracts/common/Enum.sol";
 import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";
-import "@openzeppelin/contracts/proxy/Clones.sol";
+import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 
 import "./interfaces/IBaalToken.sol";
@@ -255,19 +255,23 @@ contract Baal is Module, EIP712, ReentrancyGuard {
 
         require(_lootSingleton != address(0), "!lootSingleton");
 
-        /*Clone loot singleton using EIP1167 minimal proxy pattern*/
-        lootToken = IBaalToken(Clones.clone(_lootSingleton));
-        /*TODO this naming feels too opinionated*/
-        lootToken.setUp(
-            string(abi.encodePacked(_name, " LOOT")),
-            string(abi.encodePacked(_symbol, "-LOOT"))
-        );
+        lootToken = IBaalToken(address(new ERC1967Proxy(
+            _lootSingleton,
+            abi.encodeWithSelector(
+                IBaalToken(_lootSingleton).setUp.selector, 
+                string(abi.encodePacked(_name, " LOOT")), 
+                string(abi.encodePacked(_symbol, "-LOOT")))
+        )));
 
         require(_sharesSingleton != address(0), "!sharesSingleton");
 
-        /*Clone loot singleton using EIP1167 minimal proxy pattern*/
-        sharesToken = IBaalToken(Clones.clone(_sharesSingleton));
-        sharesToken.setUp(_name, _symbol);
+        sharesToken = IBaalToken(address(new ERC1967Proxy(
+            _sharesSingleton,
+            abi.encodeWithSelector(
+                IBaalToken(_sharesSingleton).setUp.selector, 
+                _name, 
+                _symbol)
+        )));
 
         /*Set address of Gnosis multisend library to use for all execution*/
         multisendLibrary = _multisendLibrary;

contracts/LootERC20.sol

@@ -4,12 +4,18 @@ pragma solidity 0.8.7;
 import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20SnapshotUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol";
-import "./interfaces/IBaal.sol";
-
+import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
 import "@openzeppelin/contracts/utils/Address.sol";
 
+import "./interfaces/IBaal.sol";
 
-contract Loot is ERC20SnapshotUpgradeable, ERC20PermitUpgradeable {
+contract Loot is
+    ERC20SnapshotUpgradeable,
+    ERC20PermitUpgradeable,
+    OwnableUpgradeable,
+    UUPSUpgradeable
+{
     // Baal Config
     IBaal public baal;
 
@@ -26,10 +32,14 @@ contract Loot is ERC20SnapshotUpgradeable, ERC20PermitUpgradeable {
     /// @dev initializer should prevent this from being called again
     /// @param name_ Name for ERC20 token trackers
     /// @param symbol_ Symbol for ERC20 token trackers
-    function setUp(string memory name_, string memory symbol_) external initializer {
+    function setUp(string memory name_, string memory symbol_)
+        external
+        initializer
+    {
         baal = IBaal(msg.sender); /*Configure Baal to setup sender*/
         __ERC20_init(name_, symbol_);
         __ERC20Permit_init(name_);
+        __Ownable_init();
     }
 
     /// @notice Allows baal to create a snapshot
@@ -69,4 +79,6 @@ contract Loot is ERC20SnapshotUpgradeable, ERC20PermitUpgradeable {
             "!transferable"
         );
     }
+
+    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
 }

contracts/SharesERC20.sol

@@ -3,14 +3,17 @@ pragma solidity 0.8.7;
 
 import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
 import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
+
 import "./utils/BaalVotes.sol";
 import "./interfaces/IBaal.sol";
 
 // import "hardhat/console.sol";
 
 /// @title Shares
 /// @notice Accounting for Baal non voting shares
-contract Shares is BaalVotes {
+contract Shares is BaalVotes, OwnableUpgradeable, UUPSUpgradeable {
     // Baal Config
     IBaal public baal;
 
@@ -33,6 +36,8 @@ contract Shares is BaalVotes {
         baal = IBaal(msg.sender); /*Configure Baal to setup sender*/
         __ERC20_init(name_, symbol_);
         __ERC20Permit_init(name_);
+        __Ownable_init();
+
     }
 
     /// @notice Baal-only function to mint shares.
@@ -71,4 +76,6 @@ contract Shares is BaalVotes {
             "!transferable"
         );
     }
+
+    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
 }

contracts/mock/BaalLessToken.sol

@@ -0,0 +1,68 @@
+pragma solidity 0.8.7;
+//SPDX-License-Identifier: MIT
+
+import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
+
+import "../utils/BaalVotes.sol";
+import "../interfaces/IBaal.sol";
+
+// import "hardhat/console.sol";
+
+/// @title Shares
+/// @notice Accounting for Baal non voting shares
+contract BaalLessShares is BaalVotes, OwnableUpgradeable, UUPSUpgradeable {
+    // Baal Config
+    IBaal public baal;
+    uint8 public version;
+
+    constructor() {
+        _disableInitializers();
+    }
+
+    /// @notice Configure shares - called by Baal on summon
+    /// @param _version new version
+    function setUp(uint8 _version)
+        external
+        reinitializer(_version)
+    {
+        baal = IBaal(address(0)); /*Configure Baal to setup sender*/
+        version = _version;
+    }
+
+
+    /// @notice owner-only function to mint shares.
+    /// @param recipient Address to receive shares
+    /// @param amount Amount to mint
+    function mint(address recipient, uint256 amount) external onlyOwner {
+        unchecked {
+            if (totalSupply() + amount <= type(uint256).max / 2) {
+                _mint(recipient, amount);
+            }
+        }
+    }
+
+    /// @notice owner-only function to burn shares.
+    /// @param account Address to lose shares
+    /// @param amount Amount to burn
+    function burn(address account, uint256 amount) external onlyOwner {
+        _burn(account, amount);
+    }
+
+    /// @notice new before transfer
+    /// @dev Allows transfers if msg.sender is Baal which enables minting and burning
+    /// @param from The address of the source account.
+    /// @param to The address of the destination account.
+    /// @param amount The number of `shares` tokens to transfer.
+    function _beforeTokenTransfer(
+        address from,
+        address to,
+        uint256 amount
+    ) internal override(BaalVotes) {
+        super._beforeTokenTransfer(from, to, amount);
+    }
+
+    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
+}

contracts/mock/MockBaal.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.7;
 
+import "@openzeppelin/contracts/proxy/Clones.sol";
+
 import "../Baal.sol";
 
 contract MockBaal {

test/BaalSafe.test.ts

@@ -16,6 +16,7 @@ import {
   MultiSend,
   GnosisSafeProxyFactory,
   ModuleProxyFactory,
+  BaalLessShares,
 } from '../src/types'
 
 import {
@@ -82,6 +83,7 @@ const revertMessages = {
   permitNotAuthorized: "!authorized",
   permitExpired: "expired",
   notEnoughGas: "not enough gas",
+  OwnableCallerIsNotTheOwner: "Ownable: caller is not the owner",
 };
 
 const STATES = {
@@ -305,6 +307,7 @@ describe("Baal contract", function () {
   let GnosisSafe: ContractFactory;
   let gnosisSafeSingleton: GnosisSafe;
   let gnosisSafe: GnosisSafe;
+  let gnosisSafe1: GnosisSafe;
 
   let GnosisSafeProxyFactory: ContractFactory;
   let gnosisSafeProxyFactory: GnosisSafeProxyFactory;
@@ -450,6 +453,7 @@ describe("Baal contract", function () {
 
     baal = BaalFactory.attach(addresses.baal) as Baal;
     gnosisSafe = BaalFactory.attach(addresses.safe) as GnosisSafe;
+    gnosisSafe1 = GnosisSafe.attach(addresses.safe) as GnosisSafe;
 
     shamanBaal = baal.connect(shaman); // needed to send txns to baal as the shaman
     applicantBaal = baal.connect(applicant); // needed to send txns to baal as the shaman
@@ -544,6 +548,151 @@ describe("Baal contract", function () {
     //   console.log({ decoded })
     // })
   });
+  describe("token ownership", function () {
+    it("can not transfer ownership when not owner", async function () {
+      expect(await lootToken.owner()).to.equal(baal.address);
+
+      await expect(lootToken.transferOwnership(summoner.address)).to.be.revertedWith(
+        revertMessages.OwnableCallerIsNotTheOwner
+        );
+    });
+    it("can not be upgraded when not owner", async function () {
+      expect(await lootToken.owner()).to.equal(baal.address);
+
+      await expect(lootToken.upgradeTo(sharesToken.address)).to.be.revertedWith(
+        revertMessages.OwnableCallerIsNotTheOwner
+        );
+    });
+    it("can renounce loot token ownership", async function () {
+      expect(await lootToken.owner()).to.equal(baal.address);
+
+
+      const renounceAction = await lootToken.interface.encodeFunctionData(
+        "renounceOwnership"
+      );
+
+      const renounceFromBaal = await baal.interface.encodeFunctionData(
+        "executeAsBaal",
+        [lootToken.address, 0, renounceAction]
+      );
+
+      await expect(submitAndProcessProposal(baal, renounceFromBaal, 1))
+        .to.emit(baal, "ProcessProposal")
+        .withArgs(1, true, false);
+
+        expect(await lootToken.owner()).to.equal(zeroAddress);
+    });
+    it("can renounce shares token ownership", async function () {
+      expect(await sharesToken.owner()).to.equal(baal.address);
+
+      const renounceAction = await sharesToken.interface.encodeFunctionData(
+        "renounceOwnership"
+      );
+
+      const renounceFromBaal = await baal.interface.encodeFunctionData(
+        "executeAsBaal",
+        [sharesToken.address, 0, renounceAction]
+      );
+
+      await expect(submitAndProcessProposal(baal, renounceFromBaal, 1))
+        .to.emit(baal, "ProcessProposal")
+        .withArgs(1, true, false);
+
+        expect(await sharesToken.owner()).to.equal(zeroAddress);
+    });
+    it("can change shares token ownership to avatar", async function () {
+      expect(await sharesToken.owner()).to.equal(baal.address);
+
+      const transferOwnershipAction = await sharesToken.interface.encodeFunctionData(
+        "transferOwnership",
+        [gnosisSafe.address]
+      );
+
+      const transferOwnershipFromBaal = await baal.interface.encodeFunctionData(
+        "executeAsBaal",
+        [sharesToken.address, 0, transferOwnershipAction]
+      );
+
+      await expect(submitAndProcessProposal(baal, transferOwnershipFromBaal, 1))
+        .to.emit(baal, "ProcessProposal")
+        .withArgs(1, true, false);
+
+        expect(await sharesToken.owner()).to.equal(gnosisSafe.address);
+    });
+    it("can change loot token ownership to avatar", async function () {
+      expect(await lootToken.owner()).to.equal(baal.address);
+
+      const transferOwnershipAction = await lootToken.interface.encodeFunctionData(
+        "transferOwnership",
+        [gnosisSafe.address]
+      );
+
+      const transferOwnershipFromBaal = await baal.interface.encodeFunctionData(
+        "executeAsBaal",
+        [lootToken.address, 0, transferOwnershipAction]
+      );
+
+      await expect(submitAndProcessProposal(baal, transferOwnershipFromBaal, 1))
+        .to.emit(baal, "ProcessProposal")
+        .withArgs(1, true, false);
+
+        expect(await lootToken.owner()).to.equal(gnosisSafe.address);
+    });
+
+    it("can eject and upgrade token with eoa", async function () {
+      // upgrade token contracts to remove baal deps
+      // call from safe
+      // remove baal module 
+
+      // owner should be baal
+      expect(await sharesToken.owner()).to.equal(baal.address);
+
+      const transferOwnershipAction = await sharesToken.interface.encodeFunctionData(
+        "transferOwnership",
+        [summoner.address]
+      );
+
+      const transferOwnershipFromBaal = await baal.interface.encodeFunctionData(
+        "executeAsBaal",
+        [sharesToken.address, 0, transferOwnershipAction]
+      );
+
+      // make a proposal to transfer ownership to a eoa
+      await expect(submitAndProcessProposal(baal, transferOwnershipFromBaal, 1))
+        .to.emit(baal, "ProcessProposal")
+        .withArgs(1, true, false);
+
+      expect(await sharesToken.owner()).to.equal(summoner.address);
+
+      let BaalLessSharesFactory: ContractFactory;
+
+      BaalLessSharesFactory = await ethers.getContractFactory("BaalLessShares");
+      const baalLessSharesSingleton = (await BaalLessSharesFactory.deploy()) as BaalLessShares;
+
+      const sharesTokenAsOwnerEoa = await sharesToken.connect(summoner);
+      expect(await baalLessSharesSingleton.version()).to.equal(0);
+      console.log('upgrade');
+
+      await sharesTokenAsOwnerEoa.upgradeToAndCall(
+        baalLessSharesSingleton.address, 
+        baalLessSharesSingleton.interface.encodeFunctionData("setUp", [
+          2
+      ]))
+      // after upgrade token should have same balances
+      // after upgrade token should have a version
+      expect(await sharesToken.balanceOf(summoner.address)).to.equal(100);
+      const newTokenInterface = baalLessSharesSingleton.attach(sharesToken.address);
+      expect(await newTokenInterface.version()).to.equal(2);
+      expect(await newTokenInterface.baal()).to.equal(zeroAddress);
+
+      // new owner should be able to mint
+      await sharesTokenAsOwnerEoa.mint(summoner.address, 100);
+
+      expect(await sharesToken.balanceOf(summoner.address)).to.equal(200);
+
+
+    });
+  });
   describe("shaman actions - permission level 7 (full)", function () {
     it("setAdminConfig", async function () {
       await shamanBaal.setAdminConfig(true, true);