diff --git a/contracts/adapters/KycOnboarding.sol b/contracts/adapters/KycOnboarding.sol index e2aaa3648..073788f12 100644 --- a/contracts/adapters/KycOnboarding.sol +++ b/contracts/adapters/KycOnboarding.sol @@ -80,6 +80,10 @@ contract KycOnboardingContract is mapping(DaoRegistry => mapping(address => uint256)) public totalUnits; + // Tracks the hashes of coupons that were redeemed + // hashCouponMessage(dao, Coupon(kycedMember)) => bool + mapping(bytes32 => bool) public redeemedCoupons; + constructor(address payable weth) { _weth = WETH(weth); _weth20 = IERC20(weth); @@ -245,13 +249,16 @@ contract KycOnboardingContract is !isActiveMember(dao, dao.getCurrentDelegateKey(kycedMember)), "already member" ); + bytes32 couponHash = hashCouponMessage(dao, Coupon(kycedMember)); + require(!redeemedCoupons[couponHash], "already redeemed"); uint256 maxMembers = dao.getConfiguration( _configKey(tokenAddr, MaxMembers) ); require(maxMembers > 0, "token not configured"); require(dao.getNbMembers() < maxMembers, "the DAO is full"); - _checkKycCoupon(dao, kycedMember, tokenAddr, signature); + _checkKycCoupon(dao, kycedMember, tokenAddr, couponHash, signature); + redeemedCoupons[couponHash] = true; OnboardingDetails memory details = _checkData(dao, tokenAddr, amount); totalUnits[dao][tokenAddr] += details.unitsRequested; @@ -376,13 +383,11 @@ contract KycOnboardingContract is DaoRegistry dao, address kycedMember, address tokenAddr, + bytes32 couponHash, bytes memory signature ) internal view { require( - ECDSA.recover( - hashCouponMessage(dao, Coupon(kycedMember)), - signature - ) == + ECDSA.recover(couponHash, signature) == dao.getAddressConfiguration( _configKey(tokenAddr, SignerAddressConfig) ), diff --git a/contracts/adapters/voting/OffchainVoting.sol b/contracts/adapters/voting/OffchainVoting.sol index ba1872c10..1aeecb1f1 100644 --- a/contracts/adapters/voting/OffchainVoting.sol +++ b/contracts/adapters/voting/OffchainVoting.sol @@ -372,10 +372,13 @@ contract OffchainVotingContract is "result weight too low" ); + // check whether the new result changes the outcome if ( vote.gracePeriodStartingTime == 0 || - // check whether the new result changes the outcome - vote.nbNo > vote.nbYes != result.nbNo > result.nbYes + // changed from yes to no or from no to yes + vote.nbNo > vote.nbYes != result.nbNo > result.nbYes || + // changed from tie to yes/no or from yes/no to tie + ((vote.nbNo == vote.nbYes) != (result.nbNo == result.nbYes)) ) { vote.gracePeriodStartingTime = uint64(block.timestamp); } diff --git a/contracts/companion/Reimbursement.sol b/contracts/companion/Reimbursement.sol index 5ce06cdd2..0b766cd92 100644 --- a/contracts/companion/Reimbursement.sol +++ b/contracts/companion/Reimbursement.sol @@ -106,15 +106,14 @@ contract ReimbursementContract is IReimbursement, AdapterGuard, GelatoRelay { return (false, 0); } - if (bank.balanceOf(DaoHelper.GUILD, DaoHelper.ETH_TOKEN) < gasLeft) { + uint256 payback = gasLeft * tx.gasprice; + if (bank.balanceOf(DaoHelper.GUILD, DaoHelper.ETH_TOKEN) < payback) { return (false, 0); } uint256 spendLimitPeriod = dao.getConfiguration(SpendLimitPeriod); uint256 spendLimitEth = dao.getConfiguration(SpendLimitEth); - uint256 payback = gasLeft * tx.gasprice; - if ( //slither-disable-next-line timestamp block.timestamp - _data[address(dao)].rateLimitStart < diff --git a/contracts/extensions/erc1155/ERC1155TokenExtension.sol b/contracts/extensions/erc1155/ERC1155TokenExtension.sol index fc2a98aee..b39d970ee 100644 --- a/contracts/extensions/erc1155/ERC1155TokenExtension.sol +++ b/contracts/extensions/erc1155/ERC1155TokenExtension.sol @@ -378,7 +378,7 @@ contract ERC1155TokenExtension is IExtension, IERC1155Receiver { _saveNft(msg.sender, ids[i], DaoHelper.GUILD, values[i]); } - return this.onERC1155Received.selector; + return this.onERC1155BatchReceived.selector; } /** diff --git a/test/adapters/kyc-onboarding.test.js b/test/adapters/kyc-onboarding.test.js index 83e104650..4f850d4f2 100644 --- a/test/adapters/kyc-onboarding.test.js +++ b/test/adapters/kyc-onboarding.test.js @@ -361,4 +361,68 @@ describe("Adapter - KYC Onboarding", () => { "too much funds" ); }); + + it("should not be possible to rejoin the DAO using a coupon that was already redeemed", async () => { + const applicant = accounts[2]; + + const dao = this.dao; + const bank = this.extensions.bankExt; + const onboarding = this.adapters.kycOnboarding; + const ragequit = this.adapters.ragequit; + + // remaining amount to test sending back to proposer + const ethAmount = unitPrice.mul(toBN(3)).add(remaining); + + const signerUtil = SigUtilSigner(signer.privKey); + + const couponData = { + type: "coupon-kyc", + kycedMember: applicant, + }; + + let jsHash = getMessageERC712Hash( + couponData, + dao.address, + onboarding.address, + 1 + ); + let solHash = await onboarding.hashCouponMessage(dao.address, couponData); + expect(jsHash).equal(solHash); + + const signature = signerUtil( + couponData, + dao.address, + onboarding.address, + 1 + ); + + await onboarding.onboardEth(dao.address, applicant, signature, { + from: applicant, + value: ethAmount, + gasPrice: toBN("0"), + }); + + // test active member status + expect(await isMember(bank, applicant)).equal(true); + + // Ragequit - Burn all the member units and exit the DAO + const memberUnits = await bank.balanceOf(applicant, UNITS); + await ragequit.ragequit(dao.address, memberUnits, toBN(0), [ETH_TOKEN], { + from: applicant, + gasPrice: toBN("0"), + }); + + // test active member status + expect(await isMember(bank, applicant)).equal(false); + + // Attempt to rejoin the DAO using the same KYC coupon + await expectRevert( + onboarding.onboardEth(dao.address, applicant, signature, { + from: applicant, + value: ethAmount, + gasPrice: toBN("0"), + }), + "already redeemed" + ); + }); });