From d053dc36f13a12fa0cca726c5f9f1d51b2f9a961 Mon Sep 17 00:00:00 2001 From: Carl Farterson Date: Wed, 1 Dec 2021 18:09:52 -0800 Subject: [PATCH 1/5] feat(meTokenRegistry): getPendingOwner() --- contracts/interfaces/IMeTokenRegistry.sol | 5 +++++ contracts/registries/MeTokenRegistry.sol | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/contracts/interfaces/IMeTokenRegistry.sol b/contracts/interfaces/IMeTokenRegistry.sol index ebb07503..2726ce5b 100644 --- a/contracts/interfaces/IMeTokenRegistry.sol +++ b/contracts/interfaces/IMeTokenRegistry.sol @@ -110,6 +110,11 @@ interface IMeTokenRegistry { /// @return TODO function getOwnerMeToken(address _owner) external view returns (address); + /// @notice TODO + /// @param _oldOwner TODO + /// @return TODO + function getPendingOwner(address _oldOwner) external view returns (address); + /// @notice TODO /// @param meToken Address of meToken queried /// @return meToken_ details of the meToken diff --git a/contracts/registries/MeTokenRegistry.sol b/contracts/registries/MeTokenRegistry.sol index 13b41563..976aaa0f 100644 --- a/contracts/registries/MeTokenRegistry.sol +++ b/contracts/registries/MeTokenRegistry.sol @@ -345,6 +345,16 @@ contract MeTokenRegistry is Ownable, IMeTokenRegistry { return _owners[_owner]; } + /// @inheritdoc IMeTokenRegistry + function getPendingOwner(address _oldOwner) + external + view + override + returns (address) + { + return _pendingOwners[_oldOwner]; + } + /// @inheritdoc IMeTokenRegistry function getDetails(address _meToken) external From 7ac0e4a496d38f1eb1bb6d00a6e63f2a2255f09e Mon Sep 17 00:00:00 2001 From: Carl Farterson Date: Wed, 1 Dec 2021 18:14:20 -0800 Subject: [PATCH 2/5] test(meTokenRegistry): shell out cancelTransferMeTokenOwnership(), claimMeTokenOwnership() --- test/contracts/registries/MeTokenRegistry.ts | 71 ++++++++++++-------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/test/contracts/registries/MeTokenRegistry.ts b/test/contracts/registries/MeTokenRegistry.ts index 47fe8ad0..19082875 100644 --- a/test/contracts/registries/MeTokenRegistry.ts +++ b/test/contracts/registries/MeTokenRegistry.ts @@ -107,12 +107,13 @@ describe("MeTokenRegistry.sol", () => { toETHNumber(baseY), reserveWeight / MAX_WEIGHT ); + console.log(` calculatedRes:${calculatedRes}`); expect(toETHNumber(await meToken.totalSupply())).to.equal(calculatedRes); }); }); - describe("transferOwnership()", () => { - it("Fails if not owner", async () => { + describe("transferMeTokenOwnership()", () => { + it("Fails if not a meToken owner", async () => { const meTokenAddr = await meTokenRegistry.getOwnerMeToken( account1.address ); @@ -121,11 +122,18 @@ describe("MeTokenRegistry.sol", () => { .connect(account3) .transferMeTokenOwnership(account2.address) ).to.revertedWith("meToken does not exist"); - + }); + it("Fails if recipient already owns a meToken", async () => { await expect( meTokenRegistry.transferMeTokenOwnership(account1.address) ).to.revertedWith("_newOwner already owns a meToken"); }); + it("Fails if _newOwner is address(0)", async () => { + // TODO + }); + it("Successfully queues a recipient to claim ownership", async () => { + // TODO + }); it("Emits TransferOwnership()", async () => { const meTokenAddr = await meTokenRegistry.getOwnerMeToken( account1.address @@ -133,12 +141,37 @@ describe("MeTokenRegistry.sol", () => { const tx = await meTokenRegistry .connect(account1) .transferMeTokenOwnership(account2.address); - const meTokenAddrAfter = await meTokenRegistry.getOwnerMeToken( - account1.address - ); - await expect(tx) - .to.emit(meTokenRegistry, "TransferMeTokenOwnership") - .withArgs(account1.address, account2.address, meTokenAddr); + + /* await expect(tx) + .to.emit(meTokenRegistry, "TransferOwnership") + .withArgs(account1.address, account2.address, meTokenAddr); */ + }); + }); + + describe("cancelTransferMeTokenOwnership()", () => { + it("Fails if owner has never called transferMeTokenOwnership()", async () => { + // TODO + }); + it("Fails if owner does not own a meToken", async () => { + // TODO + }); + it("Succesfully cancels transfer and removes from _pendingOwners", async () => { + // TODO + }); + }); + + describe("claimMeTokenOwnership()", () => { + it("Fails if claimer already owns a meToken", async () => { + // TODO + }); + it("Fails if not claimer not pending owner from oldOwner", async () => { + // TODO + }); + it("Successfully completes transfer, updates meToken struct, and deletes old mappings", async () => { + // TODO + }); + it("Emits ClaimMeTokenOwnership()", async () => { + // TODO }); }); @@ -147,26 +180,8 @@ describe("MeTokenRegistry.sol", () => { expect(await meTokenRegistry.isOwner(ethers.constants.AddressZero)).to.be .false; }); - it("Revert if ownership is not claimed", async () => { - expect(await meTokenRegistry.isOwner(account2.address)).to.be.false; - }); - it("Claim ownership should work", async () => { - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account1.address - ); - const tx = await meTokenRegistry - .connect(account2) - .claimMeTokenOwnership(account1.address); - const meTokenAddrAfter = await meTokenRegistry.getOwnerMeToken( - account2.address - ); - expect(meTokenAddr).to.equal(meTokenAddrAfter); - await expect(tx) - .to.emit(meTokenRegistry, "ClaimMeTokenOwnership") - .withArgs(account1.address, account2.address, meTokenAddr); - }); it("Returns true for a meToken issuer", async () => { - expect(await meTokenRegistry.isOwner(account2.address)).to.be.true; + expect(await meTokenRegistry.isOwner(account1.address)).to.be.true; }); }); describe("balancePool", () => { From 039314f33040965f3fdf159eca8fe6b37848d729 Mon Sep 17 00:00:00 2001 From: Carl Farterson Date: Thu, 2 Dec 2021 11:01:44 -0800 Subject: [PATCH 3/5] test(meTokenRegistry): begin filling in tests --- test/contracts/registries/MeTokenRegistry.ts | 50 +++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/test/contracts/registries/MeTokenRegistry.ts b/test/contracts/registries/MeTokenRegistry.ts index 19082875..04022381 100644 --- a/test/contracts/registries/MeTokenRegistry.ts +++ b/test/contracts/registries/MeTokenRegistry.ts @@ -12,10 +12,12 @@ import { } from "../../utils/helpers"; import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers"; import { hubSetup } from "../../utils/hubSetup"; -import { BigNumber } from "ethers"; +import { BigNumber, ContractTransaction } from "ethers"; import { expect } from "chai"; describe("MeTokenRegistry.sol", () => { + let meTokenAddr: string; + let tx: ContractTransaction; let meTokenRegistry: MeTokenRegistry; let hub: Hub; @@ -60,9 +62,7 @@ describe("MeTokenRegistry.sol", () => { const tx = await meTokenRegistry .connect(account0) .subscribe(name, "CARL", hubId, 0); - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account0.address - ); + meTokenAddr = await meTokenRegistry.getOwnerMeToken(account0.address); /* expect(tx) .to.emit(meTokenRegistry, "Register") .withArgs(meTokenAddr, account0.address, name, symbol, hubId); */ @@ -114,9 +114,7 @@ describe("MeTokenRegistry.sol", () => { describe("transferMeTokenOwnership()", () => { it("Fails if not a meToken owner", async () => { - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account1.address - ); + meTokenAddr = await meTokenRegistry.getOwnerMeToken(account1.address); await expect( meTokenRegistry .connect(account3) @@ -135,16 +133,13 @@ describe("MeTokenRegistry.sol", () => { // TODO }); it("Emits TransferOwnership()", async () => { - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account1.address - ); - const tx = await meTokenRegistry + tx = await meTokenRegistry .connect(account1) .transferMeTokenOwnership(account2.address); - /* await expect(tx) - .to.emit(meTokenRegistry, "TransferOwnership") - .withArgs(account1.address, account2.address, meTokenAddr); */ + await expect(tx) + .to.emit(meTokenRegistry, "TransferMeTokenOwnership") + .withArgs(account1.address, account2.address, meTokenAddr); }); }); @@ -156,6 +151,11 @@ describe("MeTokenRegistry.sol", () => { // TODO }); it("Succesfully cancels transfer and removes from _pendingOwners", async () => { + tx = await meTokenRegistry + .connect(account1) + .cancelTransferMeTokenOwnership(); + }); + it("Emits CancelTransferMeTokenOwnership()", async () => { // TODO }); }); @@ -167,11 +167,22 @@ describe("MeTokenRegistry.sol", () => { it("Fails if not claimer not pending owner from oldOwner", async () => { // TODO }); - it("Successfully completes transfer, updates meToken struct, and deletes old mappings", async () => { - // TODO + it("Successfully completes claim and updates meToken struct, deletes old mappings", async () => { + const meTokenAddr = await meTokenRegistry.getOwnerMeToken( + account1.address + ); + await meTokenRegistry + .connect(account1) + .transferMeTokenOwnership(account2.address); + tx = await meTokenRegistry + .connect(account2) + .claimMeTokenOwnership(account1.address); + // TODO: check meToken struct, mappings }); it("Emits ClaimMeTokenOwnership()", async () => { - // TODO + expect(tx) + .to.emit(meTokenRegistry, "ClaimMeTokenOwnership") + .withArgs(account1.address, account2.address, meTokenAddr); }); }); @@ -180,8 +191,11 @@ describe("MeTokenRegistry.sol", () => { expect(await meTokenRegistry.isOwner(ethers.constants.AddressZero)).to.be .false; }); + it("Returns false for if address not an owner", async () => { + expect(await meTokenRegistry.isOwner(account3.address)).to.be.false; + }); it("Returns true for a meToken issuer", async () => { - expect(await meTokenRegistry.isOwner(account1.address)).to.be.true; + expect(await meTokenRegistry.isOwner(account2.address)).to.be.true; }); }); describe("balancePool", () => { From 40ed3f3d02fd77293e27121d23391a34ee6ea1b5 Mon Sep 17 00:00:00 2001 From: Carl Farterson Date: Thu, 2 Dec 2021 14:34:59 -0800 Subject: [PATCH 4/5] fix(meTokenRegistry): require ordering --- contracts/registries/MeTokenRegistry.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/registries/MeTokenRegistry.sol b/contracts/registries/MeTokenRegistry.sol index 976aaa0f..2c9064a0 100644 --- a/contracts/registries/MeTokenRegistry.sol +++ b/contracts/registries/MeTokenRegistry.sol @@ -289,14 +289,14 @@ contract MeTokenRegistry is Ownable, IMeTokenRegistry { /// @inheritdoc IMeTokenRegistry function cancelTransferMeTokenOwnership() external override { + address _meToken = _owners[msg.sender]; + require(_meToken != address(0), "meToken does not exist"); + require( _pendingOwners[msg.sender] != address(0), "transferMeTokenOwnership() not initiated" ); - address _meToken = _owners[msg.sender]; - require(_meToken != address(0), "meToken does not exist"); - delete _pendingOwners[msg.sender]; emit CancelTransferMeTokenOwnership(msg.sender, _meToken); } From 6ebc4c7dddd70e988950c4898a70d25e20780315 Mon Sep 17 00:00:00 2001 From: Carl Farterson Date: Thu, 2 Dec 2021 15:35:53 -0800 Subject: [PATCH 5/5] test(meTokenRegistry): finish filling tests --- test/contracts/registries/MeTokenRegistry.ts | 110 +++++++++++++------ 1 file changed, 75 insertions(+), 35 deletions(-) diff --git a/test/contracts/registries/MeTokenRegistry.ts b/test/contracts/registries/MeTokenRegistry.ts index 04022381..9f905b1a 100644 --- a/test/contracts/registries/MeTokenRegistry.ts +++ b/test/contracts/registries/MeTokenRegistry.ts @@ -16,7 +16,8 @@ import { BigNumber, ContractTransaction } from "ethers"; import { expect } from "chai"; describe("MeTokenRegistry.sol", () => { - let meTokenAddr: string; + let meTokenAddr0: string; + let meTokenAddr1: string; let tx: ContractTransaction; let meTokenRegistry: MeTokenRegistry; @@ -62,13 +63,13 @@ describe("MeTokenRegistry.sol", () => { const tx = await meTokenRegistry .connect(account0) .subscribe(name, "CARL", hubId, 0); - meTokenAddr = await meTokenRegistry.getOwnerMeToken(account0.address); + meTokenAddr0 = await meTokenRegistry.getOwnerMeToken(account0.address); /* expect(tx) .to.emit(meTokenRegistry, "Register") .withArgs(meTokenAddr, account0.address, name, symbol, hubId); */ // assert token infos - const meToken = await getContractAt("MeToken", meTokenAddr); + const meToken = await getContractAt("MeToken", meTokenAddr0); expect(await meToken.name()).to.equal(name); expect(await meToken.symbol()).to.equal(symbol); expect(await meToken.decimals()).to.equal(18); @@ -96,10 +97,8 @@ describe("MeTokenRegistry.sol", () => { const balVault = await token.balanceOf(hubDetail.vault); expect(balVault).equal(amount); // assert token infos - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account1.address - ); - const meToken = await getContractAt("MeToken", meTokenAddr); + meTokenAddr1 = await meTokenRegistry.getOwnerMeToken(account1.address); + const meToken = await getContractAt("MeToken", meTokenAddr1); // should be greater than 0 const calculatedRes = calculateTokenReturnedFromZero( @@ -114,7 +113,6 @@ describe("MeTokenRegistry.sol", () => { describe("transferMeTokenOwnership()", () => { it("Fails if not a meToken owner", async () => { - meTokenAddr = await meTokenRegistry.getOwnerMeToken(account1.address); await expect( meTokenRegistry .connect(account3) @@ -127,62 +125,103 @@ describe("MeTokenRegistry.sol", () => { ).to.revertedWith("_newOwner already owns a meToken"); }); it("Fails if _newOwner is address(0)", async () => { - // TODO + await expect( + meTokenRegistry.transferMeTokenOwnership(ethers.constants.AddressZero) + ).to.be.revertedWith("Cannot transfer to 0 address"); }); it("Successfully queues a recipient to claim ownership", async () => { - // TODO - }); - it("Emits TransferOwnership()", async () => { + expect(await meTokenRegistry.getPendingOwner(account1.address)).to.equal( + ethers.constants.AddressZero + ); tx = await meTokenRegistry .connect(account1) .transferMeTokenOwnership(account2.address); - - await expect(tx) + expect(await meTokenRegistry.getPendingOwner(account1.address)).to.equal( + account2.address + ); + }); + it("Emits TransferOwnership()", async () => { + expect(tx) .to.emit(meTokenRegistry, "TransferMeTokenOwnership") - .withArgs(account1.address, account2.address, meTokenAddr); + .withArgs(account1.address, account2.address, meTokenAddr1); }); }); describe("cancelTransferMeTokenOwnership()", () => { it("Fails if owner has never called transferMeTokenOwnership()", async () => { - // TODO + await expect( + meTokenRegistry.connect(account0).cancelTransferMeTokenOwnership() + ).to.be.revertedWith("transferMeTokenOwnership() not initiated"); }); it("Fails if owner does not own a meToken", async () => { - // TODO + await expect( + meTokenRegistry.connect(account2).cancelTransferMeTokenOwnership() + ).to.be.revertedWith("meToken does not exist"); }); it("Succesfully cancels transfer and removes from _pendingOwners", async () => { tx = await meTokenRegistry .connect(account1) .cancelTransferMeTokenOwnership(); + expect(await meTokenRegistry.getPendingOwner(account1.address)).to.equal( + ethers.constants.AddressZero + ); }); it("Emits CancelTransferMeTokenOwnership()", async () => { - // TODO + expect(tx) + .to.emit(meTokenRegistry, "CancelTransferMeTokenOwnership") + .withArgs(account1.address, meTokenAddr1); }); }); describe("claimMeTokenOwnership()", () => { it("Fails if claimer already owns a meToken", async () => { - // TODO - }); - it("Fails if not claimer not pending owner from oldOwner", async () => { - // TODO - }); - it("Successfully completes claim and updates meToken struct, deletes old mappings", async () => { - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account1.address - ); + // scenario 1: already owns a meToken, not a pending owner + await expect( + meTokenRegistry + .connect(account0) + .claimMeTokenOwnership(ethers.constants.AddressZero) + ).to.be.revertedWith("Already owns a meToken"); + // Scenario 2: doesn't own a meToken and becomes pending owner for 2 meTokens, + // claims ownership to the first, then tries claiming ownership to the second + await meTokenRegistry + .connect(account0) + .transferMeTokenOwnership(account2.address); await meTokenRegistry .connect(account1) .transferMeTokenOwnership(account2.address); tx = await meTokenRegistry .connect(account2) - .claimMeTokenOwnership(account1.address); - // TODO: check meToken struct, mappings + .claimMeTokenOwnership(account0.address); + await expect( + meTokenRegistry + .connect(account2) + .claimMeTokenOwnership(account1.address) + ).to.be.revertedWith("Already owns a meToken"); + }); + it("Fails if not claimer not pending owner from oldOwner", async () => { + await expect( + meTokenRegistry + .connect(account3) + .claimMeTokenOwnership(account1.address) + ).to.be.revertedWith("!_pendingOwner"); + }); + it("Successfully completes claim and updates meToken struct, deletes old mappings", async () => { + expect(await meTokenRegistry.getOwnerMeToken(account2.address)).to.equal( + meTokenAddr0 + ); + const details = await meTokenRegistry.getDetails(meTokenAddr0); + expect(details.owner).to.equal(account2.address); + expect(await meTokenRegistry.getPendingOwner(account0.address)).to.equal( + ethers.constants.AddressZero + ); + expect(await meTokenRegistry.getOwnerMeToken(account0.address)).to.equal( + ethers.constants.AddressZero + ); }); it("Emits ClaimMeTokenOwnership()", async () => { expect(tx) .to.emit(meTokenRegistry, "ClaimMeTokenOwnership") - .withArgs(account1.address, account2.address, meTokenAddr); + .withArgs(account0.address, account2.address, meTokenAddr0); }); }); @@ -195,16 +234,17 @@ describe("MeTokenRegistry.sol", () => { expect(await meTokenRegistry.isOwner(account3.address)).to.be.false; }); it("Returns true for a meToken issuer", async () => { - expect(await meTokenRegistry.isOwner(account2.address)).to.be.true; + expect(await meTokenRegistry.isOwner(account1.address)).to.be.true; }); }); describe("balancePool", () => { it("Fails if not foundry", async () => { - const meTokenAddr = await meTokenRegistry.getOwnerMeToken( - account1.address - ); await expect( - meTokenRegistry.updateBalancePooled(true, meTokenAddr, account2.address) + meTokenRegistry.updateBalancePooled( + true, + meTokenAddr1, + account2.address + ) ).to.revertedWith("!foundry"); }); it("updateBalancePooled()", async () => {