From 0d1d11d71cbadcb6731af2a40945c63da132f73b Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Wed, 20 Sep 2023 11:16:07 +0200 Subject: [PATCH 1/7] Fix double wrap (#120) A call to the `super._mint` function was causing the wrapping of additional tokens which where not supposed to exist. Calling the `ERC20Upgradable` `_mint` function bypasses the additional mint of NEOK tokens. close #119 --- contracts/GovernanceToken/GovernanceToken.sol | 8 +++++++- test/GovernanceToken.ts | 13 +++++++++++-- test/Integration.ts | 17 +++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/contracts/GovernanceToken/GovernanceToken.sol b/contracts/GovernanceToken/GovernanceToken.sol index 30d6b28..f6dbd15 100644 --- a/contracts/GovernanceToken/GovernanceToken.sol +++ b/contracts/GovernanceToken/GovernanceToken.sol @@ -360,7 +360,7 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { DepositedTokens storage tokens = depositedTokens[from][i - 1]; if (block.timestamp >= tokens.settlementTimestamp) { if (tokens.amount > 0) { - super._mint(from, tokens.amount); + ERC20Upgradeable._mint(from, tokens.amount); tokens.amount = 0; } else { break; @@ -368,4 +368,10 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { } } } + + function _burnExtra() external onlyRole(Roles.OPERATOR_ROLE) { + // From https://escan.live/tx/0x4973105a8215b74f356b503b1dadfaa7044008c4336ac506d91d1cbbeb56410e + uint256 extraTokens = 7818999911120000000000; + tokenExternal.burn(extraTokens); + } } diff --git a/test/GovernanceToken.ts b/test/GovernanceToken.ts index 8b37d0e..6e79026 100644 --- a/test/GovernanceToken.ts +++ b/test/GovernanceToken.ts @@ -101,6 +101,7 @@ describe("GovernanceToken", () => { redemption.afterMint.reset(); daoRoles.hasRole.reset(); shareholderRegistry.isAtLeast.reset(); + neokingdomToken.mint.reset(); }); describe("transfer hooks", async () => { @@ -335,7 +336,7 @@ describe("GovernanceToken", () => { }); }); - describe("processDepositedTokens", async () => { + describe("settleTokens", async () => { describe("when no tokens have been wrapped", async () => { it("should mint nothing", async () => { await governanceToken.settleTokens(contributor.address); @@ -383,7 +384,7 @@ describe("GovernanceToken", () => { expect(result).equal(41); }); - it("should only not mint non cooled tokens", async () => { + it("should not mint non cooled tokens", async () => { await governanceToken.wrap(contributor.address, 42); await governanceToken.settleTokens(contributor.address); @@ -418,6 +419,14 @@ describe("GovernanceToken", () => { expect(balanceAfter).equal(balanceBefore.add(42)); }); + + it("should not mint an equivalent amount of neok tokens to the governance contract", async () => { + await governanceToken.wrap(contributor.address, 42); + await timeTravel(7); + await governanceToken.settleTokens(contributor.address); + + expect(neokingdomToken.mint).to.not.have.been.called; + }); }); }); diff --git a/test/Integration.ts b/test/Integration.ts index 48e6070..659dede 100644 --- a/test/Integration.ts +++ b/test/Integration.ts @@ -1254,6 +1254,13 @@ describe("Integration", async () => { }); it("back and forth NEOK <-> Governance", async () => { + async function _expectWrapped(count: number) { + let wrappedNEOKs = await neokingdomToken.balanceOf( + governanceToken.address + ); + expect(wrappedNEOKs).equal(count); + } + await governanceToken.setSettlementPeriod(3600 * 24 * 7); const share = parseEther("1"); await shareholderRegistry.mint(user1.address, parseEther("1")); @@ -1261,18 +1268,25 @@ describe("Integration", async () => { await shareholderRegistry.setStatus(contributorStatus, user1.address); await shareholderRegistry.setStatus(contributorStatus, user2.address); + await _expectWrapped(0); + // 15 Governance Tokens to user2 await governanceToken.mint(user2.address, 15); + await _expectWrapped(15); + // 5 Governance tokens minted to user1 await governanceToken.mint(user1.address, 5); + await _expectWrapped(20); // user2 withdraws 10 Governance tokens to user1 await internalMarket.connect(user2).makeOffer(10); await timeTravel(7, true); await internalMarket.connect(user2).withdraw(user1.address, 10); + await _expectWrapped(10); // user1 deposit 4 NEOK await internalMarket.connect(user1).deposit(4); + await _expectWrapped(14); // user1 voting power is 5 expect(await voting.getVotingPower(user1.address)).equal(share.add(5)); // user1 offers 3 NEOK @@ -1287,6 +1301,7 @@ describe("Integration", async () => { ); // user1 deposit 3 NEOK await internalMarket.connect(user1).deposit(3); + await _expectWrapped(17); // user1 voting power is 2 expect(await voting.getVotingPower(user1.address)).equal(share.add(2)); // deposit is finalized @@ -1299,6 +1314,8 @@ describe("Integration", async () => { await governanceToken.settleTokens(user1.address); // user1 voting power is 9 expect(await voting.getVotingPower(user1.address)).equal(share.add(9)); + + await _expectWrapped(17); }); it("internal and external token amounts", async () => { From 6ed9759ba2d573bc5e12cdff9fb4e3f2a4ce11f7 Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Wed, 20 Sep 2023 14:42:23 +0200 Subject: [PATCH 2/7] Upgrade (fix double wrap) (#125) --- .openzeppelin/unknown-9001.json | 316 ++++++++++++++++++++++++++++++-- hardhat.config.ts | 2 +- 2 files changed, 299 insertions(+), 19 deletions(-) diff --git a/.openzeppelin/unknown-9001.json b/.openzeppelin/unknown-9001.json index 5344f1a..4f2516d 100644 --- a/.openzeppelin/unknown-9001.json +++ b/.openzeppelin/unknown-9001.json @@ -2472,7 +2472,7 @@ "label": "_roles", "offset": 0, "slot": "51", - "type": "t_contract(DAORoles)13255", + "type": "t_contract(DAORoles)7607", "contract": "HasRole", "src": "contracts/extensions/HasRole.sol:11" }, @@ -2528,7 +2528,7 @@ "label": "_voting", "offset": 0, "slot": "102", - "type": "t_contract(IVoting)12166", + "type": "t_contract(IVoting)7591", "contract": "GovernanceTokenBase", "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:15" }, @@ -2536,7 +2536,7 @@ "label": "_redemptionController", "offset": 0, "slot": "103", - "type": "t_contract(IRedemptionController)8761", + "type": "t_contract(IRedemptionController)7397", "contract": "GovernanceTokenBase", "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:16" }, @@ -2544,7 +2544,7 @@ "label": "tokenExternal", "offset": 0, "slot": "104", - "type": "t_contract(INeokingdomToken)8400", + "type": "t_contract(INeokingdomToken)7364", "contract": "GovernanceTokenBase", "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:17" }, @@ -2568,7 +2568,7 @@ "label": "_accountBalanceSnapshots", "offset": 0, "slot": "107", - "type": "t_mapping(t_address,t_struct(Snapshots)7016_storage)", + "type": "t_mapping(t_address,t_struct(Snapshots)5954_storage)", "contract": "GovernanceTokenSnapshot", "src": "contracts/GovernanceToken/GovernanceTokenSnapshot.sol:21" }, @@ -2576,7 +2576,7 @@ "label": "_totalSupplySnapshots", "offset": 0, "slot": "108", - "type": "t_struct(Snapshots)7016_storage", + "type": "t_struct(Snapshots)5954_storage", "contract": "GovernanceTokenSnapshot", "src": "contracts/GovernanceToken/GovernanceTokenSnapshot.sol:22" }, @@ -2584,7 +2584,7 @@ "label": "_shareholderRegistry", "offset": 0, "slot": "110", - "type": "t_contract(IShareholderRegistry)11142", + "type": "t_contract(IShareholderRegistry)7494", "contract": "GovernanceToken", "src": "contracts/GovernanceToken/GovernanceToken.sol:20" }, @@ -2592,7 +2592,7 @@ "label": "depositedTokens", "offset": 0, "slot": "111", - "type": "t_mapping(t_address,t_array(t_struct(DepositedTokens)6201_storage)dyn_storage)", + "type": "t_mapping(t_address,t_array(t_struct(DepositedTokens)5139_storage)dyn_storage)", "contract": "GovernanceToken", "src": "contracts/GovernanceToken/GovernanceToken.sol:33" }, @@ -2610,7 +2610,7 @@ "label": "address", "numberOfBytes": "20" }, - "t_array(t_struct(DepositedTokens)6201_storage)dyn_storage": { + "t_array(t_struct(DepositedTokens)5139_storage)dyn_storage": { "label": "struct GovernanceToken.DepositedTokens[]", "numberOfBytes": "32" }, @@ -2630,27 +2630,27 @@ "label": "bool", "numberOfBytes": "1" }, - "t_contract(DAORoles)13255": { + "t_contract(DAORoles)7607": { "label": "contract DAORoles", "numberOfBytes": "20" }, - "t_contract(INeokingdomToken)8400": { + "t_contract(INeokingdomToken)7364": { "label": "contract INeokingdomToken", "numberOfBytes": "20" }, - "t_contract(IRedemptionController)8761": { + "t_contract(IRedemptionController)7397": { "label": "contract IRedemptionController", "numberOfBytes": "20" }, - "t_contract(IShareholderRegistry)11142": { + "t_contract(IShareholderRegistry)7494": { "label": "contract IShareholderRegistry", "numberOfBytes": "20" }, - "t_contract(IVoting)12166": { + "t_contract(IVoting)7591": { "label": "contract IVoting", "numberOfBytes": "20" }, - "t_mapping(t_address,t_array(t_struct(DepositedTokens)6201_storage)dyn_storage)": { + "t_mapping(t_address,t_array(t_struct(DepositedTokens)5139_storage)dyn_storage)": { "label": "mapping(address => struct GovernanceToken.DepositedTokens[])", "numberOfBytes": "32" }, @@ -2658,7 +2658,7 @@ "label": "mapping(address => mapping(address => uint256))", "numberOfBytes": "32" }, - "t_mapping(t_address,t_struct(Snapshots)7016_storage)": { + "t_mapping(t_address,t_struct(Snapshots)5954_storage)": { "label": "mapping(address => struct GovernanceTokenSnapshot.Snapshots)", "numberOfBytes": "32" }, @@ -2670,7 +2670,7 @@ "label": "string", "numberOfBytes": "32" }, - "t_struct(DepositedTokens)6201_storage": { + "t_struct(DepositedTokens)5139_storage": { "label": "struct GovernanceToken.DepositedTokens", "members": [ { @@ -2688,7 +2688,7 @@ ], "numberOfBytes": "64" }, - "t_struct(Snapshots)7016_storage": { + "t_struct(Snapshots)5954_storage": { "label": "struct GovernanceTokenSnapshot.Snapshots", "members": [ { @@ -3386,6 +3386,286 @@ } } } + }, + "8f84054f22cda16e676faba845b0dc14336134dee5ff01b1ac755874f1f8ffa0": { + "address": "0xF28797507FA95A2A093dE9484F5c2E56Ad1b0585", + "txHash": "0x8b227b514473a5039593aafb8a1915f80bb43ae7a154136c6f79abf59e5d7f1e", + "layout": { + "solcVersion": "0.8.19", + "storage": [ + { + "label": "_initialized", + "offset": 0, + "slot": "0", + "type": "t_uint8", + "contract": "Initializable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:62", + "retypedFrom": "bool" + }, + { + "label": "_initializing", + "offset": 1, + "slot": "0", + "type": "t_bool", + "contract": "Initializable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:67" + }, + { + "label": "__gap", + "offset": 0, + "slot": "1", + "type": "t_array(t_uint256)50_storage", + "contract": "ContextUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol:36" + }, + { + "label": "_roles", + "offset": 0, + "slot": "51", + "type": "t_contract(DAORoles)7625", + "contract": "HasRole", + "src": "contracts/extensions/HasRole.sol:11" + }, + { + "label": "_balances", + "offset": 0, + "slot": "52", + "type": "t_mapping(t_address,t_uint256)", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:37" + }, + { + "label": "_allowances", + "offset": 0, + "slot": "53", + "type": "t_mapping(t_address,t_mapping(t_address,t_uint256))", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:39" + }, + { + "label": "_totalSupply", + "offset": 0, + "slot": "54", + "type": "t_uint256", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:41" + }, + { + "label": "_name", + "offset": 0, + "slot": "55", + "type": "t_string_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:43" + }, + { + "label": "_symbol", + "offset": 0, + "slot": "56", + "type": "t_string_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:44" + }, + { + "label": "__gap", + "offset": 0, + "slot": "57", + "type": "t_array(t_uint256)45_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:400" + }, + { + "label": "_voting", + "offset": 0, + "slot": "102", + "type": "t_contract(IVoting)7609", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:15" + }, + { + "label": "_redemptionController", + "offset": 0, + "slot": "103", + "type": "t_contract(IRedemptionController)7415", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:16" + }, + { + "label": "tokenExternal", + "offset": 0, + "slot": "104", + "type": "t_contract(INeokingdomToken)7382", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:17" + }, + { + "label": "_vestingBalance", + "offset": 0, + "slot": "105", + "type": "t_mapping(t_address,t_uint256)", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:28" + }, + { + "label": "_currentSnapshotId", + "offset": 0, + "slot": "106", + "type": "t_uint256", + "contract": "Snapshottable", + "src": "contracts/extensions/Snapshottable.sol:11" + }, + { + "label": "_accountBalanceSnapshots", + "offset": 0, + "slot": "107", + "type": "t_mapping(t_address,t_struct(Snapshots)5972_storage)", + "contract": "GovernanceTokenSnapshot", + "src": "contracts/GovernanceToken/GovernanceTokenSnapshot.sol:21" + }, + { + "label": "_totalSupplySnapshots", + "offset": 0, + "slot": "108", + "type": "t_struct(Snapshots)5972_storage", + "contract": "GovernanceTokenSnapshot", + "src": "contracts/GovernanceToken/GovernanceTokenSnapshot.sol:22" + }, + { + "label": "_shareholderRegistry", + "offset": 0, + "slot": "110", + "type": "t_contract(IShareholderRegistry)7512", + "contract": "GovernanceToken", + "src": "contracts/GovernanceToken/GovernanceToken.sol:20" + }, + { + "label": "depositedTokens", + "offset": 0, + "slot": "111", + "type": "t_mapping(t_address,t_array(t_struct(DepositedTokens)5139_storage)dyn_storage)", + "contract": "GovernanceToken", + "src": "contracts/GovernanceToken/GovernanceToken.sol:33" + }, + { + "label": "settlementPeriod", + "offset": 0, + "slot": "112", + "type": "t_uint256", + "contract": "GovernanceToken", + "src": "contracts/GovernanceToken/GovernanceToken.sol:35" + } + ], + "types": { + "t_address": { + "label": "address", + "numberOfBytes": "20" + }, + "t_array(t_struct(DepositedTokens)5139_storage)dyn_storage": { + "label": "struct GovernanceToken.DepositedTokens[]", + "numberOfBytes": "32" + }, + "t_array(t_uint256)45_storage": { + "label": "uint256[45]", + "numberOfBytes": "1440" + }, + "t_array(t_uint256)50_storage": { + "label": "uint256[50]", + "numberOfBytes": "1600" + }, + "t_array(t_uint256)dyn_storage": { + "label": "uint256[]", + "numberOfBytes": "32" + }, + "t_bool": { + "label": "bool", + "numberOfBytes": "1" + }, + "t_contract(DAORoles)7625": { + "label": "contract DAORoles", + "numberOfBytes": "20" + }, + "t_contract(INeokingdomToken)7382": { + "label": "contract INeokingdomToken", + "numberOfBytes": "20" + }, + "t_contract(IRedemptionController)7415": { + "label": "contract IRedemptionController", + "numberOfBytes": "20" + }, + "t_contract(IShareholderRegistry)7512": { + "label": "contract IShareholderRegistry", + "numberOfBytes": "20" + }, + "t_contract(IVoting)7609": { + "label": "contract IVoting", + "numberOfBytes": "20" + }, + "t_mapping(t_address,t_array(t_struct(DepositedTokens)5139_storage)dyn_storage)": { + "label": "mapping(address => struct GovernanceToken.DepositedTokens[])", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_mapping(t_address,t_uint256))": { + "label": "mapping(address => mapping(address => uint256))", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_struct(Snapshots)5972_storage)": { + "label": "mapping(address => struct GovernanceTokenSnapshot.Snapshots)", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_uint256)": { + "label": "mapping(address => uint256)", + "numberOfBytes": "32" + }, + "t_string_storage": { + "label": "string", + "numberOfBytes": "32" + }, + "t_struct(DepositedTokens)5139_storage": { + "label": "struct GovernanceToken.DepositedTokens", + "members": [ + { + "label": "amount", + "type": "t_uint256", + "offset": 0, + "slot": "0" + }, + { + "label": "settlementTimestamp", + "type": "t_uint256", + "offset": 0, + "slot": "1" + } + ], + "numberOfBytes": "64" + }, + "t_struct(Snapshots)5972_storage": { + "label": "struct GovernanceTokenSnapshot.Snapshots", + "members": [ + { + "label": "ids", + "type": "t_array(t_uint256)dyn_storage", + "offset": 0, + "slot": "0" + }, + { + "label": "values", + "type": "t_array(t_uint256)dyn_storage", + "offset": 0, + "slot": "1" + } + ], + "numberOfBytes": "64" + }, + "t_uint256": { + "label": "uint256", + "numberOfBytes": "32" + }, + "t_uint8": { + "label": "uint8", + "numberOfBytes": "1" + } + } + } } } } diff --git a/hardhat.config.ts b/hardhat.config.ts index 4358ba6..ddcb214 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -77,7 +77,7 @@ const config: HardhatUserConfig = { accounts: [TEVMOS_PRIVATE_KEY], }, evmos: { - url: "https://jsonrpc-evmos-ia.cosmosia.notional.ventures/", + url: "https://eth.bd.evmos.org:8545", accounts: [EVMOS_PRIVATE_KEY], }, }, From 4e4c782bf23f03cb2fec5b3c41923a264a1f4eac Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Wed, 20 Sep 2023 15:11:44 +0200 Subject: [PATCH 3/7] Remove cleanup function (#126) close #124 --- .openzeppelin/unknown-9001.json | 280 ++++++++++++++++++ contracts/GovernanceToken/GovernanceToken.sol | 6 - 2 files changed, 280 insertions(+), 6 deletions(-) diff --git a/.openzeppelin/unknown-9001.json b/.openzeppelin/unknown-9001.json index 4f2516d..2e512bf 100644 --- a/.openzeppelin/unknown-9001.json +++ b/.openzeppelin/unknown-9001.json @@ -3666,6 +3666,286 @@ } } } + }, + "246342d7309e3de8809de3d5caa182c103f09298ba3c3d554f7b1920d947b1df": { + "address": "0x0Dd55104F3462f54229672B2027bf74efd732fEB", + "txHash": "0xe961e9a203ed8cd95f96578be128ef5000a6e16dc31cdc233f0b5be21844a766", + "layout": { + "solcVersion": "0.8.19", + "storage": [ + { + "label": "_initialized", + "offset": 0, + "slot": "0", + "type": "t_uint8", + "contract": "Initializable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:62", + "retypedFrom": "bool" + }, + { + "label": "_initializing", + "offset": 1, + "slot": "0", + "type": "t_bool", + "contract": "Initializable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:67" + }, + { + "label": "__gap", + "offset": 0, + "slot": "1", + "type": "t_array(t_uint256)50_storage", + "contract": "ContextUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol:36" + }, + { + "label": "_roles", + "offset": 0, + "slot": "51", + "type": "t_contract(DAORoles)7607", + "contract": "HasRole", + "src": "contracts/extensions/HasRole.sol:11" + }, + { + "label": "_balances", + "offset": 0, + "slot": "52", + "type": "t_mapping(t_address,t_uint256)", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:37" + }, + { + "label": "_allowances", + "offset": 0, + "slot": "53", + "type": "t_mapping(t_address,t_mapping(t_address,t_uint256))", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:39" + }, + { + "label": "_totalSupply", + "offset": 0, + "slot": "54", + "type": "t_uint256", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:41" + }, + { + "label": "_name", + "offset": 0, + "slot": "55", + "type": "t_string_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:43" + }, + { + "label": "_symbol", + "offset": 0, + "slot": "56", + "type": "t_string_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:44" + }, + { + "label": "__gap", + "offset": 0, + "slot": "57", + "type": "t_array(t_uint256)45_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:400" + }, + { + "label": "_voting", + "offset": 0, + "slot": "102", + "type": "t_contract(IVoting)7591", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:15" + }, + { + "label": "_redemptionController", + "offset": 0, + "slot": "103", + "type": "t_contract(IRedemptionController)7397", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:16" + }, + { + "label": "tokenExternal", + "offset": 0, + "slot": "104", + "type": "t_contract(INeokingdomToken)7364", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:17" + }, + { + "label": "_vestingBalance", + "offset": 0, + "slot": "105", + "type": "t_mapping(t_address,t_uint256)", + "contract": "GovernanceTokenBase", + "src": "contracts/GovernanceToken/GovernanceTokenBase.sol:28" + }, + { + "label": "_currentSnapshotId", + "offset": 0, + "slot": "106", + "type": "t_uint256", + "contract": "Snapshottable", + "src": "contracts/extensions/Snapshottable.sol:11" + }, + { + "label": "_accountBalanceSnapshots", + "offset": 0, + "slot": "107", + "type": "t_mapping(t_address,t_struct(Snapshots)5954_storage)", + "contract": "GovernanceTokenSnapshot", + "src": "contracts/GovernanceToken/GovernanceTokenSnapshot.sol:21" + }, + { + "label": "_totalSupplySnapshots", + "offset": 0, + "slot": "108", + "type": "t_struct(Snapshots)5954_storage", + "contract": "GovernanceTokenSnapshot", + "src": "contracts/GovernanceToken/GovernanceTokenSnapshot.sol:22" + }, + { + "label": "_shareholderRegistry", + "offset": 0, + "slot": "110", + "type": "t_contract(IShareholderRegistry)7494", + "contract": "GovernanceToken", + "src": "contracts/GovernanceToken/GovernanceToken.sol:20" + }, + { + "label": "depositedTokens", + "offset": 0, + "slot": "111", + "type": "t_mapping(t_address,t_array(t_struct(DepositedTokens)5139_storage)dyn_storage)", + "contract": "GovernanceToken", + "src": "contracts/GovernanceToken/GovernanceToken.sol:33" + }, + { + "label": "settlementPeriod", + "offset": 0, + "slot": "112", + "type": "t_uint256", + "contract": "GovernanceToken", + "src": "contracts/GovernanceToken/GovernanceToken.sol:35" + } + ], + "types": { + "t_address": { + "label": "address", + "numberOfBytes": "20" + }, + "t_array(t_struct(DepositedTokens)5139_storage)dyn_storage": { + "label": "struct GovernanceToken.DepositedTokens[]", + "numberOfBytes": "32" + }, + "t_array(t_uint256)45_storage": { + "label": "uint256[45]", + "numberOfBytes": "1440" + }, + "t_array(t_uint256)50_storage": { + "label": "uint256[50]", + "numberOfBytes": "1600" + }, + "t_array(t_uint256)dyn_storage": { + "label": "uint256[]", + "numberOfBytes": "32" + }, + "t_bool": { + "label": "bool", + "numberOfBytes": "1" + }, + "t_contract(DAORoles)7607": { + "label": "contract DAORoles", + "numberOfBytes": "20" + }, + "t_contract(INeokingdomToken)7364": { + "label": "contract INeokingdomToken", + "numberOfBytes": "20" + }, + "t_contract(IRedemptionController)7397": { + "label": "contract IRedemptionController", + "numberOfBytes": "20" + }, + "t_contract(IShareholderRegistry)7494": { + "label": "contract IShareholderRegistry", + "numberOfBytes": "20" + }, + "t_contract(IVoting)7591": { + "label": "contract IVoting", + "numberOfBytes": "20" + }, + "t_mapping(t_address,t_array(t_struct(DepositedTokens)5139_storage)dyn_storage)": { + "label": "mapping(address => struct GovernanceToken.DepositedTokens[])", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_mapping(t_address,t_uint256))": { + "label": "mapping(address => mapping(address => uint256))", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_struct(Snapshots)5954_storage)": { + "label": "mapping(address => struct GovernanceTokenSnapshot.Snapshots)", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_uint256)": { + "label": "mapping(address => uint256)", + "numberOfBytes": "32" + }, + "t_string_storage": { + "label": "string", + "numberOfBytes": "32" + }, + "t_struct(DepositedTokens)5139_storage": { + "label": "struct GovernanceToken.DepositedTokens", + "members": [ + { + "label": "amount", + "type": "t_uint256", + "offset": 0, + "slot": "0" + }, + { + "label": "settlementTimestamp", + "type": "t_uint256", + "offset": 0, + "slot": "1" + } + ], + "numberOfBytes": "64" + }, + "t_struct(Snapshots)5954_storage": { + "label": "struct GovernanceTokenSnapshot.Snapshots", + "members": [ + { + "label": "ids", + "type": "t_array(t_uint256)dyn_storage", + "offset": 0, + "slot": "0" + }, + { + "label": "values", + "type": "t_array(t_uint256)dyn_storage", + "offset": 0, + "slot": "1" + } + ], + "numberOfBytes": "64" + }, + "t_uint256": { + "label": "uint256", + "numberOfBytes": "32" + }, + "t_uint8": { + "label": "uint8", + "numberOfBytes": "1" + } + } + } } } } diff --git a/contracts/GovernanceToken/GovernanceToken.sol b/contracts/GovernanceToken/GovernanceToken.sol index f6dbd15..1b421ac 100644 --- a/contracts/GovernanceToken/GovernanceToken.sol +++ b/contracts/GovernanceToken/GovernanceToken.sol @@ -368,10 +368,4 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { } } } - - function _burnExtra() external onlyRole(Roles.OPERATOR_ROLE) { - // From https://escan.live/tx/0x4973105a8215b74f356b503b1dadfaa7044008c4336ac506d91d1cbbeb56410e - uint256 extraTokens = 7818999911120000000000; - tokenExternal.burn(extraTokens); - } } From b5e785bdcbd41315c644982eb4d1c5cdb81d243e Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Fri, 20 Oct 2023 11:12:16 +0200 Subject: [PATCH 4/7] La audit fixes (#116) - [x] close #90 - [x] close #91 - [x] close #92 - [x] close #93 - [x] close #95 - [x] close #96 - [x] close #97 - [x] close #98 - [x] close #119 --------- Co-authored-by: Alberto Granzotto --- contracts/GovernanceToken/GovernanceToken.sol | 32 +++++--- .../GovernanceToken/GovernanceTokenBase.sol | 8 +- .../GovernanceTokenSnapshot.sol | 3 +- .../GovernanceToken/IGovernanceToken.sol | 3 +- contracts/InternalMarket/InternalMarket.sol | 14 ++-- .../InternalMarket/InternalMarketBase.sol | 3 +- .../NeokingdomToken/INeokingdomToken.sol | 3 +- contracts/NeokingdomToken/NeokingdomToken.sol | 2 +- .../IRedemptionController.sol | 3 +- .../RedemptionController.sol | 7 +- .../RedemptionControllerBase.sol | 2 +- .../ResolutionManager/ResolutionManager.sol | 30 ++------ .../ResolutionManagerBase.sol | 5 +- .../IShareholderRegistry.sol | 3 +- .../ShareholderRegistry.sol | 7 +- .../ShareholderRegistryBase.sol | 16 ++-- .../ShareholderRegistrySnapshot.sol | 4 +- contracts/Voting/IVoting.sol | 3 +- contracts/Voting/Voting.sol | 5 +- contracts/Voting/VotingBase.sol | 2 +- contracts/Voting/VotingSnapshot.sol | 2 +- contracts/extensions/DAORoles.sol | 2 +- contracts/extensions/HasRole.sol | 2 +- contracts/extensions/ISnapshot.sol | 2 +- contracts/extensions/Roles.sol | 2 +- contracts/extensions/Snapshottable.sol | 2 +- contracts/mocks/ERC20Mock.sol | 2 +- contracts/mocks/HasRoleMock.sol | 2 +- contracts/mocks/NeokingdomTokenMock.sol | 2 +- contracts/mocks/NeokingdomTokenV2Mock.sol | 2 +- contracts/mocks/NewTelediskoTokenMock.sol | 2 +- contracts/mocks/ResolutionExecutorMock.sol | 2 +- contracts/mocks/ResolutionManagerV2Mock.sol | 2 +- contracts/mocks/ShareholderRegistryMock.sol | 2 +- contracts/mocks/TokenMock.sol | 2 +- contracts/mocks/VotingMock.sol | 2 +- echidna/proxies/InternalMarketProxy.sol | 2 +- echidna/proxies/RedemptionControllerProxy.sol | 2 +- echidna/proxies/TokenMock.sol | 2 +- hardhat.config.ts | 2 +- test/GovernanceToken.ts | 40 ++++++++++ test/GovernanceTokenSnapshot.ts | 4 + test/Integration.ts | 77 +++++++++++++++++++ test/ResolutionManager.ts | 8 ++ test/ShareholderRegistrySnapshot.ts | 8 ++ 45 files changed, 235 insertions(+), 97 deletions(-) diff --git a/contracts/GovernanceToken/GovernanceToken.sol b/contracts/GovernanceToken/GovernanceToken.sol index 1b421ac..33a69ff 100644 --- a/contracts/GovernanceToken/GovernanceToken.sol +++ b/contracts/GovernanceToken/GovernanceToken.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; @@ -46,6 +46,10 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { string memory name, string memory symbol ) public initializer { + require( + address(roles) != address(0), + "GovernanceToken: 0x0 not allowed" + ); _initialize(name, symbol); _setRoles(roles); } @@ -154,6 +158,15 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { uint256 amount ) public virtual onlyRole(Roles.RESOLUTION_ROLE) { _mint(to, amount); + + if ( + _shareholderRegistry.isAtLeast( + _shareholderRegistry.CONTRIBUTOR_STATUS(), + to + ) + ) { + _redemptionController.afterMint(to, amount); + } } /** @@ -292,16 +305,6 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { super._afterTokenTransfer(from, to, amount); _voting.afterTokenTransfer(from, to, amount); - if ( - from == address(0) && - _shareholderRegistry.isAtLeast( - _shareholderRegistry.CONTRIBUTOR_STATUS(), - to - ) - ) { - _redemptionController.afterMint(to, amount); - } - // Invariants require( balanceOf(from) >= _vestingBalance[from], @@ -342,7 +345,12 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { * @param amount Amount of external tokens to wrap. */ function _wrap(address from, uint amount) internal virtual { - tokenExternal.transferFrom(from, address(this), amount); + require( + tokenExternal.transferFrom(from, address(this), amount), + "GovernanceToken: transfer failed" + ); + require(amount > 0, "GovernanceToken: attempt to wrap 0 tokens"); + uint256 settlementTimestamp = block.timestamp + settlementPeriod; depositedTokens[from].push( DepositedTokens(amount, settlementTimestamp) diff --git a/contracts/GovernanceToken/GovernanceTokenBase.sol b/contracts/GovernanceToken/GovernanceTokenBase.sol index 7b06617..fe33fcc 100644 --- a/contracts/GovernanceToken/GovernanceTokenBase.sol +++ b/contracts/GovernanceToken/GovernanceTokenBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import "../RedemptionController/IRedemptionController.sol"; @@ -69,7 +68,10 @@ abstract contract GovernanceTokenBase is ERC20Upgradeable, IGovernanceToken { //} function _unwrap(address from, address to, uint amount) internal virtual { - tokenExternal.transfer(to, amount); + require( + tokenExternal.transfer(to, amount), + "GovernanceToken: transfer failed" + ); super._burn(from, amount); } diff --git a/contracts/GovernanceToken/GovernanceTokenSnapshot.sol b/contracts/GovernanceToken/GovernanceTokenSnapshot.sol index 10f7364..0bab57e 100644 --- a/contracts/GovernanceToken/GovernanceTokenSnapshot.sol +++ b/contracts/GovernanceToken/GovernanceTokenSnapshot.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; import "./IGovernanceToken.sol"; diff --git a/contracts/GovernanceToken/IGovernanceToken.sol b/contracts/GovernanceToken/IGovernanceToken.sol index b405745..e910116 100644 --- a/contracts/GovernanceToken/IGovernanceToken.sol +++ b/contracts/GovernanceToken/IGovernanceToken.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import "../extensions/ISnapshot.sol"; diff --git a/contracts/InternalMarket/InternalMarket.sol b/contracts/InternalMarket/InternalMarket.sol index 29f472a..9a5bb9e 100644 --- a/contracts/InternalMarket/InternalMarket.sol +++ b/contracts/InternalMarket/InternalMarket.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; @@ -23,13 +22,18 @@ contract InternalMarket is Initializable, HasRole, InternalMarketBase { /** * @dev Initializes the contract with the given roles and internal token. * @param roles DAORoles instance containing custom access control roles. - * @param tokenInternal_ Reference to governance token. + * @param governanceToken Reference to governance token. */ function initialize( DAORoles roles, - IGovernanceToken tokenInternal_ + IGovernanceToken governanceToken ) public initializer { - _initialize(tokenInternal_, 7 days); + require( + address(roles) != address(0) && + address(governanceToken) != address(0), + "InternalMarket: 0x0 not allowed" + ); + _initialize(governanceToken, 7 days); _setRoles(roles); } diff --git a/contracts/InternalMarket/InternalMarketBase.sol b/contracts/InternalMarket/InternalMarketBase.sol index 833639d..fec0a3a 100644 --- a/contracts/InternalMarket/InternalMarketBase.sol +++ b/contracts/InternalMarket/InternalMarketBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "../ShareholderRegistry/IShareholderRegistry.sol"; diff --git a/contracts/NeokingdomToken/INeokingdomToken.sol b/contracts/NeokingdomToken/INeokingdomToken.sol index 2cbce67..aeea5a4 100644 --- a/contracts/NeokingdomToken/INeokingdomToken.sol +++ b/contracts/NeokingdomToken/INeokingdomToken.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; diff --git a/contracts/NeokingdomToken/NeokingdomToken.sol b/contracts/NeokingdomToken/NeokingdomToken.sol index f27b126..e2dd53e 100644 --- a/contracts/NeokingdomToken/NeokingdomToken.sol +++ b/contracts/NeokingdomToken/NeokingdomToken.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.9; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol"; diff --git a/contracts/RedemptionController/IRedemptionController.sol b/contracts/RedemptionController/IRedemptionController.sol index b281aad..9b23419 100644 --- a/contracts/RedemptionController/IRedemptionController.sol +++ b/contracts/RedemptionController/IRedemptionController.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; diff --git a/contracts/RedemptionController/RedemptionController.sol b/contracts/RedemptionController/RedemptionController.sol index 834c0d3..155c6cc 100644 --- a/contracts/RedemptionController/RedemptionController.sol +++ b/contracts/RedemptionController/RedemptionController.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "./RedemptionControllerBase.sol"; @@ -36,6 +35,10 @@ contract RedemptionController is * @param roles The addresses of DAORoles for this contract. */ function initialize(DAORoles roles) public initializer { + require( + address(roles) != address(0), + "RedemptionController: 0x0 not allowed" + ); _setRoles(roles); _initialize(); } diff --git a/contracts/RedemptionController/RedemptionControllerBase.sol b/contracts/RedemptionController/RedemptionControllerBase.sol index 69883ea..40e7f03 100644 --- a/contracts/RedemptionController/RedemptionControllerBase.sol +++ b/contracts/RedemptionController/RedemptionControllerBase.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "./IRedemptionController.sol"; diff --git a/contracts/ResolutionManager/ResolutionManager.sol b/contracts/ResolutionManager/ResolutionManager.sol index d0e2b70..0ef7e19 100644 --- a/contracts/ResolutionManager/ResolutionManager.sol +++ b/contracts/ResolutionManager/ResolutionManager.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import { Roles } from "../extensions/Roles.sol"; @@ -26,6 +26,13 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { IGovernanceToken governanceToken, IVoting voting ) public initializer { + require( + address(roles) != address(0) && + address(shareholderRegistry) != address(0) && + address(governanceToken) != address(0) && + address(voting) != address(0), + "ResolutionManager: 0x0 not allowed" + ); _setRoles(roles); _initialize(shareholderRegistry, governanceToken, voting); } @@ -167,13 +174,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { * @param resolutionId The id of the resolution to approve. */ function approveResolution(uint256 resolutionId) external virtual { - require( - _shareholderRegistry.isAtLeast( - _shareholderRegistry.MANAGING_BOARD_STATUS(), - _msgSender() - ), - "Resolution: only managing board can approve" - ); _approveResolution(resolutionId); } @@ -182,13 +182,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { * @param resolutionId The id of the resolution to reject. */ function rejectResolution(uint256 resolutionId) external virtual { - require( - _shareholderRegistry.isAtLeast( - _shareholderRegistry.MANAGING_BOARD_STATUS(), - _msgSender() - ), - "Resolution: only managing board can reject" - ); _rejectResolution(resolutionId); } @@ -209,13 +202,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { address[] memory executionTo, bytes[] memory executionData ) external virtual { - require( - _shareholderRegistry.isAtLeast( - _shareholderRegistry.MANAGING_BOARD_STATUS(), - _msgSender() - ), - "Resolution: only managing board can update" - ); _updateResolution( resolutionId, dataURI, diff --git a/contracts/ResolutionManager/ResolutionManagerBase.sol b/contracts/ResolutionManager/ResolutionManagerBase.sol index 4d0e873..819e139 100644 --- a/contracts/ResolutionManager/ResolutionManagerBase.sol +++ b/contracts/ResolutionManager/ResolutionManagerBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../ShareholderRegistry/IShareholderRegistry.sol"; import "../GovernanceToken/IGovernanceToken.sol"; @@ -267,7 +266,7 @@ abstract contract ResolutionManagerBase { bool isNegative, address[] memory executionTo, bytes[] memory executionData - ) internal virtual onlyPending(resolutionId) { + ) internal virtual onlyPending(resolutionId) exists(resolutionId) { emit ResolutionUpdated(msg.sender, resolutionId); Resolution storage resolution = resolutions[resolutionId]; diff --git a/contracts/ShareholderRegistry/IShareholderRegistry.sol b/contracts/ShareholderRegistry/IShareholderRegistry.sol index 7243337..b9236f1 100644 --- a/contracts/ShareholderRegistry/IShareholderRegistry.sol +++ b/contracts/ShareholderRegistry/IShareholderRegistry.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../extensions/ISnapshot.sol"; diff --git a/contracts/ShareholderRegistry/ShareholderRegistry.sol b/contracts/ShareholderRegistry/ShareholderRegistry.sol index 88d9ada..42265d2 100644 --- a/contracts/ShareholderRegistry/ShareholderRegistry.sol +++ b/contracts/ShareholderRegistry/ShareholderRegistry.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "./ShareholderRegistrySnapshot.sol"; @@ -29,6 +28,10 @@ contract ShareholderRegistry is string memory name, string memory symbol ) public initializer { + require( + address(roles) != address(0), + "ShareholderRegistry: 0x0 not allowed" + ); _initialize(name, symbol); _setRoles(roles); } diff --git a/contracts/ShareholderRegistry/ShareholderRegistryBase.sol b/contracts/ShareholderRegistry/ShareholderRegistryBase.sol index fcf0b88..9df6737 100644 --- a/contracts/ShareholderRegistry/ShareholderRegistryBase.sol +++ b/contracts/ShareholderRegistry/ShareholderRegistryBase.sol @@ -1,11 +1,8 @@ // SPDX-License-Identifier: MIT - -// TODO: update _statuses when account has no shares -// TODO: check who can move shares - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; import "../Voting/IVoting.sol"; contract ShareholderRegistryBase is ERC20Upgradeable { @@ -40,6 +37,10 @@ contract ShareholderRegistryBase is ERC20Upgradeable { } function _setStatus(bytes32 status, address account) internal virtual { + require( + !Address.isContract(account), + "ShareholderRegistry: cannot set status for smart contract" + ); require( status == 0 || isAtLeast(SHAREHOLDER_STATUS, account), "ShareholderRegistry: address has no tokens" @@ -83,7 +84,10 @@ contract ShareholderRegistryBase is ERC20Upgradeable { ) internal view virtual returns (bool) { return balance > 0 && - // shareholder < investor < contributor < managing board + // investor < contributor < managing board + // TODO: shareholder is currently equivalent to investor. + // We need to verify with the lawyer whether we can remove it + // completely from the smart contracts. (status == INVESTOR_STATUS || status == SHAREHOLDER_STATUS || status == accountStatus || diff --git a/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol b/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol index 6940493..84ee78b 100644 --- a/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol +++ b/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol @@ -1,7 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts v4.4.1 (token/ERC20/extensions/ERC20Snapshot.sol) - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "./ShareholderRegistryBase.sol"; import "../extensions/Snapshottable.sol"; diff --git a/contracts/Voting/IVoting.sol b/contracts/Voting/IVoting.sol index b078a5a..cab918a 100644 --- a/contracts/Voting/IVoting.sol +++ b/contracts/Voting/IVoting.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../extensions/ISnapshot.sol"; diff --git a/contracts/Voting/Voting.sol b/contracts/Voting/Voting.sol index d12cd71..000df4f 100644 --- a/contracts/Voting/Voting.sol +++ b/contracts/Voting/Voting.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "../ShareholderRegistry/IShareholderRegistry.sol"; @@ -18,6 +18,7 @@ contract Voting is VotingSnapshot, Initializable, HasRole { * @param roles Instance of a DAORoles contract. */ function initialize(DAORoles roles) public initializer { + require(address(roles) != address(0), "Voting: 0x0 not allowed"); _setRoles(roles); } @@ -95,7 +96,7 @@ contract Voting is VotingSnapshot, Initializable, HasRole { /** * @notice Hook called on every governance token transfer. - * @dev Only the governance token can call this method. + * @dev Called by GovernanceToken and ShareholderRegistry. * @param from The sender's address. * @param to The receiver's address. * @param amount The amount transferred. diff --git a/contracts/Voting/VotingBase.sol b/contracts/Voting/VotingBase.sol index 2cb6667..fc37a67 100644 --- a/contracts/Voting/VotingBase.sol +++ b/contracts/Voting/VotingBase.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import "../ShareholderRegistry/IShareholderRegistry.sol"; diff --git a/contracts/Voting/VotingSnapshot.sol b/contracts/Voting/VotingSnapshot.sol index 2b6b407..35c3ac1 100644 --- a/contracts/Voting/VotingSnapshot.sol +++ b/contracts/Voting/VotingSnapshot.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; import "../extensions/Snapshottable.sol"; diff --git a/contracts/extensions/DAORoles.sol b/contracts/extensions/DAORoles.sol index bd8eece..7d2d9c8 100644 --- a/contracts/extensions/DAORoles.sol +++ b/contracts/extensions/DAORoles.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.16; import "@openzeppelin/contracts/access/AccessControl.sol"; diff --git a/contracts/extensions/HasRole.sol b/contracts/extensions/HasRole.sol index 161fc65..2c760d6 100644 --- a/contracts/extensions/HasRole.sol +++ b/contracts/extensions/HasRole.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol"; import "@openzeppelin/contracts/utils/Strings.sol"; diff --git a/contracts/extensions/ISnapshot.sol b/contracts/extensions/ISnapshot.sol index c98c272..0b639ad 100644 --- a/contracts/extensions/ISnapshot.sol +++ b/contracts/extensions/ISnapshot.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; interface ISnapshot { function snapshot() external returns (uint256); diff --git a/contracts/extensions/Roles.sol b/contracts/extensions/Roles.sol index 257c19e..5ca5b28 100644 --- a/contracts/extensions/Roles.sol +++ b/contracts/extensions/Roles.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; library Roles { bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE"); diff --git a/contracts/extensions/Snapshottable.sol b/contracts/extensions/Snapshottable.sol index 96eca8f..3bd7017 100644 --- a/contracts/extensions/Snapshottable.sol +++ b/contracts/extensions/Snapshottable.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; diff --git a/contracts/mocks/ERC20Mock.sol b/contracts/mocks/ERC20Mock.sol index bf5cd38..61e1b11 100644 --- a/contracts/mocks/ERC20Mock.sol +++ b/contracts/mocks/ERC20Mock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; diff --git a/contracts/mocks/HasRoleMock.sol b/contracts/mocks/HasRoleMock.sol index 6e5bdf7..65912c8 100644 --- a/contracts/mocks/HasRoleMock.sol +++ b/contracts/mocks/HasRoleMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../extensions/HasRole.sol"; diff --git a/contracts/mocks/NeokingdomTokenMock.sol b/contracts/mocks/NeokingdomTokenMock.sol index 8209f4b..71ffc0f 100644 --- a/contracts/mocks/NeokingdomTokenMock.sol +++ b/contracts/mocks/NeokingdomTokenMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; contract GovernanceTokenMock { mapping(address => uint256) mockResult_balanceOfAt; diff --git a/contracts/mocks/NeokingdomTokenV2Mock.sol b/contracts/mocks/NeokingdomTokenV2Mock.sol index 07e1d30..b65d5f3 100644 --- a/contracts/mocks/NeokingdomTokenV2Mock.sol +++ b/contracts/mocks/NeokingdomTokenV2Mock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../GovernanceToken/GovernanceToken.sol"; import "../extensions/Roles.sol"; diff --git a/contracts/mocks/NewTelediskoTokenMock.sol b/contracts/mocks/NewTelediskoTokenMock.sol index 2a7aa38..fe4d3cb 100644 --- a/contracts/mocks/NewTelediskoTokenMock.sol +++ b/contracts/mocks/NewTelediskoTokenMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; diff --git a/contracts/mocks/ResolutionExecutorMock.sol b/contracts/mocks/ResolutionExecutorMock.sol index 6f97a8e..a9dabb8 100644 --- a/contracts/mocks/ResolutionExecutorMock.sol +++ b/contracts/mocks/ResolutionExecutorMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; contract ResolutionExecutorMock { event MockExecutionSimple(uint256 a); diff --git a/contracts/mocks/ResolutionManagerV2Mock.sol b/contracts/mocks/ResolutionManagerV2Mock.sol index 4e0c5c2..4f60444 100644 --- a/contracts/mocks/ResolutionManagerV2Mock.sol +++ b/contracts/mocks/ResolutionManagerV2Mock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../ResolutionManager/ResolutionManager.sol"; import "../extensions/Roles.sol"; diff --git a/contracts/mocks/ShareholderRegistryMock.sol b/contracts/mocks/ShareholderRegistryMock.sol index 17c1609..015eee0 100644 --- a/contracts/mocks/ShareholderRegistryMock.sol +++ b/contracts/mocks/ShareholderRegistryMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../ShareholderRegistry/IShareholderRegistry.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; diff --git a/contracts/mocks/TokenMock.sol b/contracts/mocks/TokenMock.sol index c97b39b..c6f0b2c 100644 --- a/contracts/mocks/TokenMock.sol +++ b/contracts/mocks/TokenMock.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; diff --git a/contracts/mocks/VotingMock.sol b/contracts/mocks/VotingMock.sol index 0cef53c..8634780 100644 --- a/contracts/mocks/VotingMock.sol +++ b/contracts/mocks/VotingMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; contract VotingMock { event AfterTokenTransferCalled(address from, address to, uint256 amount); diff --git a/echidna/proxies/InternalMarketProxy.sol b/echidna/proxies/InternalMarketProxy.sol index 361d61a..cf31924 100644 --- a/echidna/proxies/InternalMarketProxy.sol +++ b/echidna/proxies/InternalMarketProxy.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "./TokenMock.sol"; import "../../contracts/InternalMarket/InternalMarket.sol"; diff --git a/echidna/proxies/RedemptionControllerProxy.sol b/echidna/proxies/RedemptionControllerProxy.sol index 2f1c9d9..213e626 100644 --- a/echidna/proxies/RedemptionControllerProxy.sol +++ b/echidna/proxies/RedemptionControllerProxy.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../../contracts/RedemptionController/RedemptionController.sol"; diff --git a/echidna/proxies/TokenMock.sol b/echidna/proxies/TokenMock.sol index 0ddea86..33cdbb4 100644 --- a/echidna/proxies/TokenMock.sol +++ b/echidna/proxies/TokenMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; diff --git a/hardhat.config.ts b/hardhat.config.ts index ddcb214..88acca2 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -44,7 +44,7 @@ const config: HardhatUserConfig = { solidity: { compilers: [ { - version: "0.8.19", + version: "0.8.16", settings: { optimizer: { enabled: true, diff --git a/test/GovernanceToken.ts b/test/GovernanceToken.ts index 6e79026..52b39fe 100644 --- a/test/GovernanceToken.ts +++ b/test/GovernanceToken.ts @@ -94,6 +94,8 @@ describe("GovernanceToken", () => { shareholderRegistry.isAtLeast .whenCalledWith(contributorStatus, contributor2.address) .returns(true); + neokingdomToken.transfer.returns(true); + neokingdomToken.transferFrom.returns(true); }); afterEach(async () => { @@ -101,6 +103,8 @@ describe("GovernanceToken", () => { redemption.afterMint.reset(); daoRoles.hasRole.reset(); shareholderRegistry.isAtLeast.reset(); + neokingdomToken.transfer.reset(); + neokingdomToken.transferFrom.reset(); neokingdomToken.mint.reset(); }); @@ -253,6 +257,19 @@ describe("GovernanceToken", () => { ); }); + it("should fail when the transfer fails", async () => { + neokingdomToken.transferFrom.returns(false); + await expect(governanceToken.wrap(contributor.address, 1)).revertedWith( + "GovernanceToken: transfer failed" + ); + }); + + it("should fail wrapping 0 tokens", async () => { + await expect( + governanceToken.connect(contributor).wrap(contributor.address, 0) + ).revertedWith("GovernanceToken: attempt to wrap 0 tokens"); + }); + it("should transfer external token to itself", async () => { await governanceToken.wrap(contributor.address, 41); expect(neokingdomToken.transferFrom).calledWith( @@ -420,6 +437,22 @@ describe("GovernanceToken", () => { expect(balanceAfter).equal(balanceBefore.add(42)); }); + it("should not mint an equivalent amount of neok tokens to the governance contract", async () => { + neokingdomToken.mint.reset(); + await governanceToken.wrap(contributor.address, 42); + await timeTravel(7); + await governanceToken.settleTokens(contributor.address); + + expect(neokingdomToken.mint).to.not.have.been.called; + }); + + it("should not call RedemptionController.afterMint", async () => { + await governanceToken.wrap(contributor.address, 42); + await timeTravel(7); + await governanceToken.settleTokens(contributor.address); + expect(redemption.afterMint).not.called; + }); + it("should not mint an equivalent amount of neok tokens to the governance contract", async () => { await governanceToken.wrap(contributor.address, 42); await timeTravel(7); @@ -450,6 +483,13 @@ describe("GovernanceToken", () => { ).revertedWith("ERC20: burn amount exceeds balance"); }); + it("should fail when the transfer fails", async () => { + neokingdomToken.transfer.returns(false); + await expect( + governanceToken.unwrap(contributor.address, contributor.address, 1) + ).revertedWith("GovernanceToken: transfer failed"); + }); + it("should transfer external token to 'to' address", async () => { await governanceToken.mint(contributor.address, 41); await governanceToken.unwrap( diff --git a/test/GovernanceTokenSnapshot.ts b/test/GovernanceTokenSnapshot.ts index 5216609..65ab744 100644 --- a/test/GovernanceTokenSnapshot.ts +++ b/test/GovernanceTokenSnapshot.ts @@ -115,11 +115,15 @@ describe("GovernanceTokenSnapshot", () => { beforeEach(async () => { snapshotId = await network.provider.send("evm_snapshot"); daoRoles.hasRole.returns(true); + neokingdomToken.transfer.returns(true); + neokingdomToken.transferFrom.returns(true); }); afterEach(async () => { await network.provider.send("evm_revert", [snapshotId]); daoRoles.hasRole.reset(); + neokingdomToken.transfer.reset(); + neokingdomToken.transferFrom.reset(); }); describe("snapshot logic", async () => { diff --git a/test/Integration.ts b/test/Integration.ts index 659dede..2960394 100644 --- a/test/Integration.ts +++ b/test/Integration.ts @@ -42,6 +42,7 @@ const INITIAL_USDC = 1000; describe("Integration", async () => { let snapshotId: string; let offerDurationDays: number; + let settlementPeriod = 7; let redemptionStartDays: number; let redemptionWindowDays: number; let redemptionMaxDaysInThePast: number; @@ -1130,6 +1131,82 @@ describe("Integration", async () => { ); }); + describe("least authority audit proof of fix (july 2023)", async () => { + it("issue A: Deposited Tokens Can Be Redeemed", async () => { + await _makeContributor(user1, 10); + await _makeContributor(user2, 10); + + // user2 offers 10 tokens + await internalMarket.connect(user2).makeOffer(e(10)); + await timeTravel(offerDurationDays, true); + + // user2 transfers 10 tokens to user1 + await internalMarket.connect(user2).withdraw(user1.address, e(10)); + + // user1 deposit their tokens + await internalMarket.connect(user1).deposit(e(10)); + await timeTravel(settlementPeriod); + await governanceToken.settleTokens(user1.address); + + // user2 offer all tokens, hoping to redeem all of them... + await internalMarket.connect(user1).makeOffer(e(20)); + await timeTravel(redemptionStartDays, true); + await expect(internalMarket.connect(user1).redeem(e(20))).revertedWith( + "Redemption controller: amount exceeds redeemable balance" + ); + + // ...but they can only redeem those that were minted directly to them + await internalMarket.connect(user1).redeem(e(10)); + expect(await tokenMock.balanceOf(user1.address)).equal( + e(INITIAL_USDC + 10) + ); + }); + + it("Issue B: Unsettled Deposits Can Be Locked", async () => { + await _makeContributor(user1, 10); + await _makeContributor(user2, 0); + + // user1 offers token, no one buys + await internalMarket.connect(user1).makeOffer(e(10)); + await timeTravel(offerDurationDays, true); + + // tokens are transferred to user 2 + await internalMarket.connect(user1).withdraw(user2.address, e(10)); + + // user 2 makes 2 deposits, one of which with 0 tokens, which + // could lock the previous deposit forever + await internalMarket.connect(user2).deposit(e(9)); + await expect(internalMarket.connect(user2).deposit(e(0))).revertedWith( + "GovernanceToken: attempt to wrap 0 tokens" + ); + + await timeTravel(settlementPeriod, true); + await governanceToken.settleTokens(user2.address); + + expect(await governanceToken.balanceOf(user2.address)).equal(e(9)); + }); + + it("Issue C: Missing Modifier Preventing the Update of Non-Existent Resolutions", async () => { + const nonExistingResolutionId = 999; + await expect( + resolutionManager + .connect(managingBoard) + .updateResolution(nonExistingResolutionId, "", 0, false, [], []) + ).revertedWith("Resolution: does not exist"); + }); + + it("issue D: the status of internalMarket or shareholderRegistry can be set to contributor status", async () => { + await expect( + shareholderRegistry.setStatus( + contributorStatus, + internalMarket.address + ) + ).revertedWith( + "ShareholderRegistry: cannot set status for smart contract" + ); + }); + }); + it("redemption edge cases", async () => { await _makeContributor(user1, 10); await _makeContributor(user2, 0); diff --git a/test/ResolutionManager.ts b/test/ResolutionManager.ts index 8ecf6ee..58a08be 100644 --- a/test/ResolutionManager.ts +++ b/test/ResolutionManager.ts @@ -306,6 +306,14 @@ describe("Resolution", async () => { ).revertedWith("Resolution: already rejected"); }); + it("doesn't allow the managing board to update a non-existing resolution", async () => { + await expect( + resolution + .connect(managingBoard) + .updateResolution(42, "updated test", 6, true, [], []) + ).revertedWith("Resolution: does not exist"); + }); + it("doesn't allow anyone else to update a resolution", async () => { shareholderRegistry.isAtLeast .whenCalledWith(managingBoardStatus, user1.address) diff --git a/test/ShareholderRegistrySnapshot.ts b/test/ShareholderRegistrySnapshot.ts index df4a749..3c16a89 100644 --- a/test/ShareholderRegistrySnapshot.ts +++ b/test/ShareholderRegistrySnapshot.ts @@ -104,6 +104,14 @@ describe("Shareholder Registry", () => { ).revertedWith("ShareholderRegistry: address has no tokens"); }); + it("should fail if address is a contract", async () => { + await expect( + registry.setStatus(CONTRIBUTOR_STATUS, registry.address) + ).revertedWith( + "ShareholderRegistry: cannot set status for smart contract" + ); + }); + it("should be callable only by a resolution", async () => { await expect( registry.connect(alice).setStatus(CONTRIBUTOR_STATUS, alice.address) From 129ad4a677f0883b61c37668b092f3a079ff1d6f Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Fri, 20 Oct 2023 13:09:11 +0200 Subject: [PATCH 5/7] Add withdrawn event (#129) `withdraw` now emits an event. close #128 --- contracts/InternalMarket/InternalMarketBase.sol | 3 +++ test/InternalMarket.ts | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/contracts/InternalMarket/InternalMarketBase.sol b/contracts/InternalMarket/InternalMarketBase.sol index fec0a3a..c50d73d 100644 --- a/contracts/InternalMarket/InternalMarketBase.sol +++ b/contracts/InternalMarket/InternalMarketBase.sol @@ -17,6 +17,7 @@ contract InternalMarketBase { ); event OfferMatched(uint128 id, address from, address to, uint256 amount); + event Withdrawn(address from, address to, uint256 amount); struct Offer { uint256 expiredAt; @@ -203,6 +204,8 @@ contract InternalMarketBase { } else { tokenInternal.unwrap(from, to, amount); } + + emit Withdrawn(from, to, amount); } function _burn(address from, uint256 amount) internal virtual { diff --git a/test/InternalMarket.ts b/test/InternalMarket.ts index a22626e..2e9f669 100644 --- a/test/InternalMarket.ts +++ b/test/InternalMarket.ts @@ -768,6 +768,13 @@ describe("InternalMarket", async () => { 11 + 25 ); }); + + it("should emit a Withdrawn event", async () => { + await setEVMTimestamp(ts + WEEK + DAY * 3); + expect(internalMarket.connect(alice).withdraw(bob.address, 11)) + .to.emit(internalMarket, "Withdrawn") + .withArgs(alice.address, bob.address, 11); + }); }); describe("match+withdraw", async () => { From 9b6a96d79d8d0611c5be769befe837d3371a4ff5 Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Fri, 20 Oct 2023 17:42:28 +0200 Subject: [PATCH 6/7] Add audit section --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index 0ecc7fc..45727c7 100644 --- a/README.md +++ b/README.md @@ -54,3 +54,12 @@ npx hardhat test # Deploy to production npx hardhat deploy --network evmos ``` + +# Audits + +- [SolidProof](https://solidproof.io/) + - Tag: https://github.com/NeokingdomDAO/contracts/releases/tag/audit1 + - Report: https://github.com/solidproof/projects/blob/main/2023/NeokingdomDAO/SmartContract_Audit_Solidproof_NeoKingdomDAO.pdf +- [LeastAuthority](https://leastauthority.com) + - Tag: https://github.com/NeokingdomDAO/contracts/releases/tag/audit2 + - Report: https://leastauthority.com/blog/audits/neokingdom-dao-smart-contracts/ From 4d78acfede49a700026b98c9b3304c250556ee88 Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Fri, 27 Oct 2023 16:08:03 +0200 Subject: [PATCH 7/7] Add events to Redemption Controller (#102) - Add event `RedemptionCreated` for each time a redemption is created (after an offer) - Add event `RedemptionUpdate` for each time the user redeems close #101 --- .../RedemptionControllerBase.sol | 55 ++++++-- test/RedemptionController.ts | 126 +++++++++++++++++- 2 files changed, 172 insertions(+), 9 deletions(-) diff --git a/contracts/RedemptionController/RedemptionControllerBase.sol b/contracts/RedemptionController/RedemptionControllerBase.sol index 40e7f03..4d019b3 100644 --- a/contracts/RedemptionController/RedemptionControllerBase.sol +++ b/contracts/RedemptionController/RedemptionControllerBase.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.16; import "./IRedemptionController.sol"; +import "hardhat/console.sol"; // The contract tells how many tokens are redeemable by Contributors abstract contract RedemptionControllerBase is IRedemptionController { @@ -29,6 +30,21 @@ abstract contract RedemptionControllerBase is IRedemptionController { mapping(address => MintBudget[]) internal _mintBudgets; mapping(address => uint256) internal _mintBudgetsStartIndex; + event RedemptionCreated( + address account, + uint256 index, + uint256 amount, + uint256 starts, + uint256 ends + ); + + event RedemptionUpdated( + address from, + uint256 index, + uint256 amountRequested, + uint256 amountRedeemed + ); + function _initialize() internal { redemptionStart = 60 days; redemptionWindow = 10 days; @@ -69,7 +85,7 @@ abstract contract RedemptionControllerBase is IRedemptionController { } function _afterOffer(address account, uint256 amount) internal virtual { - // Find tokens minted ofer the last 3 months of activity, no earlier than 15 months + // Find tokens minted over the last 3 months of activity, no earlier than 15 months if (_mintBudgets[account].length == 0) { return; } @@ -192,20 +208,36 @@ abstract contract RedemptionControllerBase is IRedemptionController { } } - function _afterRedeem(address account, uint256 amount) internal virtual { + function _afterRedeem( + address account, + uint256 amountRequested + ) internal virtual { Redeemable[] storage redeemables = _redeemables[account]; + uint256 amountLeft = amountRequested; - for (uint256 i = 0; i < redeemables.length && amount > 0; i++) { + for (uint256 i = 0; i < redeemables.length && amountLeft > 0; i++) { Redeemable storage redeemable = redeemables[i]; if ( block.timestamp >= redeemable.start && block.timestamp < redeemable.end ) { - if (amount < redeemable.amount) { - redeemable.amount -= amount; - amount = 0; + if (amountLeft < redeemable.amount) { + redeemable.amount -= amountLeft; + emit RedemptionUpdated( + account, + i, + amountRequested, + amountLeft + ); + amountLeft = 0; } else { - amount -= redeemable.amount; + amountLeft -= redeemable.amount; + emit RedemptionUpdated( + account, + i, + amountRequested, + redeemable.amount + ); redeemable.amount = 0; // FIXME: delete object from array? } @@ -213,7 +245,7 @@ abstract contract RedemptionControllerBase is IRedemptionController { } require( - amount == 0, + amountLeft == 0, "Redemption controller: amount exceeds redeemable balance" ); } @@ -232,6 +264,13 @@ abstract contract RedemptionControllerBase is IRedemptionController { redemptionStarts, redemptionStarts + redemptionWindow ); + emit RedemptionCreated( + account, + _redeemables[account].length, + amount, + offerRedeemable.start, + offerRedeemable.end + ); _redeemables[account].push(offerRedeemable); } } diff --git a/test/RedemptionController.ts b/test/RedemptionController.ts index 0c1488a..6957209 100644 --- a/test/RedemptionController.ts +++ b/test/RedemptionController.ts @@ -12,13 +12,20 @@ import { RedemptionController__factory, } from "../typechain"; -import { mineEVMBlock, timeTravel } from "./utils/evm"; +import { + getEVMTimestamp, + mineEVMBlock, + setEVMTimestamp, + timeTravel, +} from "./utils/evm"; import { roles } from "./utils/roles"; chai.use(solidity); chai.use(chaiAsPromised); const { expect } = chai; +const DAY = 3600 * 24; + describe("RedemptionController", () => { let snapshotId: string; @@ -92,6 +99,90 @@ describe("RedemptionController", () => { `AccessControl: account ${account.address.toLowerCase()} is missing role ${TOKEN_MANAGER_ROLE}` ); }); + + describe("when 10 tokens are minted", async () => { + async function _timestamps() { + const nextTimestamp = (await getEVMTimestamp()) + 1; + await setEVMTimestamp(nextTimestamp); + const expectedStart = nextTimestamp + 60 * DAY; + const expectedEnd = nextTimestamp + 70 * DAY; + + return [expectedStart, expectedEnd]; + } + + beforeEach(async () => { + await redemptionController.afterMint(account.address, 10); + }); + + it("emits a RedeemCreated with 10 tokens and expiration data after offer", async () => { + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 10)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 0, 10, expectedStart, expectedEnd); + }); + + it("emits a RedeemCreated with partially matched mints and expiration data after offer", async () => { + await redemptionController.afterOffer(account.address, 7); + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 10)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 1, 3, expectedStart, expectedEnd); + }); + + it("emits a RedeemCreated from multiple mints with incremental ids", async () => { + await redemptionController.afterMint(account.address, 12); + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 22)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 0, 10, expectedStart, expectedEnd) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 1, 12, expectedStart, expectedEnd); + }); + + it("emits a RedeemCreated from expired redemptions", async () => { + await redemptionController.afterOffer(account.address, 10); + await timeTravel(70, true); // redemption expires + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 10)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 1, 10, expectedStart, expectedEnd); + }); + + it("emits a RedeemCreated from partially matched expired redemptions", async () => { + await redemptionController.afterOffer(account.address, 10); + await timeTravel(60, true); // redemption starts + await redemptionController.afterRedeem(account.address, 7); + await timeTravel(10, true); // redemption starts + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 10)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 1, 3, expectedStart, expectedEnd); + }); + + it("emits a RedeemCreated from multiple expired redemptions, partially", async () => { + await redemptionController.afterOffer(account.address, 6); + await redemptionController.afterOffer(account.address, 4); + await timeTravel(70, true); // redemption expires + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 9)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 2, 6, expectedStart, expectedEnd) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 3, 3, expectedStart, expectedEnd); + }); + + it("emits a RedeemCreated from multiple expired redemptions, completely", async () => { + await redemptionController.afterOffer(account.address, 6); + await redemptionController.afterOffer(account.address, 4); + await timeTravel(70, true); // redemption expires + const [expectedStart, expectedEnd] = await _timestamps(); + await expect(redemptionController.afterOffer(account.address, 15)) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 2, 6, expectedStart, expectedEnd) + .to.emit(redemptionController, "RedemptionCreated") + .withArgs(account.address, 3, 4, expectedStart, expectedEnd); + }); + }); }); describe("afterRedeem", async () => { @@ -110,6 +201,39 @@ describe("RedemptionController", () => { "Redemption controller: amount exceeds redeemable balance" ); }); + + describe.only("when 10 tokens are redeemable", async () => { + beforeEach(async () => { + await redemptionController.afterMint(account.address, 10); + await redemptionController.afterOffer(account.address, 10); + await timeTravel(60, true); + }); + + it("emits a RedeemUpdated upon full redemption", async () => { + await expect(redemptionController.afterRedeem(account.address, 10)) + .to.emit(redemptionController, "RedemptionUpdated") + .withArgs(account.address, 0, 10, 10); + }); + + it("emits a RedeemUpdated upon partial redemption", async () => { + await expect(redemptionController.afterRedeem(account.address, 7)) + .to.emit(redemptionController, "RedemptionUpdated") + .withArgs(account.address, 0, 7, 7); + }); + + it("emits a RedeemUpdated upon partial redemption for each redemption covering the amount requested", async () => { + await timeTravel(10, true); // expire old redemption + // make two redemptions + await redemptionController.afterOffer(account.address, 7); + await redemptionController.afterOffer(account.address, 3); + await timeTravel(60, true); // redemption time + await expect(redemptionController.afterRedeem(account.address, 10)) + .to.emit(redemptionController, "RedemptionUpdated") + .withArgs(account.address, 1, 10, 7) + .to.emit(redemptionController, "RedemptionUpdated") + .withArgs(account.address, 2, 10, 3); + }); + }); }); describe("redeemableBalance", async () => {