From fd82e10e81e7cfe26ed2f1c3415ff78d18618de2 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 24 Mar 2022 13:39:39 -0400 Subject: [PATCH 01/12] (M-1) Unsafe ERC-20 transfer --- contracts/lib/CurrencyTransferLib.sol | 28 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/contracts/lib/CurrencyTransferLib.sol b/contracts/lib/CurrencyTransferLib.sol index 5881b5ff3..df570af80 100644 --- a/contracts/lib/CurrencyTransferLib.sol +++ b/contracts/lib/CurrencyTransferLib.sol @@ -4,8 +4,12 @@ pragma solidity ^0.8.11; // Helper interfaces import { IWETH } from "../interfaces/IWETH.sol"; import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; +import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; library CurrencyTransferLib { + + using SafeERC20Upgradeable for IERC20Upgradeable; + /// @dev The address interpreted as native token of the chain. address public constant NATIVE_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; @@ -67,11 +71,11 @@ library CurrencyTransferLib { return; } - bool success = _from == address(this) - ? IERC20Upgradeable(_currency).transfer(_to, _amount) - : IERC20Upgradeable(_currency).transferFrom(_from, _to, _amount); - - require(success, "currency transfer failed."); + if(_from == address(this)) { + IERC20Upgradeable(_currency).safeTransfer(_to, _amount); + } else { + IERC20Upgradeable(_currency).safeTransferFrom(_from, _to, _amount); + } } /// @dev Transfer `amount` of ERC20 token from `from` to `to`. @@ -86,12 +90,16 @@ library CurrencyTransferLib { } uint256 balBefore = IERC20Upgradeable(_currency).balanceOf(_to); - bool success = _from == address(this) - ? IERC20Upgradeable(_currency).transfer(_to, _amount) - : IERC20Upgradeable(_currency).transferFrom(_from, _to, _amount); + + if(_from == address(this)) { + IERC20Upgradeable(_currency).safeTransfer(_to, _amount); + } else { + IERC20Upgradeable(_currency).safeTransferFrom(_from, _to, _amount); + } + uint256 balAfter = IERC20Upgradeable(_currency).balanceOf(_to); - require(success && (balAfter == balBefore + _amount), "currency transfer failed."); + require(balAfter == balBefore + _amount, "currency transfer failed."); } /// @dev Transfers `amount` of native token to `to`. @@ -113,7 +121,7 @@ library CurrencyTransferLib { (bool success, ) = to.call{ value: value }(""); if (!success) { IWETH(_nativeTokenWrapper).deposit{ value: value }(); - require(IERC20Upgradeable(_nativeTokenWrapper).transfer(to, value), "transfer failed"); + IERC20Upgradeable(_nativeTokenWrapper).safeTransfer(to, value); } } } From c6986b27c656dd4d003a77bd3a0ca63671d51b64 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 24 Mar 2022 14:21:00 -0400 Subject: [PATCH 02/12] (G-1) Variable packing improvement --- contracts/drop/DropERC721.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/drop/DropERC721.sol b/contracts/drop/DropERC721.sol index a37ff7211..3060f402d 100644 --- a/contracts/drop/DropERC721.sol +++ b/contracts/drop/DropERC721.sol @@ -75,14 +75,14 @@ contract DropERC721 is /// @dev The address that receives all platform fees from all sales. address private platformFeeRecipient; + /// @dev The % of primary sales collected as platform fees. + uint16 private platformFeeBps; + /// @dev The (default) address that receives all royalty value. address private royaltyRecipient; /// @dev The (default) % of a sale to take as royalty (in basis points). - uint128 private royaltyBps; - - /// @dev The % of primary sales collected as platform fees. - uint128 private platformFeeBps; + uint16 private royaltyBps; /// @dev Contract level metadata. string public contractURI; @@ -143,9 +143,9 @@ contract DropERC721 is // Initialize this contract's state. royaltyRecipient = _royaltyRecipient; - royaltyBps = _royaltyBps; + royaltyBps = uint16(_royaltyBps); platformFeeRecipient = _platformFeeRecipient; - platformFeeBps = _platformFeeBps; + platformFeeBps = uint16(_platformFeeBps); primarySaleRecipient = _saleRecipient; contractURI = _contractURI; _owner = _defaultAdmin; @@ -624,7 +624,7 @@ contract DropERC721 is require(_royaltyBps <= MAX_BPS, "exceed royalty bps"); royaltyRecipient = _royaltyRecipient; - royaltyBps = uint128(_royaltyBps); + royaltyBps = uint16(_royaltyBps); emit DefaultRoyalty(_royaltyRecipient, _royaltyBps); } @@ -649,7 +649,7 @@ contract DropERC721 is { require(_platformFeeBps <= MAX_BPS, "bps <= 10000."); - platformFeeBps = uint64(_platformFeeBps); + platformFeeBps = uint16(_platformFeeBps); platformFeeRecipient = _platformFeeRecipient; emit PlatformFeeInfoUpdated(_platformFeeRecipient, _platformFeeBps); From ef37414aedf883e09b800f48e745571bb9355eaa Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 24 Mar 2022 15:30:34 -0400 Subject: [PATCH 03/12] UX suggestion: allowlist qty restriction takes precedence over general qty restriction --- contracts/drop/DropERC721.sol | 25 +++++++++++++++++++------ docs/DropERC721.md | 3 ++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/contracts/drop/DropERC721.sol b/contracts/drop/DropERC721.sol index 3060f402d..2917431d1 100644 --- a/contracts/drop/DropERC721.sol +++ b/contracts/drop/DropERC721.sol @@ -322,10 +322,14 @@ contract DropERC721 is // Get the claim conditions. uint256 activeConditionId = getActiveClaimConditionId(); - // Verify claim validity. If not valid, revert. - verifyClaim(activeConditionId, _msgSender(), _quantity, _currency, _pricePerToken); - - // Verify inclusion in allowlist + /** + * We make allowlist checks (i.e. verifyClaimMerkleProof) before verifying the claim's general + * validity (i.e. verifyClaim) because we give precedence to the check of allow list quantity + * restriction over the check of the general claim condition's quantityLimitPerTransaction + * restriction. + */ + + // Verify inclusion in allowlist. (bool validMerkleProof, uint256 merkleProofIndex) = verifyClaimMerkleProof( activeConditionId, _msgSender(), @@ -334,8 +338,14 @@ contract DropERC721 is _proofMaxQuantityPerTransaction ); + // Verify claim validity. If not valid, revert. + verifyClaim(activeConditionId, _msgSender(), _quantity, _proofMaxQuantityPerTransaction, _currency, _pricePerToken); + if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { - // Mark the claimer's use of their position in the allowlist. + /** + * Mark the claimer's use of their position in the allowlist. A spot in an allowlist + * can be used only once. + */ claimCondition.limitMerkleProofClaim[activeConditionId].set(merkleProofIndex); } @@ -470,6 +480,7 @@ contract DropERC721 is uint256 _conditionId, address _claimer, uint256 _quantity, + uint256 _proofMaxQuantityPerTransaction, address _currency, uint256 _pricePerToken ) public view { @@ -479,8 +490,10 @@ contract DropERC721 is _currency == currentClaimPhase.currency && _pricePerToken == currentClaimPhase.pricePerToken, "invalid currency or price specified." ); + + // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && _quantity <= currentClaimPhase.quantityLimitPerTransaction, + _quantity > 0 && (_proofMaxQuantityPerTransaction > 0 || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( diff --git a/docs/DropERC721.md b/docs/DropERC721.md index 62c6ac35f..e155ef30a 100644 --- a/docs/DropERC721.md +++ b/docs/DropERC721.md @@ -1202,7 +1202,7 @@ function transferFrom(address from, address to, uint256 tokenId) external nonpay ### verifyClaim ```solidity -function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, address _currency, uint256 _pricePerToken) external view +function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, uint256 _proofMaxQuantityPerTransaction, address _currency, uint256 _pricePerToken) external view ``` @@ -1216,6 +1216,7 @@ function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, | _conditionId | uint256 | undefined | _claimer | address | undefined | _quantity | uint256 | undefined +| _proofMaxQuantityPerTransaction | uint256 | undefined | _currency | address | undefined | _pricePerToken | uint256 | undefined From 3fac7afa68184c97be560c320ff61956ac669af1 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Tue, 29 Mar 2022 13:14:23 -0400 Subject: [PATCH 04/12] update verifyClaim interface --- contracts/drop/DropERC721.sol | 9 +++++---- docs/DropERC721.md | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/contracts/drop/DropERC721.sol b/contracts/drop/DropERC721.sol index 2917431d1..f44a90409 100644 --- a/contracts/drop/DropERC721.sol +++ b/contracts/drop/DropERC721.sol @@ -339,7 +339,8 @@ contract DropERC721 is ); // Verify claim validity. If not valid, revert. - verifyClaim(activeConditionId, _msgSender(), _quantity, _proofMaxQuantityPerTransaction, _currency, _pricePerToken); + bool toVerifyMaxQuantityPerTransaction = _proofMaxQuantityPerTransaction == 0; + verifyClaim(activeConditionId, _msgSender(), _quantity, _currency, _pricePerToken, toVerifyMaxQuantityPerTransaction); if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { /** @@ -480,9 +481,9 @@ contract DropERC721 is uint256 _conditionId, address _claimer, uint256 _quantity, - uint256 _proofMaxQuantityPerTransaction, address _currency, - uint256 _pricePerToken + uint256 _pricePerToken, + bool verifyMaxQuantityPerTransaction ) public view { ClaimCondition memory currentClaimPhase = claimCondition.phases[_conditionId]; @@ -493,7 +494,7 @@ contract DropERC721 is // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && (_proofMaxQuantityPerTransaction > 0 || _quantity <= currentClaimPhase.quantityLimitPerTransaction), + _quantity > 0 && (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( diff --git a/docs/DropERC721.md b/docs/DropERC721.md index e155ef30a..1393e0734 100644 --- a/docs/DropERC721.md +++ b/docs/DropERC721.md @@ -1202,7 +1202,7 @@ function transferFrom(address from, address to, uint256 tokenId) external nonpay ### verifyClaim ```solidity -function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, uint256 _proofMaxQuantityPerTransaction, address _currency, uint256 _pricePerToken) external view +function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, address _currency, uint256 _pricePerToken, bool verifyMaxQuantityPerTransaction) external view ``` @@ -1216,9 +1216,9 @@ function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, | _conditionId | uint256 | undefined | _claimer | address | undefined | _quantity | uint256 | undefined -| _proofMaxQuantityPerTransaction | uint256 | undefined | _currency | address | undefined | _pricePerToken | uint256 | undefined +| verifyMaxQuantityPerTransaction | bool | undefined ### verifyClaimMerkleProof From 221a4a321fe72527088b67f10e5ab17c12f206c1 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Tue, 29 Mar 2022 13:36:33 -0400 Subject: [PATCH 05/12] (M-2) Claim condition race condition --- contracts/drop/DropERC721.sol | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contracts/drop/DropERC721.sol b/contracts/drop/DropERC721.sol index f44a90409..550ab1c07 100644 --- a/contracts/drop/DropERC721.sol +++ b/contracts/drop/DropERC721.sol @@ -390,8 +390,11 @@ contract DropERC721 is "startTimestamp must be in ascending order." ); + uint256 supplyClaimedAlready = claimCondition.phases[newStartIndex + i].supplyClaimed; + require(supplyClaimedAlready < _phases[i].maxClaimableSupply, "max supply claimed already"); + claimCondition.phases[newStartIndex + i] = _phases[i]; - claimCondition.phases[newStartIndex + i].supplyClaimed = 0; + claimCondition.phases[newStartIndex + i].supplyClaimed = supplyClaimedAlready; lastConditionStartTimestamp = _phases[i].startTimestamp; } @@ -727,4 +730,4 @@ contract DropERC721 is { return ERC2771ContextUpgradeable._msgData(); } -} +} \ No newline at end of file From 810fa8163956d6d6cce35ceafee71b4ba20b77ea Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 17:02:49 -0400 Subject: [PATCH 06/12] DropERC1155: (G-1) Variable packing improvement --- contracts/drop/DropERC1155.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/drop/DropERC1155.sol b/contracts/drop/DropERC1155.sol index 831268da3..90579d849 100644 --- a/contracts/drop/DropERC1155.sol +++ b/contracts/drop/DropERC1155.sol @@ -73,14 +73,14 @@ contract DropERC1155 is /// @dev The address that receives all platform fees from all sales. address private platformFeeRecipient; + /// @dev The % of primary sales collected as platform fees. + uint16 private platformFeeBps; + /// @dev The recipient of who gets the royalty. address private royaltyRecipient; /// @dev The (default) address that receives all royalty value. - uint128 private royaltyBps; - - /// @dev The % of primary sales collected as platform fees. - uint128 private platformFeeBps; + uint16 private royaltyBps; /// @dev Contract level metadata. string public contractURI; @@ -149,11 +149,11 @@ contract DropERC1155 is name = _name; symbol = _symbol; royaltyRecipient = _royaltyRecipient; - royaltyBps = _royaltyBps; + royaltyBps = uint16(_royaltyBps); platformFeeRecipient = _platformFeeRecipient; primarySaleRecipient = _saleRecipient; contractURI = _contractURI; - platformFeeBps = _platformFeeBps; + platformFeeBps = uint16(_platformFeeBps); _owner = _defaultAdmin; _setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin); @@ -569,7 +569,7 @@ contract DropERC1155 is require(_royaltyBps <= MAX_BPS, "exceed royalty bps"); royaltyRecipient = _royaltyRecipient; - royaltyBps = uint128(_royaltyBps); + royaltyBps = uint16(_royaltyBps); emit DefaultRoyalty(_royaltyRecipient, _royaltyBps); } @@ -594,7 +594,7 @@ contract DropERC1155 is { require(_platformFeeBps <= MAX_BPS, "bps <= 10000."); - platformFeeBps = uint64(_platformFeeBps); + platformFeeBps = uint16(_platformFeeBps); platformFeeRecipient = _platformFeeRecipient; emit PlatformFeeInfoUpdated(_platformFeeRecipient, _platformFeeBps); From 956f4821cdf37a9e7df2c431925b8969aee02ab5 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 17:20:21 -0400 Subject: [PATCH 07/12] DropERC1155: UX suggestion: allowlist qty restriction takes precedence over general qty restriction --- contracts/drop/DropERC1155.sol | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/contracts/drop/DropERC1155.sol b/contracts/drop/DropERC1155.sol index 90579d849..17d82c176 100644 --- a/contracts/drop/DropERC1155.sol +++ b/contracts/drop/DropERC1155.sol @@ -257,10 +257,14 @@ contract DropERC1155 is // Get the active claim condition index. uint256 activeConditionId = getActiveClaimConditionId(_tokenId); - // Verify claim validity. If not valid, revert. - verifyClaim(activeConditionId, _msgSender(), _tokenId, _quantity, _currency, _pricePerToken); - - // Verify inclusion in allowlist + /** + * We make allowlist checks (i.e. verifyClaimMerkleProof) before verifying the claim's general + * validity (i.e. verifyClaim) because we give precedence to the check of allow list quantity + * restriction over the check of the general claim condition's quantityLimitPerTransaction + * restriction. + */ + + // Verify inclusion in allowlist. (bool validMerkleProof, uint256 merkleProofIndex) = verifyClaimMerkleProof( activeConditionId, _msgSender(), @@ -270,8 +274,15 @@ contract DropERC1155 is _proofMaxQuantityPerTransaction ); + // Verify claim validity. If not valid, revert. + bool toVerifyMaxQuantityPerTransaction = _proofMaxQuantityPerTransaction == 0; + verifyClaim(activeConditionId, _msgSender(), _tokenId, _quantity, _currency, _pricePerToken, toVerifyMaxQuantityPerTransaction); + if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { - // Mark the claimer's use of their position in the allowlist. + /** + * Mark the claimer's use of their position in the allowlist. A spot in an allowlist + * can be used only once. + */ claimCondition[_tokenId].limitMerkleProofClaim[activeConditionId].set(merkleProofIndex); } @@ -402,7 +413,8 @@ contract DropERC1155 is uint256 _tokenId, uint256 _quantity, address _currency, - uint256 _pricePerToken + uint256 _pricePerToken, + bool verifyMaxQuantityPerTransaction ) public view { ClaimCondition memory currentClaimPhase = claimCondition[_tokenId].phases[_conditionId]; @@ -410,8 +422,9 @@ contract DropERC1155 is _currency == currentClaimPhase.currency && _pricePerToken == currentClaimPhase.pricePerToken, "invalid currency or price specified." ); + // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && _quantity <= currentClaimPhase.quantityLimitPerTransaction, + _quantity > 0 && (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( From bae4be77a6a1dc69874951ff25c0a47540c433e9 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 17:22:15 -0400 Subject: [PATCH 08/12] DropERC1155: (M-2) Claim condition race condition --- contracts/drop/DropERC1155.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/drop/DropERC1155.sol b/contracts/drop/DropERC1155.sol index 17d82c176..28d6e57e8 100644 --- a/contracts/drop/DropERC1155.sol +++ b/contracts/drop/DropERC1155.sol @@ -328,8 +328,11 @@ contract DropERC1155 is "startTimestamp must be in ascending order." ); + uint256 supplyClaimedAlready = condition.phases[newStartIndex + i].supplyClaimed; + require(supplyClaimedAlready < _phases[i].maxClaimableSupply, "max supply claimed already"); + condition.phases[newStartIndex + i] = _phases[i]; - condition.phases[newStartIndex + i].supplyClaimed = 0; + condition.phases[newStartIndex + i].supplyClaimed = supplyClaimedAlready; lastConditionStartTimestamp = _phases[i].startTimestamp; } From 025145b7bacb834347dd5777777e1b55bd89bbc1 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 17:26:04 -0400 Subject: [PATCH 09/12] DropERC20: UX suggestion: allowlist qty restriction takes precedence over general qty restriction --- contracts/drop/DropERC20.sol | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/contracts/drop/DropERC20.sol b/contracts/drop/DropERC20.sol index 394236546..6e9aed69c 100644 --- a/contracts/drop/DropERC20.sol +++ b/contracts/drop/DropERC20.sol @@ -183,7 +183,8 @@ contract DropERC20 is address _claimer, uint256 _quantity, address _currency, - uint256 _pricePerToken + uint256 _pricePerToken, + bool verifyMaxQuantityPerTransaction ) public view { ClaimCondition memory currentClaimPhase = claimCondition.phases[_conditionId]; @@ -191,8 +192,9 @@ contract DropERC20 is _currency == currentClaimPhase.currency && _pricePerToken == currentClaimPhase.pricePerToken, "invalid currency or price specified." ); + // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && _quantity <= currentClaimPhase.quantityLimitPerTransaction, + _quantity > 0 && (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( @@ -247,9 +249,14 @@ contract DropERC20 is // Get the claim conditions. uint256 activeConditionId = getActiveClaimConditionId(); - // Verify claim validity. If not valid, revert. - verifyClaim(activeConditionId, _msgSender(), _quantity, _currency, _pricePerToken); - + /** + * We make allowlist checks (i.e. verifyClaimMerkleProof) before verifying the claim's general + * validity (i.e. verifyClaim) because we give precedence to the check of allow list quantity + * restriction over the check of the general claim condition's quantityLimitPerTransaction + * restriction. + */ + + // Verify inclusion in allowlist. (bool validMerkleProof, uint256 merkleProofIndex) = verifyClaimMerkleProof( activeConditionId, _msgSender(), @@ -258,10 +265,15 @@ contract DropERC20 is _proofMaxQuantityPerTransaction ); - // if the current claim condition and has a merkle root and the provided proof is valid - // if validMerkleProof is false, it means that claim condition does not have a merkle root - // if invalid proofs are provided, the verifyClaimMerkleProof would revert. + // Verify claim validity. If not valid, revert. + bool toVerifyMaxQuantityPerTransaction = _proofMaxQuantityPerTransaction == 0; + verifyClaim(activeConditionId, _msgSender(), _quantity, _currency, _pricePerToken, toVerifyMaxQuantityPerTransaction); + if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { + /** + * Mark the claimer's use of their position in the allowlist. A spot in an allowlist + * can be used only once. + */ claimCondition.limitMerkleProofClaim[activeConditionId].set(merkleProofIndex); } From fd82346cb92ad42e1cf71785aa38878c033ad6da Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 17:27:51 -0400 Subject: [PATCH 10/12] DropERC20: (M-2) Claim condition race condition --- contracts/drop/DropERC20.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/drop/DropERC20.sol b/contracts/drop/DropERC20.sol index 6e9aed69c..115761671 100644 --- a/contracts/drop/DropERC20.sol +++ b/contracts/drop/DropERC20.sol @@ -309,8 +309,11 @@ contract DropERC20 is "startTimestamp must be in ascending order." ); + uint256 supplyClaimedAlready = claimCondition.phases[newStartIndex + i].supplyClaimed; + require(supplyClaimedAlready < _phases[i].maxClaimableSupply, "max supply claimed already"); + claimCondition.phases[newStartIndex + i] = _phases[i]; - claimCondition.phases[newStartIndex + i].supplyClaimed = 0; + claimCondition.phases[newStartIndex + i].supplyClaimed = supplyClaimedAlready; lastConditionStartTimestamp = _phases[i].startTimestamp; } From 640a92c001c21b04d67a5c63f4883cf5c4eeeae7 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 18:01:42 -0400 Subject: [PATCH 11/12] run prettier + docs gen --- contracts/drop/DropERC1155.sol | 15 ++++++++++++--- contracts/drop/DropERC20.sol | 14 +++++++++++--- contracts/drop/DropERC721.sol | 16 ++++++++++++---- contracts/lib/CurrencyTransferLib.sol | 7 +++---- docs/DropERC1155.md | 3 ++- docs/DropERC20.md | 3 ++- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/contracts/drop/DropERC1155.sol b/contracts/drop/DropERC1155.sol index 28d6e57e8..ac1a9c3c5 100644 --- a/contracts/drop/DropERC1155.sol +++ b/contracts/drop/DropERC1155.sol @@ -263,7 +263,7 @@ contract DropERC1155 is * restriction over the check of the general claim condition's quantityLimitPerTransaction * restriction. */ - + // Verify inclusion in allowlist. (bool validMerkleProof, uint256 merkleProofIndex) = verifyClaimMerkleProof( activeConditionId, @@ -276,7 +276,15 @@ contract DropERC1155 is // Verify claim validity. If not valid, revert. bool toVerifyMaxQuantityPerTransaction = _proofMaxQuantityPerTransaction == 0; - verifyClaim(activeConditionId, _msgSender(), _tokenId, _quantity, _currency, _pricePerToken, toVerifyMaxQuantityPerTransaction); + verifyClaim( + activeConditionId, + _msgSender(), + _tokenId, + _quantity, + _currency, + _pricePerToken, + toVerifyMaxQuantityPerTransaction + ); if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { /** @@ -427,7 +435,8 @@ contract DropERC1155 is ); // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), + _quantity > 0 && + (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( diff --git a/contracts/drop/DropERC20.sol b/contracts/drop/DropERC20.sol index 115761671..60a4103a7 100644 --- a/contracts/drop/DropERC20.sol +++ b/contracts/drop/DropERC20.sol @@ -194,7 +194,8 @@ contract DropERC20 is ); // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), + _quantity > 0 && + (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( @@ -255,7 +256,7 @@ contract DropERC20 is * restriction over the check of the general claim condition's quantityLimitPerTransaction * restriction. */ - + // Verify inclusion in allowlist. (bool validMerkleProof, uint256 merkleProofIndex) = verifyClaimMerkleProof( activeConditionId, @@ -267,7 +268,14 @@ contract DropERC20 is // Verify claim validity. If not valid, revert. bool toVerifyMaxQuantityPerTransaction = _proofMaxQuantityPerTransaction == 0; - verifyClaim(activeConditionId, _msgSender(), _quantity, _currency, _pricePerToken, toVerifyMaxQuantityPerTransaction); + verifyClaim( + activeConditionId, + _msgSender(), + _quantity, + _currency, + _pricePerToken, + toVerifyMaxQuantityPerTransaction + ); if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { /** diff --git a/contracts/drop/DropERC721.sol b/contracts/drop/DropERC721.sol index 550ab1c07..6a6c8513b 100644 --- a/contracts/drop/DropERC721.sol +++ b/contracts/drop/DropERC721.sol @@ -328,7 +328,7 @@ contract DropERC721 is * restriction over the check of the general claim condition's quantityLimitPerTransaction * restriction. */ - + // Verify inclusion in allowlist. (bool validMerkleProof, uint256 merkleProofIndex) = verifyClaimMerkleProof( activeConditionId, @@ -340,7 +340,14 @@ contract DropERC721 is // Verify claim validity. If not valid, revert. bool toVerifyMaxQuantityPerTransaction = _proofMaxQuantityPerTransaction == 0; - verifyClaim(activeConditionId, _msgSender(), _quantity, _currency, _pricePerToken, toVerifyMaxQuantityPerTransaction); + verifyClaim( + activeConditionId, + _msgSender(), + _quantity, + _currency, + _pricePerToken, + toVerifyMaxQuantityPerTransaction + ); if (validMerkleProof && _proofMaxQuantityPerTransaction > 0) { /** @@ -497,7 +504,8 @@ contract DropERC721 is // If we're checking for an allowlist quantity restriction, ignore the general quantity restriction. require( - _quantity > 0 && (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), + _quantity > 0 && + (!verifyMaxQuantityPerTransaction || _quantity <= currentClaimPhase.quantityLimitPerTransaction), "invalid quantity claimed." ); require( @@ -730,4 +738,4 @@ contract DropERC721 is { return ERC2771ContextUpgradeable._msgData(); } -} \ No newline at end of file +} diff --git a/contracts/lib/CurrencyTransferLib.sol b/contracts/lib/CurrencyTransferLib.sol index df570af80..1701e4a1b 100644 --- a/contracts/lib/CurrencyTransferLib.sol +++ b/contracts/lib/CurrencyTransferLib.sol @@ -7,7 +7,6 @@ import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; library CurrencyTransferLib { - using SafeERC20Upgradeable for IERC20Upgradeable; /// @dev The address interpreted as native token of the chain. @@ -71,7 +70,7 @@ library CurrencyTransferLib { return; } - if(_from == address(this)) { + if (_from == address(this)) { IERC20Upgradeable(_currency).safeTransfer(_to, _amount); } else { IERC20Upgradeable(_currency).safeTransferFrom(_from, _to, _amount); @@ -90,8 +89,8 @@ library CurrencyTransferLib { } uint256 balBefore = IERC20Upgradeable(_currency).balanceOf(_to); - - if(_from == address(this)) { + + if (_from == address(this)) { IERC20Upgradeable(_currency).safeTransfer(_to, _amount); } else { IERC20Upgradeable(_currency).safeTransferFrom(_from, _to, _amount); diff --git a/docs/DropERC1155.md b/docs/DropERC1155.md index da626226c..16978ff22 100644 --- a/docs/DropERC1155.md +++ b/docs/DropERC1155.md @@ -1072,7 +1072,7 @@ function uri(uint256 _tokenId) external view returns (string _tokenURI) ### verifyClaim ```solidity -function verifyClaim(uint256 _conditionId, address _claimer, uint256 _tokenId, uint256 _quantity, address _currency, uint256 _pricePerToken) external view +function verifyClaim(uint256 _conditionId, address _claimer, uint256 _tokenId, uint256 _quantity, address _currency, uint256 _pricePerToken, bool verifyMaxQuantityPerTransaction) external view ``` @@ -1089,6 +1089,7 @@ function verifyClaim(uint256 _conditionId, address _claimer, uint256 _tokenId, u | _quantity | uint256 | undefined | _currency | address | undefined | _pricePerToken | uint256 | undefined +| verifyMaxQuantityPerTransaction | bool | undefined ### verifyClaimMerkleProof diff --git a/docs/DropERC20.md b/docs/DropERC20.md index 1361450e3..c6797c2b0 100644 --- a/docs/DropERC20.md +++ b/docs/DropERC20.md @@ -1086,7 +1086,7 @@ function transferFrom(address from, address to, uint256 amount) external nonpaya ### verifyClaim ```solidity -function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, address _currency, uint256 _pricePerToken) external view +function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, address _currency, uint256 _pricePerToken, bool verifyMaxQuantityPerTransaction) external view ``` @@ -1102,6 +1102,7 @@ function verifyClaim(uint256 _conditionId, address _claimer, uint256 _quantity, | _quantity | uint256 | undefined | _currency | address | undefined | _pricePerToken | uint256 | undefined +| verifyMaxQuantityPerTransaction | bool | undefined ### verifyClaimMerkleProof From 546274524c726df2d2ab7b39d6347ff98e1b24c8 Mon Sep 17 00:00:00 2001 From: Krishang Nadgauda Date: Thu, 31 Mar 2022 18:53:56 -0400 Subject: [PATCH 12/12] fix/update DropERC721 tests --- src/test/drop/DropERC721.t.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/drop/DropERC721.t.sol b/src/test/drop/DropERC721.t.sol index 957800495..e650a4b3f 100644 --- a/src/test/drop/DropERC721.t.sol +++ b/src/test/drop/DropERC721.t.sol @@ -22,7 +22,9 @@ contract BaseDropERC721Test is BaseTest { DropERC721.ClaimCondition[] memory conditions = new DropERC721.ClaimCondition[](2); conditions[0].startTimestamp = 0; + conditions[0].maxClaimableSupply = 10; conditions[1].startTimestamp = 1; + conditions[1].maxClaimableSupply = 10; drop.setClaimConditions(conditions, false); (currentStartId, count) = drop.claimCondition();