diff --git a/contracts/Baal.sol b/contracts/Baal.sol index 0fc76ff..b3abfa7 100644 --- a/contracts/Baal.sol +++ b/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; diff --git a/contracts/LootERC20.sol b/contracts/LootERC20.sol index 1bca199..54eedc7 100644 --- a/contracts/LootERC20.sol +++ b/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 {} } diff --git a/contracts/SharesERC20.sol b/contracts/SharesERC20.sol index ed745cc..44c1c66 100644 --- a/contracts/SharesERC20.sol +++ b/contracts/SharesERC20.sol @@ -3,6 +3,9 @@ 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"; @@ -10,7 +13,7 @@ import "./interfaces/IBaal.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 {} } diff --git a/contracts/mock/BaalLessToken.sol b/contracts/mock/BaalLessToken.sol new file mode 100644 index 0000000..f00ed40 --- /dev/null +++ b/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 {} +} diff --git a/contracts/mock/MockBaal.sol b/contracts/mock/MockBaal.sol index a5f2553..a997b87 100644 --- a/contracts/mock/MockBaal.sol +++ b/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 { diff --git a/test/BaalSafe.test.ts b/test/BaalSafe.test.ts index 84d5844..82fec94 100644 --- a/test/BaalSafe.test.ts +++ b/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);