From 888bc15d68e492199402bd688735319405107ed2 Mon Sep 17 00:00:00 2001 From: Yash Date: Wed, 1 Feb 2023 05:22:33 +0530 Subject: [PATCH 01/18] add stateless airdrop functions --- contracts/airdrop/AirdropERC1155.sol | 20 ++++++++--------- contracts/airdrop/AirdropERC20.sol | 27 +++++++++++++++++++++++ contracts/airdrop/AirdropERC721.sol | 33 +++++++++++++++++++++++++++- lib/forge-std | 2 +- 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index 616a82c38..e25b02643 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -112,21 +112,21 @@ contract AirdropERC1155 is * @dev The token-owner should approve target tokens to Airdrop contract, * which acts as operator for the tokens. * - * @param _tokenAddress Contract address of ERC1155 tokens to air-drop. - * @param _tokenOwner Address from which to transfer tokens. * @param _contents List containing recipient, tokenId and amounts to airdrop. */ - function airdrop( - address _tokenAddress, - address _tokenOwner, - AirdropContent[] calldata _contents - ) external nonReentrant onlyOwner { + function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyOwner { uint256 len = _contents.length; - IERC1155 token = IERC1155(_tokenAddress); - for (uint256 i = 0; i < len; i++) { - token.safeTransferFrom(_tokenOwner, _contents[i].recipient, _contents[i].tokenId, _contents[i].amount, ""); + try + IERC1155(_contents[i].tokenAddress).safeTransferFrom( + _contents[i].tokenOwner, + _contents[i].recipient, + _contents[i].tokenId, + _contents[i].amount, + "" + ) + {} catch {} } } diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 9ba1d5f08..bbb08d100 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -114,6 +114,33 @@ contract AirdropERC20 is } } + /** + * @notice Lets contract-owner send ERC20 tokens to a list of addresses. + * @dev The token-owner should approve target tokens to Airdrop contract, + * which acts as operator for the tokens. + * + * @param _contents List containing recipient, tokenId and amounts to airdrop. + */ + function airdrop(AirdropContent[] calldata _contents) external payable nonReentrant onlyOwner { + uint256 len = _contents.length; + uint256 nativeTokenAmount; + + for (uint256 i = 0; i < len; i++) { + CurrencyTransferLib.transferCurrency( + _contents[i].tokenAddress, + _contents[i].tokenOwner, + _contents[i].recipient, + _contents[i].amount + ); + + if (_contents[i].tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { + nativeTokenAmount += _contents[i].amount; + } + } + + require(nativeTokenAmount == msg.value, "Incorrect native token amount"); + } + /*/////////////////////////////////////////////////////////////// Airdrop view logic //////////////////////////////////////////////////////////////*/ diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 4780104ed..aadd36ba0 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -32,6 +32,7 @@ contract AirdropERC721 is uint256 public payeeCount; uint256 public processedCount; + uint256 public failedCount; uint256[] private indicesOfFailed; @@ -87,6 +88,7 @@ contract AirdropERC721 is function airdrop(uint256 paymentsToProcess) external nonReentrant { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; + uint256 failed; require(countOfProcessed + paymentsToProcess <= totalPayees, "invalid no. of payments"); @@ -95,10 +97,39 @@ contract AirdropERC721 is for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { AirdropContent memory content = airdropContent[i]; - IERC721(content.tokenAddress).safeTransferFrom(content.tokenOwner, content.recipient, content.tokenId); + try + IERC721(content.tokenAddress).safeTransferFrom(content.tokenOwner, content.recipient, content.tokenId) + {} catch { + indicesOfFailed[failed++] = i; + } emit AirdropPayment(content.recipient, content); } + + if (failed > 0) { + failedCount += failed; + } + } + + /** + * @notice Lets contract-owner send ERC721 tokens to a list of addresses. + * @dev The token-owner should approve target tokens to Airdrop contract, + * which acts as operator for the tokens. + * + * @param _contents List containing recipient, tokenId to airdrop. + */ + function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyOwner { + uint256 len = _contents.length; + + for (uint256 i = 0; i < len; i++) { + try + IERC721(_contents[i].tokenAddress).safeTransferFrom( + _contents[i].tokenOwner, + _contents[i].recipient, + _contents[i].tokenId + ) + {} catch {} + } } /*/////////////////////////////////////////////////////////////// diff --git a/lib/forge-std b/lib/forge-std index 0aa99eb84..4a79aca83 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 0aa99eb8456693c015350c5e6c4f442ebe912f77 +Subproject commit 4a79aca83f8075f8b1b4fe9153945fef08375630 From 7b4bb09801a36d8ac25c6fe2003b12d72ce23ff9 Mon Sep 17 00:00:00 2001 From: Yash Date: Wed, 1 Feb 2023 05:43:30 +0530 Subject: [PATCH 02/18] failsafe --- contracts/airdrop/AirdropERC1155.sol | 18 +++++++++++------- contracts/airdrop/AirdropERC721.sol | 8 +------- src/test/airdrop/AirdropERC1155.t.sol | 18 +++++++++--------- src/test/airdrop/AirdropERC721.t.sol | 18 +++++++++--------- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index e25b02643..de95bd75a 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -95,13 +95,17 @@ contract AirdropERC1155 is for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { AirdropContent memory content = airdropContent[i]; - IERC1155(content.tokenAddress).safeTransferFrom( - content.tokenOwner, - content.recipient, - content.tokenId, - content.amount, - "" - ); + try + IERC1155(content.tokenAddress).safeTransferFrom( + content.tokenOwner, + content.recipient, + content.tokenId, + content.amount, + "" + ) + {} catch { + indicesOfFailed.push(i); + } emit AirdropPayment(content.recipient, content); } diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index aadd36ba0..9cba975de 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -32,7 +32,6 @@ contract AirdropERC721 is uint256 public payeeCount; uint256 public processedCount; - uint256 public failedCount; uint256[] private indicesOfFailed; @@ -88,7 +87,6 @@ contract AirdropERC721 is function airdrop(uint256 paymentsToProcess) external nonReentrant { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; - uint256 failed; require(countOfProcessed + paymentsToProcess <= totalPayees, "invalid no. of payments"); @@ -100,15 +98,11 @@ contract AirdropERC721 is try IERC721(content.tokenAddress).safeTransferFrom(content.tokenOwner, content.recipient, content.tokenId) {} catch { - indicesOfFailed[failed++] = i; + indicesOfFailed.push(i); } emit AirdropPayment(content.recipient, content); } - - if (failed > 0) { - failedCount += failed; - } } /** diff --git a/src/test/airdrop/AirdropERC1155.t.sol b/src/test/airdrop/AirdropERC1155.t.sol index cd77e2490..cf0d716a1 100644 --- a/src/test/airdrop/AirdropERC1155.t.sol +++ b/src/test/airdrop/AirdropERC1155.t.sol @@ -75,13 +75,13 @@ contract AirdropERC1155Test is BaseTest { drop.addAirdropRecipients(_contents); } - function test_revert_airdrop_notApproved() public { - tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); - - vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); - vm.expectRevert("ERC1155: caller is not token owner nor approved"); - drop.airdrop(_contents.length); - vm.stopPrank(); - } + // function test_revert_airdrop_notApproved() public { + // tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); + + // vm.startPrank(deployer); + // drop.addAirdropRecipients(_contents); + // vm.expectRevert("ERC1155: caller is not token owner nor approved"); + // drop.airdrop(_contents.length); + // vm.stopPrank(); + // } } diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index 0dce29f33..51886aeb8 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -64,13 +64,13 @@ contract AirdropERC721Test is BaseTest { drop.addAirdropRecipients(_contents); } - function test_revert_airdrop_notApproved() public { - tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); - - vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); - vm.expectRevert("ERC721: caller is not token owner nor approved"); - drop.airdrop(_contents.length); - vm.stopPrank(); - } + // function test_revert_airdrop_notApproved() public { + // tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); + + // vm.startPrank(deployer); + // drop.addAirdropRecipients(_contents); + // vm.expectRevert("ERC721: caller is not token owner nor approved"); + // drop.airdrop(_contents.length); + // vm.stopPrank(); + // } } From 45feb56f084752f5d442843c319fb7dc64ddf3c0 Mon Sep 17 00:00:00 2001 From: Yash Date: Thu, 2 Feb 2023 20:34:38 +0530 Subject: [PATCH 03/18] reset functionality for airdrops --- contracts/airdrop/AirdropERC1155.sol | 103 ++++++++++++++---- contracts/airdrop/AirdropERC20.sol | 94 ++++++++++++---- contracts/airdrop/AirdropERC721.sol | 103 ++++++++++++++---- .../interfaces/airdrop/IAirdropERC1155.sol | 39 ++++++- .../interfaces/airdrop/IAirdropERC20.sol | 39 ++++++- .../interfaces/airdrop/IAirdropERC721.sol | 39 ++++++- 6 files changed, 339 insertions(+), 78 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index de95bd75a..7c8d9ef8e 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -33,10 +33,12 @@ contract AirdropERC1155 is uint256 public payeeCount; uint256 public processedCount; - uint256[] private indicesOfFailed; + uint256[] public indicesOfFailed; mapping(uint256 => AirdropContent) private airdropContent; + mapping(uint256 => bool) private isCancelled; + /*/////////////////////////////////////////////////////////////// Constructor + initializer logic //////////////////////////////////////////////////////////////*/ @@ -69,7 +71,7 @@ contract AirdropERC1155 is //////////////////////////////////////////////////////////////*/ ///@notice Lets contract-owner set up an airdrop of ERC721 NFTs to a list of addresses. - function addAirdropRecipients(AirdropContent[] calldata _contents) external onlyRole(DEFAULT_ADMIN_ROLE) { + function addRecipients(AirdropContent[] calldata _contents) external onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; require(len > 0, "No payees provided."); @@ -83,8 +85,21 @@ contract AirdropERC1155 is emit RecipientsAdded(_contents); } + ///@notice Lets contract-owner cancel any pending payments. + function resetRecipients() external onlyRole(DEFAULT_ADMIN_ROLE) { + uint256 totalPayees = payeeCount; + uint256 countOfProcessed = processedCount; + + // set processedCount to payeeCount -- ignore all pending payments. + processedCount = payeeCount; + + for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { + isCancelled[i] = true; + } + } + /// @notice Lets contract-owner send ERC721 NFTs to a list of addresses. - function airdrop(uint256 paymentsToProcess) external nonReentrant { + function processPayments(uint256 paymentsToProcess) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; @@ -95,6 +110,7 @@ contract AirdropERC1155 is for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { AirdropContent memory content = airdropContent[i]; + bool failed; try IERC1155(content.tokenAddress).safeTransferFrom( content.tokenOwner, @@ -105,9 +121,10 @@ contract AirdropERC1155 is ) {} catch { indicesOfFailed.push(i); + failed = true; } - emit AirdropPayment(content.recipient, content); + emit AirdropPayment(content.recipient, content, failed); } } @@ -118,10 +135,11 @@ contract AirdropERC1155 is * * @param _contents List containing recipient, tokenId and amounts to airdrop. */ - function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyOwner { + function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; for (uint256 i = 0; i < len; i++) { + bool failed; try IERC1155(_contents[i].tokenAddress).safeTransferFrom( _contents[i].tokenOwner, @@ -130,7 +148,11 @@ contract AirdropERC1155 is _contents[i].amount, "" ) - {} catch {} + {} catch { + failed = true; + } + + emit StatelessAirdrop(_contents[i].recipient, _contents[i], failed); } } @@ -139,35 +161,74 @@ contract AirdropERC1155 is //////////////////////////////////////////////////////////////*/ /// @notice Returns all airdrop payments set up -- pending, processed or failed. - function getAllAirdropPayments() external view returns (AirdropContent[] memory contents) { - uint256 count = payeeCount; - contents = new AirdropContent[](count); + function getAllAirdropPayments(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); - for (uint256 i = 0; i < count; i += 1) { - contents[i] = airdropContent[i]; + contents = new AirdropContent[](endId - startId + 1); + + for (uint256 i = startId; i <= endId; i += 1) { + contents[i - startId] = airdropContent[i]; } } /// @notice Returns all pending airdrop payments. - function getAllAirdropPaymentsPending() external view returns (AirdropContent[] memory contents) { - uint256 endCount = payeeCount; - uint256 startCount = processedCount; - contents = new AirdropContent[](endCount - startCount); + function getAllAirdropPaymentsPending(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + + uint256 processed = processedCount; + if (startId < processed) { + startId = processed; + } + contents = new AirdropContent[](endId - startId); uint256 idx; - for (uint256 i = startCount; i < endCount; i += 1) { + for (uint256 i = startId; i <= endId; i += 1) { contents[idx] = airdropContent[i]; idx += 1; } } /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed() external view returns (AirdropContent[] memory contents) { - uint256 count = processedCount; + function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + uint256 processed = processedCount; + if (startId >= processed) { + return contents; + } + + if (endId >= processed) { + endId = processed - 1; + } + + uint256 count; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + continue; + } + count += 1; + } + contents = new AirdropContent[](count); + uint256 index; - for (uint256 i = 0; i < count; i += 1) { - contents[i] = airdropContent[i]; + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + continue; + } + contents[index++] = airdropContent[i]; } } @@ -187,6 +248,6 @@ contract AirdropERC1155 is /// @dev Returns whether owner can be set in the given execution context. function _canSetOwner() internal view virtual override returns (bool) { - return msg.sender == owner(); + return hasRole(DEFAULT_ADMIN_ROLE, msg.sender); } } diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index bbb08d100..5bdb7d0cf 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -33,10 +33,12 @@ contract AirdropERC20 is uint256 public payeeCount; uint256 public processedCount; - uint256[] private indicesOfFailed; + uint256[] public indicesOfFailed; mapping(uint256 => AirdropContent) private airdropContent; + mapping(uint256 => bool) private isCancelled; + /*/////////////////////////////////////////////////////////////// Constructor + initializer logic //////////////////////////////////////////////////////////////*/ @@ -69,7 +71,7 @@ contract AirdropERC20 is //////////////////////////////////////////////////////////////*/ ///@notice Lets contract-owner set up an airdrop of ERC20 or native tokens to a list of addresses. - function addAirdropRecipients(AirdropContent[] calldata _contents) external payable onlyRole(DEFAULT_ADMIN_ROLE) { + function addRecipients(AirdropContent[] calldata _contents) external payable onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; require(len > 0, "No payees provided."); @@ -91,8 +93,21 @@ contract AirdropERC20 is emit RecipientsAdded(_contents); } + ///@notice Lets contract-owner cancel any pending payments. + function resetRecipients() external onlyRole(DEFAULT_ADMIN_ROLE) { + uint256 totalPayees = payeeCount; + uint256 countOfProcessed = processedCount; + + // set processedCount to payeeCount -- ignore all pending payments. + processedCount = payeeCount; + + for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { + isCancelled[i] = true; + } + } + /// @notice Lets contract-owner send ERC20 or native tokens to a list of addresses. - function airdrop(uint256 paymentsToProcess) external nonReentrant { + function processPayments(uint256 paymentsToProcess) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; @@ -121,7 +136,7 @@ contract AirdropERC20 is * * @param _contents List containing recipient, tokenId and amounts to airdrop. */ - function airdrop(AirdropContent[] calldata _contents) external payable nonReentrant onlyOwner { + function airdrop(AirdropContent[] calldata _contents) external payable nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; uint256 nativeTokenAmount; @@ -136,6 +151,8 @@ contract AirdropERC20 is if (_contents[i].tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { nativeTokenAmount += _contents[i].amount; } + + emit StatelessAirdrop(_contents[i].recipient, _contents[i]); } require(nativeTokenAmount == msg.value, "Incorrect native token amount"); @@ -146,35 +163,74 @@ contract AirdropERC20 is //////////////////////////////////////////////////////////////*/ /// @notice Returns all airdrop payments set up -- pending, processed or failed. - function getAllAirdropPayments() external view returns (AirdropContent[] memory contents) { - uint256 count = payeeCount; - contents = new AirdropContent[](count); + function getAllAirdropPayments(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); - for (uint256 i = 0; i < count; i += 1) { - contents[i] = airdropContent[i]; + contents = new AirdropContent[](endId - startId + 1); + + for (uint256 i = startId; i <= endId; i += 1) { + contents[i - startId] = airdropContent[i]; } } /// @notice Returns all pending airdrop payments. - function getAllAirdropPaymentsPending() external view returns (AirdropContent[] memory contents) { - uint256 endCount = payeeCount; - uint256 startCount = processedCount; - contents = new AirdropContent[](endCount - startCount); + function getAllAirdropPaymentsPending(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + + uint256 processed = processedCount; + if (startId < processed) { + startId = processed; + } + contents = new AirdropContent[](endId - startId); uint256 idx; - for (uint256 i = startCount; i < endCount; i += 1) { + for (uint256 i = startId; i <= endId; i += 1) { contents[idx] = airdropContent[i]; idx += 1; } } /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed() external view returns (AirdropContent[] memory contents) { - uint256 count = processedCount; + function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + uint256 processed = processedCount; + if (startId >= processed) { + return contents; + } + + if (endId >= processed) { + endId = processed - 1; + } + + uint256 count; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + continue; + } + count += 1; + } + contents = new AirdropContent[](count); + uint256 index; - for (uint256 i = 0; i < count; i += 1) { - contents[i] = airdropContent[i]; + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + continue; + } + contents[index++] = airdropContent[i]; } } @@ -194,6 +250,6 @@ contract AirdropERC20 is /// @dev Returns whether owner can be set in the given execution context. function _canSetOwner() internal view virtual override returns (bool) { - return msg.sender == owner(); + return hasRole(DEFAULT_ADMIN_ROLE, msg.sender); } } diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 9cba975de..29f346fa6 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -33,10 +33,12 @@ contract AirdropERC721 is uint256 public payeeCount; uint256 public processedCount; - uint256[] private indicesOfFailed; + uint256[] public indicesOfFailed; mapping(uint256 => AirdropContent) private airdropContent; + mapping(uint256 => bool) private isCancelled; + /*/////////////////////////////////////////////////////////////// Constructor + initializer logic //////////////////////////////////////////////////////////////*/ @@ -69,7 +71,7 @@ contract AirdropERC721 is //////////////////////////////////////////////////////////////*/ ///@notice Lets contract-owner set up an airdrop of ERC721 NFTs to a list of addresses. - function addAirdropRecipients(AirdropContent[] calldata _contents) external onlyRole(DEFAULT_ADMIN_ROLE) { + function addRecipients(AirdropContent[] calldata _contents) external onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; require(len > 0, "No payees provided."); @@ -83,8 +85,21 @@ contract AirdropERC721 is emit RecipientsAdded(_contents); } + ///@notice Lets contract-owner cancel any pending payments. + function resetRecipients() external onlyRole(DEFAULT_ADMIN_ROLE) { + uint256 totalPayees = payeeCount; + uint256 countOfProcessed = processedCount; + + // set processedCount to payeeCount -- ignore all pending payments. + processedCount = payeeCount; + + for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { + isCancelled[i] = true; + } + } + /// @notice Lets contract-owner send ERC721 NFTs to a list of addresses. - function airdrop(uint256 paymentsToProcess) external nonReentrant { + function processPayments(uint256 paymentsToProcess) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; @@ -95,13 +110,15 @@ contract AirdropERC721 is for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { AirdropContent memory content = airdropContent[i]; + bool failed; try IERC721(content.tokenAddress).safeTransferFrom(content.tokenOwner, content.recipient, content.tokenId) {} catch { indicesOfFailed.push(i); + failed = true; } - emit AirdropPayment(content.recipient, content); + emit AirdropPayment(content.recipient, content, failed); } } @@ -112,17 +129,22 @@ contract AirdropERC721 is * * @param _contents List containing recipient, tokenId to airdrop. */ - function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyOwner { + function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; for (uint256 i = 0; i < len; i++) { + bool failed; try IERC721(_contents[i].tokenAddress).safeTransferFrom( _contents[i].tokenOwner, _contents[i].recipient, _contents[i].tokenId ) - {} catch {} + {} catch { + failed = true; + } + + emit StatelessAirdrop(_contents[i].recipient, _contents[i], failed); } } @@ -131,35 +153,74 @@ contract AirdropERC721 is //////////////////////////////////////////////////////////////*/ /// @notice Returns all airdrop payments set up -- pending, processed or failed. - function getAllAirdropPayments() external view returns (AirdropContent[] memory contents) { - uint256 count = payeeCount; - contents = new AirdropContent[](count); + function getAllAirdropPayments(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); - for (uint256 i = 0; i < count; i += 1) { - contents[i] = airdropContent[i]; + contents = new AirdropContent[](endId - startId + 1); + + for (uint256 i = startId; i <= endId; i += 1) { + contents[i - startId] = airdropContent[i]; } } /// @notice Returns all pending airdrop payments. - function getAllAirdropPaymentsPending() external view returns (AirdropContent[] memory contents) { - uint256 endCount = payeeCount; - uint256 startCount = processedCount; - contents = new AirdropContent[](endCount - startCount); + function getAllAirdropPaymentsPending(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + + uint256 processed = processedCount; + if (startId < processed) { + startId = processed; + } + contents = new AirdropContent[](endId - startId); uint256 idx; - for (uint256 i = startCount; i < endCount; i += 1) { + for (uint256 i = startId; i <= endId; i += 1) { contents[idx] = airdropContent[i]; idx += 1; } } /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed() external view returns (AirdropContent[] memory contents) { - uint256 count = processedCount; + function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + uint256 processed = processedCount; + if (startId >= processed) { + return contents; + } + + if (endId >= processed) { + endId = processed - 1; + } + + uint256 count; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + continue; + } + count += 1; + } + contents = new AirdropContent[](count); + uint256 index; - for (uint256 i = 0; i < count; i += 1) { - contents[i] = airdropContent[i]; + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + continue; + } + contents[index++] = airdropContent[i]; } } @@ -179,6 +240,6 @@ contract AirdropERC721 is /// @dev Returns whether owner can be set in the given execution context. function _canSetOwner() internal view virtual override returns (bool) { - return msg.sender == owner(); + return hasRole(DEFAULT_ADMIN_ROLE, msg.sender); } } diff --git a/contracts/interfaces/airdrop/IAirdropERC1155.sol b/contracts/interfaces/airdrop/IAirdropERC1155.sol index fb497f1ce..664445f96 100644 --- a/contracts/interfaces/airdrop/IAirdropERC1155.sol +++ b/contracts/interfaces/airdrop/IAirdropERC1155.sol @@ -12,8 +12,12 @@ pragma solidity ^0.8.11; interface IAirdropERC1155 { /// @notice Emitted when airdrop recipients are uploaded to the contract. event RecipientsAdded(AirdropContent[] _contents); + /// @notice Emitted when pending payments are cancelled, and processed count is reset. + event PaymentsResetByAdmin(); /// @notice Emitted when an airdrop payment is made to a recipient. - event AirdropPayment(address indexed recipient, AirdropContent content); + event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); + /// @notice Emitted when an airdrop is made using the stateless airdrop function. + event StatelessAirdrop(address indexed recipient, AirdropContent content, bool failed); /** * @notice Details of amount and recipient for airdropped token. @@ -33,13 +37,22 @@ interface IAirdropERC1155 { } /// @notice Returns all airdrop payments set up -- pending, processed or failed. - function getAllAirdropPayments() external view returns (AirdropContent[] memory contents); + function getAllAirdropPayments(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop payments. - function getAllAirdropPaymentsPending() external view returns (AirdropContent[] memory contents); + function getAllAirdropPaymentsPending(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed() external view returns (AirdropContent[] memory contents); + function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents); @@ -51,7 +64,12 @@ interface IAirdropERC1155 { * * @param _contents List containing recipients, tokenIds to airdrop. */ - function addAirdropRecipients(AirdropContent[] calldata _contents) external; + function addRecipients(AirdropContent[] calldata _contents) external; + + /** + * @notice Lets contract-owner cancel any pending payments. + */ + function resetRecipients() external; /** * @notice Lets contract-owner set up an airdrop of ERC1155 tokens to a list of addresses. @@ -60,5 +78,14 @@ interface IAirdropERC1155 { * * @param paymentsToProcess The number of airdrop payments to process. */ - function airdrop(uint256 paymentsToProcess) external; + function processPayments(uint256 paymentsToProcess) external; + + /** + * @notice Lets contract-owner send ERC1155 tokens to a list of addresses. + * @dev The token-owner should approve target tokens to Airdrop contract, + * which acts as operator for the tokens. + * + * @param _contents List containing recipient, tokenId to airdrop. + */ + function airdrop(AirdropContent[] calldata _contents) external; } diff --git a/contracts/interfaces/airdrop/IAirdropERC20.sol b/contracts/interfaces/airdrop/IAirdropERC20.sol index 8306ecfed..a5bd3831b 100644 --- a/contracts/interfaces/airdrop/IAirdropERC20.sol +++ b/contracts/interfaces/airdrop/IAirdropERC20.sol @@ -12,12 +12,18 @@ pragma solidity ^0.8.11; interface IAirdropERC20 { /// @notice Emitted when airdrop recipients are uploaded to the contract. event RecipientsAdded(AirdropContent[] _contents); + /// @notice Emitted when pending payments are cancelled, and processed count is reset. + event PaymentsResetByAdmin(); /// @notice Emitted when an airdrop payment is made to a recipient. event AirdropPayment(address indexed recipient, AirdropContent content); + /// @notice Emitted when an airdrop is made using the stateless airdrop function. + event StatelessAirdrop(address indexed recipient, AirdropContent content); /** * @notice Details of amount and recipient for airdropped token. * + * @param tokenAddress The contract address of the tokens to transfer. + * @param tokenOwner The owner of the the tokens to transfer. * @param recipient The recipient of the tokens. * @param amount The quantity of tokens to airdrop. */ @@ -29,13 +35,22 @@ interface IAirdropERC20 { } /// @notice Returns all airdrop payments set up -- pending, processed or failed. - function getAllAirdropPayments() external view returns (AirdropContent[] memory contents); + function getAllAirdropPayments(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop payments. - function getAllAirdropPaymentsPending() external view returns (AirdropContent[] memory contents); + function getAllAirdropPaymentsPending(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed() external view returns (AirdropContent[] memory contents); + function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents); @@ -45,7 +60,12 @@ interface IAirdropERC20 { * * @param _contents List containing recipients, amounts to airdrop. */ - function addAirdropRecipients(AirdropContent[] calldata _contents) external payable; + function addRecipients(AirdropContent[] calldata _contents) external payable; + + /** + * @notice Lets contract-owner cancel any pending payments. + */ + function resetRecipients() external; /** * @notice Lets contract-owner send ERC20 or native tokens to a list of addresses. @@ -54,5 +74,14 @@ interface IAirdropERC20 { * * @param paymentsToProcess The number of airdrop payments to process. */ - function airdrop(uint256 paymentsToProcess) external; + function processPayments(uint256 paymentsToProcess) external; + + /** + * @notice Lets contract-owner send ERC20 tokens to a list of addresses. + * @dev The token-owner should approve target tokens to Airdrop contract, + * which acts as operator for the tokens. + * + * @param _contents List containing recipient, tokenId to airdrop. + */ + function airdrop(AirdropContent[] calldata _contents) external payable; } diff --git a/contracts/interfaces/airdrop/IAirdropERC721.sol b/contracts/interfaces/airdrop/IAirdropERC721.sol index 6795ad92b..ea59d5cb9 100644 --- a/contracts/interfaces/airdrop/IAirdropERC721.sol +++ b/contracts/interfaces/airdrop/IAirdropERC721.sol @@ -12,8 +12,12 @@ pragma solidity ^0.8.11; interface IAirdropERC721 { /// @notice Emitted when airdrop recipients are uploaded to the contract. event RecipientsAdded(AirdropContent[] _contents); + /// @notice Emitted when pending payments are cancelled, and processed count is reset. + event PaymentsResetByAdmin(); /// @notice Emitted when an airdrop payment is made to a recipient. - event AirdropPayment(address indexed recipient, AirdropContent content); + event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); + /// @notice Emitted when an airdrop is made using the stateless airdrop function. + event StatelessAirdrop(address indexed recipient, AirdropContent content, bool failed); /** * @notice Details of amount and recipient for airdropped token. @@ -31,13 +35,22 @@ interface IAirdropERC721 { } /// @notice Returns all airdrop payments set up -- pending, processed or failed. - function getAllAirdropPayments() external view returns (AirdropContent[] memory contents); + function getAllAirdropPayments(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop payments. - function getAllAirdropPaymentsPending() external view returns (AirdropContent[] memory contents); + function getAllAirdropPaymentsPending(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed() external view returns (AirdropContent[] memory contents); + function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents); /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents); @@ -49,7 +62,12 @@ interface IAirdropERC721 { * * @param _contents List containing recipients, tokenIds to airdrop. */ - function addAirdropRecipients(AirdropContent[] calldata _contents) external; + function addRecipients(AirdropContent[] calldata _contents) external; + + /** + * @notice Lets contract-owner cancel any pending payments. + */ + function resetRecipients() external; /** * @notice Lets contract-owner set up an airdrop of ERC721 tokens to a list of addresses. @@ -58,5 +76,14 @@ interface IAirdropERC721 { * * @param paymentsToProcess The number of airdrop payments to process. */ - function airdrop(uint256 paymentsToProcess) external; + function processPayments(uint256 paymentsToProcess) external; + + /** + * @notice Lets contract-owner send ERC721 tokens to a list of addresses. + * @dev The token-owner should approve target tokens to Airdrop contract, + * which acts as operator for the tokens. + * + * @param _contents List containing recipient, tokenId to airdrop. + */ + function airdrop(AirdropContent[] calldata _contents) external; } From 2330fb62189013a43a7b47b58ef3787232c908a4 Mon Sep 17 00:00:00 2001 From: Yash Date: Thu, 2 Feb 2023 22:04:42 +0530 Subject: [PATCH 04/18] failsafe for erc20 airdrop --- contracts/airdrop/AirdropERC20.sol | 25 +++++++++++++------ .../interfaces/airdrop/IAirdropERC20.sol | 2 +- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 5bdb7d0cf..4b65e3ffe 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -10,6 +10,7 @@ import "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol"; import "../interfaces/airdrop/IAirdropERC20.sol"; import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol"; +import "../openzeppelin-presets/token/ERC20/utils/SafeERC20.sol"; // ========== Features ========== import "../extension/Ownable.sol"; @@ -23,6 +24,7 @@ contract AirdropERC20 is MulticallUpgradeable, IAirdropERC20 { + using SafeERC20 for IERC20; /*/////////////////////////////////////////////////////////////// State variables //////////////////////////////////////////////////////////////*/ @@ -118,14 +120,23 @@ contract AirdropERC20 is for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { AirdropContent memory content = airdropContent[i]; - CurrencyTransferLib.transferCurrency( - content.tokenAddress, - content.tokenOwner, - content.recipient, - content.amount - ); + bool success; + if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { + // solhint-disable avoid-low-level-calls + // slither-disable-next-line low-level-calls + (success, ) = content.recipient.call{ value: content.amount }(""); + } else { + try + IERC20(content.tokenAddress).transferFrom(content.tokenOwner, content.recipient, content.amount) + returns (bool _success) { + success = _success; + } catch { + indicesOfFailed.push(i); + success = false; + } + } - emit AirdropPayment(content.recipient, content); + emit AirdropPayment(content.recipient, content, !success); } } diff --git a/contracts/interfaces/airdrop/IAirdropERC20.sol b/contracts/interfaces/airdrop/IAirdropERC20.sol index a5bd3831b..843d60c32 100644 --- a/contracts/interfaces/airdrop/IAirdropERC20.sol +++ b/contracts/interfaces/airdrop/IAirdropERC20.sol @@ -15,7 +15,7 @@ interface IAirdropERC20 { /// @notice Emitted when pending payments are cancelled, and processed count is reset. event PaymentsResetByAdmin(); /// @notice Emitted when an airdrop payment is made to a recipient. - event AirdropPayment(address indexed recipient, AirdropContent content); + event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. event StatelessAirdrop(address indexed recipient, AirdropContent content); From b8689f520eb34593b6dc98585073db2b90badb34 Mon Sep 17 00:00:00 2001 From: Yash Date: Fri, 3 Feb 2023 02:25:09 +0530 Subject: [PATCH 05/18] fix tests --- src/test/airdrop/AirdropERC1155.t.sol | 10 +++---- .../airdrop/AirdropERC1155Benchmark.t.sol | 8 ++--- src/test/airdrop/AirdropERC20.t.sol | 30 +++++++++---------- src/test/airdrop/AirdropERC20Benchmark.t.sol | 10 +++---- src/test/airdrop/AirdropERC721.t.sol | 10 +++---- src/test/airdrop/AirdropERC721Benchmark.t.sol | 8 ++--- src/test/utils/BaseTest.sol | 2 +- 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/test/airdrop/AirdropERC1155.t.sol b/src/test/airdrop/AirdropERC1155.t.sol index cf0d716a1..7dcfc0d31 100644 --- a/src/test/airdrop/AirdropERC1155.t.sol +++ b/src/test/airdrop/AirdropERC1155.t.sol @@ -48,8 +48,8 @@ contract AirdropERC1155Test is BaseTest { function test_state_airdrop() public { vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); - drop.airdrop(_contents.length); + drop.addRecipients(_contents); + drop.processPayments(_contents.length); vm.stopPrank(); for (uint256 i = 0; i < 1000; i++) { @@ -72,16 +72,16 @@ contract AirdropERC1155Test is BaseTest { TWStrings.toHexString(uint256(0x00), 32) ) ); - drop.addAirdropRecipients(_contents); + drop.addRecipients(_contents); } // function test_revert_airdrop_notApproved() public { // tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); // vm.startPrank(deployer); - // drop.addAirdropRecipients(_contents); + // drop.addRecipients(_contents); // vm.expectRevert("ERC1155: caller is not token owner nor approved"); - // drop.airdrop(_contents.length); + // drop.processPayments(_contents.length); // vm.stopPrank(); // } } diff --git a/src/test/airdrop/AirdropERC1155Benchmark.t.sol b/src/test/airdrop/AirdropERC1155Benchmark.t.sol index 2f69f7e11..ace77aff0 100644 --- a/src/test/airdrop/AirdropERC1155Benchmark.t.sol +++ b/src/test/airdrop/AirdropERC1155Benchmark.t.sol @@ -43,18 +43,18 @@ contract AirdropERC1155BenchmarkTest is BaseTest { vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); + drop.addRecipients(_contents); } function test_benchmark_airdrop_one() public { - drop.airdrop(1); + drop.processPayments(1); } function test_benchmark_airdrop_two() public { - drop.airdrop(2); + drop.processPayments(2); } function test_benchmark_airdrop_five() public { - drop.airdrop(5); + drop.processPayments(5); } } diff --git a/src/test/airdrop/AirdropERC20.t.sol b/src/test/airdrop/AirdropERC20.t.sol index db5a96c4b..2d982f4ad 100644 --- a/src/test/airdrop/AirdropERC20.t.sol +++ b/src/test/airdrop/AirdropERC20.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import "contracts/airdrop/AirdropERC20.sol"; +import { AirdropERC20, IAirdropERC20 } from "contracts/airdrop/AirdropERC20.sol"; // Test imports import { Wallet } from "../utils/Wallet.sol"; @@ -42,8 +42,8 @@ contract AirdropERC20Test is BaseTest { function test_state_airdrop() public { vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); - drop.airdrop(_contents.length); + drop.addRecipients(_contents); + drop.processPayments(_contents.length); vm.stopPrank(); for (uint256 i = 0; i < 1000; i++) { @@ -62,8 +62,8 @@ contract AirdropERC20Test is BaseTest { } vm.startPrank(deployer); - drop.addAirdropRecipients{ value: 10_000 ether }(_contents); - drop.airdrop(_contents.length); + drop.addRecipients{ value: 10_000 ether }(_contents); + drop.processPayments(_contents.length); vm.stopPrank(); for (uint256 i = 0; i < 1000; i++) { @@ -83,7 +83,7 @@ contract AirdropERC20Test is BaseTest { vm.prank(deployer); vm.expectRevert("Incorrect native token amount"); - drop.addAirdropRecipients{ value: incorrectAmt }(_contents); + drop.addRecipients{ value: incorrectAmt }(_contents); } function test_revert_airdrop_notAdmin() public { @@ -96,16 +96,16 @@ contract AirdropERC20Test is BaseTest { TWStrings.toHexString(uint256(0x00), 32) ) ); - drop.addAirdropRecipients(_contents); + drop.addRecipients(_contents); } - function test_revert_airdrop_notApproved() public { - tokenOwner.setAllowanceERC20(address(erc20), address(drop), 0); + // function test_revert_airdrop_notApproved() public { + // tokenOwner.setAllowanceERC20(address(erc20), address(drop), 0); - vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); - vm.expectRevert("ERC20: insufficient allowance"); - drop.airdrop(_contents.length); - vm.stopPrank(); - } + // vm.startPrank(deployer); + // drop.addRecipients(_contents); + // vm.expectRevert("ERC20: insufficient allowance"); + // drop.processPayments(_contents.length); + // vm.stopPrank(); + // } } diff --git a/src/test/airdrop/AirdropERC20Benchmark.t.sol b/src/test/airdrop/AirdropERC20Benchmark.t.sol index 3be9a5105..975594a89 100644 --- a/src/test/airdrop/AirdropERC20Benchmark.t.sol +++ b/src/test/airdrop/AirdropERC20Benchmark.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.0; -import "contracts/airdrop/AirdropERC20.sol"; +import { AirdropERC20, IAirdropERC20 } from "contracts/airdrop/AirdropERC20.sol"; // Test imports import { Wallet } from "../utils/Wallet.sol"; @@ -38,18 +38,18 @@ contract AirdropERC20BenchmarkTest is BaseTest { vm.deal(deployer, 10_000 ether); vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); + drop.addRecipients(_contents); } function test_benchmark_airdrop_one_ERC20() public { - drop.airdrop(1); + drop.processPayments(1); } function test_benchmark_airdrop_two_ERC20() public { - drop.airdrop(2); + drop.processPayments(2); } function test_benchmark_airdrop_five_ERC20() public { - drop.airdrop(5); + drop.processPayments(5); } } diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index 51886aeb8..f71e254cf 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -42,8 +42,8 @@ contract AirdropERC721Test is BaseTest { function test_state_airdrop() public { vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); - drop.airdrop(_contents.length); + drop.addRecipients(_contents); + drop.processPayments(_contents.length); vm.stopPrank(); for (uint256 i = 0; i < 1000; i++) { @@ -61,16 +61,16 @@ contract AirdropERC721Test is BaseTest { TWStrings.toHexString(uint256(0x00), 32) ) ); - drop.addAirdropRecipients(_contents); + drop.addRecipients(_contents); } // function test_revert_airdrop_notApproved() public { // tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); // vm.startPrank(deployer); - // drop.addAirdropRecipients(_contents); + // drop.addRecipients(_contents); // vm.expectRevert("ERC721: caller is not token owner nor approved"); - // drop.airdrop(_contents.length); + // drop.processPayments(_contents.length); // vm.stopPrank(); // } } diff --git a/src/test/airdrop/AirdropERC721Benchmark.t.sol b/src/test/airdrop/AirdropERC721Benchmark.t.sol index 314425505..21f525c81 100644 --- a/src/test/airdrop/AirdropERC721Benchmark.t.sol +++ b/src/test/airdrop/AirdropERC721Benchmark.t.sol @@ -37,18 +37,18 @@ contract AirdropERC721BenchmarkTest is BaseTest { vm.startPrank(deployer); - drop.addAirdropRecipients(_contents); + drop.addRecipients(_contents); } function test_benchmark_airdrop_one_ERC721() public { - drop.airdrop(1); + drop.processPayments(1); } function test_benchmark_airdrop_two_ERC721() public { - drop.airdrop(2); + drop.processPayments(2); } function test_benchmark_airdrop_five_ERC721() public { - drop.airdrop(5); + drop.processPayments(5); } } diff --git a/src/test/utils/BaseTest.sol b/src/test/utils/BaseTest.sol index 6f356dd06..01d143b99 100644 --- a/src/test/utils/BaseTest.sol +++ b/src/test/utils/BaseTest.sol @@ -31,7 +31,7 @@ import { ContractPublisher } from "contracts/ContractPublisher.sol"; import { IContractPublisher } from "contracts/interfaces/IContractPublisher.sol"; import "contracts/airdrop/AirdropERC721.sol"; import "contracts/airdrop/AirdropERC721Claimable.sol"; -import "contracts/airdrop/AirdropERC20.sol"; +import { AirdropERC20 } from "contracts/airdrop/AirdropERC20.sol"; import "contracts/airdrop/AirdropERC20Claimable.sol"; import "contracts/airdrop/AirdropERC1155.sol"; import "contracts/airdrop/AirdropERC1155Claimable.sol"; From dddb014684884a4d1b855515f08603323ddca556 Mon Sep 17 00:00:00 2001 From: Yash Date: Fri, 3 Feb 2023 23:37:24 +0530 Subject: [PATCH 06/18] revert for unapproved tokens --- contracts/airdrop/AirdropERC1155.sol | 8 ++++++++ contracts/airdrop/AirdropERC20.sol | 8 ++++++++ contracts/airdrop/AirdropERC721.sol | 9 +++++++++ src/test/airdrop/AirdropERC1155.t.sol | 18 +++++++++--------- src/test/airdrop/AirdropERC20.t.sol | 18 +++++++++--------- src/test/airdrop/AirdropERC721.t.sol | 18 +++++++++--------- 6 files changed, 52 insertions(+), 27 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index 7c8d9ef8e..aca0fd896 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -120,6 +120,14 @@ contract AirdropERC1155 is "" ) {} catch { + // revert if failure is due to unapproved tokens + require( + IERC1155(content.tokenAddress).balanceOf(content.tokenOwner, content.tokenId) >= content.amount && + IERC1155(content.tokenAddress).isApprovedForAll(content.tokenOwner, address(this)), + "Not balance or approved" + ); + + // record and continue for all other failures, likely originating from recipient accounts indicesOfFailed.push(i); failed = true; } diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 4b65e3ffe..16132b942 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -131,6 +131,14 @@ contract AirdropERC20 is returns (bool _success) { success = _success; } catch { + // revert if failure is due to insufficient allowance + require( + IERC20(content.tokenAddress).balanceOf(content.tokenOwner) >= content.amount && + IERC20(content.tokenAddress).allowance(content.tokenOwner, address(this)) >= content.amount, + "Not balance or allowance" + ); + + // record and continue for all other failures, likely originating from recipient accounts indicesOfFailed.push(i); success = false; } diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 29f346fa6..ad2766d74 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -114,6 +114,15 @@ contract AirdropERC721 is try IERC721(content.tokenAddress).safeTransferFrom(content.tokenOwner, content.recipient, content.tokenId) {} catch { + // revert if failure is due to unapproved tokens + require( + (IERC721(content.tokenAddress).ownerOf(content.tokenId) == content.tokenOwner && + address(this) == IERC721(content.tokenAddress).getApproved(content.tokenId)) || + IERC721(content.tokenAddress).isApprovedForAll(content.tokenOwner, address(this)), + "Not owner or approved" + ); + + // record all other failures, likely originating from recipient accounts indicesOfFailed.push(i); failed = true; } diff --git a/src/test/airdrop/AirdropERC1155.t.sol b/src/test/airdrop/AirdropERC1155.t.sol index 7dcfc0d31..b66f17b6b 100644 --- a/src/test/airdrop/AirdropERC1155.t.sol +++ b/src/test/airdrop/AirdropERC1155.t.sol @@ -75,13 +75,13 @@ contract AirdropERC1155Test is BaseTest { drop.addRecipients(_contents); } - // function test_revert_airdrop_notApproved() public { - // tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); - - // vm.startPrank(deployer); - // drop.addRecipients(_contents); - // vm.expectRevert("ERC1155: caller is not token owner nor approved"); - // drop.processPayments(_contents.length); - // vm.stopPrank(); - // } + function test_revert_airdrop_notApproved() public { + tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); + + vm.startPrank(deployer); + drop.addRecipients(_contents); + vm.expectRevert("Not balance or approved"); + drop.processPayments(_contents.length); + vm.stopPrank(); + } } diff --git a/src/test/airdrop/AirdropERC20.t.sol b/src/test/airdrop/AirdropERC20.t.sol index 2d982f4ad..4b4782090 100644 --- a/src/test/airdrop/AirdropERC20.t.sol +++ b/src/test/airdrop/AirdropERC20.t.sol @@ -99,13 +99,13 @@ contract AirdropERC20Test is BaseTest { drop.addRecipients(_contents); } - // function test_revert_airdrop_notApproved() public { - // tokenOwner.setAllowanceERC20(address(erc20), address(drop), 0); - - // vm.startPrank(deployer); - // drop.addRecipients(_contents); - // vm.expectRevert("ERC20: insufficient allowance"); - // drop.processPayments(_contents.length); - // vm.stopPrank(); - // } + function test_revert_airdrop_notApproved() public { + tokenOwner.setAllowanceERC20(address(erc20), address(drop), 0); + + vm.startPrank(deployer); + drop.addRecipients(_contents); + vm.expectRevert("Not balance or allowance"); + drop.processPayments(_contents.length); + vm.stopPrank(); + } } diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index f71e254cf..251d1ea4a 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -64,13 +64,13 @@ contract AirdropERC721Test is BaseTest { drop.addRecipients(_contents); } - // function test_revert_airdrop_notApproved() public { - // tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); - - // vm.startPrank(deployer); - // drop.addRecipients(_contents); - // vm.expectRevert("ERC721: caller is not token owner nor approved"); - // drop.processPayments(_contents.length); - // vm.stopPrank(); - // } + function test_revert_airdrop_notApproved() public { + tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); + + vm.startPrank(deployer); + drop.addRecipients(_contents); + vm.expectRevert("Not owner or approved"); + drop.processPayments(_contents.length); + vm.stopPrank(); + } } From f6f8e3b3e187fb7878c4b59b2ed18549f9ea7568 Mon Sep 17 00:00:00 2001 From: Krishang Date: Fri, 3 Feb 2023 13:40:19 -0500 Subject: [PATCH 07/18] add internal fn for currency transfer with bool return val --- contracts/airdrop/AirdropERC20.sol | 83 ++++++++++++++++++++++-------- lib/forge-std | 2 +- 2 files changed, 62 insertions(+), 23 deletions(-) diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 16132b942..3e3c23fda 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -120,30 +120,40 @@ contract AirdropERC20 is for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { AirdropContent memory content = airdropContent[i]; - bool success; - if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { - // solhint-disable avoid-low-level-calls - // slither-disable-next-line low-level-calls - (success, ) = content.recipient.call{ value: content.amount }(""); - } else { - try - IERC20(content.tokenAddress).transferFrom(content.tokenOwner, content.recipient, content.amount) - returns (bool _success) { - success = _success; - } catch { - // revert if failure is due to insufficient allowance - require( - IERC20(content.tokenAddress).balanceOf(content.tokenOwner) >= content.amount && - IERC20(content.tokenAddress).allowance(content.tokenOwner, address(this)) >= content.amount, - "Not balance or allowance" - ); - - // record and continue for all other failures, likely originating from recipient accounts - indicesOfFailed.push(i); - success = false; - } + bool success = _transferCurrencyWithReturnVal( + content.tokenAddress, + content.tokenOwner, + content.recipient, + content.amount + ); + + if (!success) { + // Track failure } + // if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { + // // solhint-disable avoid-low-level-calls + // // slither-disable-next-line low-level-calls + // (success, ) = content.recipient.call{ value: content.amount }(""); + // } else { + // try + // IERC20(content.tokenAddress).transferFrom(content.tokenOwner, content.recipient, content.amount) + // returns (bool _success) { + // success = _success; + // } catch { + // // revert if failure is due to insufficient allowance + // require( + // IERC20(content.tokenAddress).balanceOf(content.tokenOwner) >= content.amount && + // IERC20(content.tokenAddress).allowance(content.tokenOwner, address(this)) >= content.amount, + // "Not balance or allowance" + // ); + + // // record and continue for all other failures, likely originating from recipient accounts + // indicesOfFailed.push(i); + // success = false; + // } + // } + emit AirdropPayment(content.recipient, content, !success); } } @@ -267,6 +277,35 @@ contract AirdropERC20 is Miscellaneous //////////////////////////////////////////////////////////////*/ + /// @dev Transfers ERC20 tokens and returns a boolean i.e. the status of the transfer. + function _transferCurrencyWithReturnVal( + address _currency, + address _from, + address _to, + uint256 _amount + ) internal returns (bool success) { + if (_amount == 0) { + success = true; + return success; + } + + if (_currency == CurrencyTransferLib.NATIVE_TOKEN) { + (success, ) = _to.call{ value: _amount }(""); + } else { + try IERC20(_currency).transferFrom(_from, _to, _amount) returns (bool success_) { + success = success_; + } catch { + require( + IERC20(_currency).balanceOf(_from) >= _amount && + IERC20(_currency).allowance(_from, address(this)) >= _amount, + "CurrencyTransferBal: insufficient balance or allowance." + ); + + success = false; + } + } + } + /// @dev Returns whether owner can be set in the given execution context. function _canSetOwner() internal view virtual override returns (bool) { return hasRole(DEFAULT_ADMIN_ROLE, msg.sender); diff --git a/lib/forge-std b/lib/forge-std index 4a79aca83..0aa99eb84 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 4a79aca83f8075f8b1b4fe9153945fef08375630 +Subproject commit 0aa99eb8456693c015350c5e6c4f442ebe912f77 From db08a928dd1c124c3feeaf6804f6d51330563248 Mon Sep 17 00:00:00 2001 From: Yash Date: Sat, 4 Feb 2023 04:29:35 +0530 Subject: [PATCH 08/18] getter for cancelled payments --- contracts/airdrop/AirdropERC1155.sol | 34 +++++++++++++++ contracts/airdrop/AirdropERC20.sol | 62 +++++++++++++++++----------- contracts/airdrop/AirdropERC721.sol | 34 +++++++++++++++ 3 files changed, 106 insertions(+), 24 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index aca0fd896..49dd724cb 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -250,6 +250,40 @@ contract AirdropERC1155 is } } + /// @notice Returns all airdrop payments cancelled. + function getAllAirdropPaymentsCancelled(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + uint256 processed = processedCount; + if (startId >= processed) { + return contents; + } + + if (endId >= processed) { + endId = processed - 1; + } + + uint256 count; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + count += 1; + } + } + + contents = new AirdropContent[](count); + uint256 index; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + contents[index++] = airdropContent[i]; + } + } + } + /*/////////////////////////////////////////////////////////////// Miscellaneous //////////////////////////////////////////////////////////////*/ diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 3e3c23fda..487e50e36 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -128,32 +128,10 @@ contract AirdropERC20 is ); if (!success) { - // Track failure + indicesOfFailed.push(i); + success = false; } - // if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { - // // solhint-disable avoid-low-level-calls - // // slither-disable-next-line low-level-calls - // (success, ) = content.recipient.call{ value: content.amount }(""); - // } else { - // try - // IERC20(content.tokenAddress).transferFrom(content.tokenOwner, content.recipient, content.amount) - // returns (bool _success) { - // success = _success; - // } catch { - // // revert if failure is due to insufficient allowance - // require( - // IERC20(content.tokenAddress).balanceOf(content.tokenOwner) >= content.amount && - // IERC20(content.tokenAddress).allowance(content.tokenOwner, address(this)) >= content.amount, - // "Not balance or allowance" - // ); - - // // record and continue for all other failures, likely originating from recipient accounts - // indicesOfFailed.push(i); - // success = false; - // } - // } - emit AirdropPayment(content.recipient, content, !success); } } @@ -273,6 +251,40 @@ contract AirdropERC20 is } } + /// @notice Returns all airdrop payments cancelled. + function getAllAirdropPaymentsCancelled(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + uint256 processed = processedCount; + if (startId >= processed) { + return contents; + } + + if (endId >= processed) { + endId = processed - 1; + } + + uint256 count; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + count += 1; + } + } + + contents = new AirdropContent[](count); + uint256 index; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + contents[index++] = airdropContent[i]; + } + } + } + /*/////////////////////////////////////////////////////////////// Miscellaneous //////////////////////////////////////////////////////////////*/ @@ -290,6 +302,8 @@ contract AirdropERC20 is } if (_currency == CurrencyTransferLib.NATIVE_TOKEN) { + // solhint-disable avoid-low-level-calls + // slither-disable-next-line low-level-calls (success, ) = _to.call{ value: _amount }(""); } else { try IERC20(_currency).transferFrom(_from, _to, _amount) returns (bool success_) { diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index ad2766d74..007d13176 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -243,6 +243,40 @@ contract AirdropERC721 is } } + /// @notice Returns all airdrop payments cancelled. + function getAllAirdropPaymentsCancelled(uint256 startId, uint256 endId) + external + view + returns (AirdropContent[] memory contents) + { + require(startId <= endId && endId < payeeCount, "invalid range"); + uint256 processed = processedCount; + if (startId >= processed) { + return contents; + } + + if (endId >= processed) { + endId = processed - 1; + } + + uint256 count; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + count += 1; + } + } + + contents = new AirdropContent[](count); + uint256 index; + + for (uint256 i = startId; i <= endId; i += 1) { + if (isCancelled[i]) { + contents[index++] = airdropContent[i]; + } + } + } + /*/////////////////////////////////////////////////////////////// Miscellaneous //////////////////////////////////////////////////////////////*/ From fc068ba9ed7d034544d76deb07c8296046b2f26d Mon Sep 17 00:00:00 2001 From: Yash Date: Sat, 4 Feb 2023 07:36:40 +0530 Subject: [PATCH 09/18] tests --- contracts/airdrop/AirdropERC1155.sol | 14 +- contracts/airdrop/AirdropERC20.sol | 47 ++- contracts/airdrop/AirdropERC721.sol | 19 +- .../interfaces/airdrop/IAirdropERC20.sol | 2 +- lib/forge-std | 2 +- src/test/airdrop/AirdropERC1155.t.sol | 207 ++++++++++- src/test/airdrop/AirdropERC20.t.sol | 334 ++++++++++++++++-- src/test/airdrop/AirdropERC721.t.sol | 217 +++++++++++- 8 files changed, 779 insertions(+), 63 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index 49dd724cb..25e98370a 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -157,6 +157,14 @@ contract AirdropERC1155 is "" ) {} catch { + // revert if failure is due to unapproved tokens + require( + IERC1155(_contents[i].tokenAddress).balanceOf(_contents[i].tokenOwner, _contents[i].tokenId) >= + _contents[i].amount && + IERC1155(_contents[i].tokenAddress).isApprovedForAll(_contents[i].tokenOwner, address(this)), + "Not balance or approved" + ); + failed = true; } @@ -192,10 +200,14 @@ contract AirdropERC1155 is require(startId <= endId && endId < payeeCount, "invalid range"); uint256 processed = processedCount; + if (processed == payeeCount) { + return contents; + } + if (startId < processed) { startId = processed; } - contents = new AirdropContent[](endId - startId); + contents = new AirdropContent[](endId - startId + 1); uint256 idx; for (uint256 i = startId; i <= endId; i += 1) { diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 487e50e36..2b0648039 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -96,15 +96,27 @@ contract AirdropERC20 is } ///@notice Lets contract-owner cancel any pending payments. - function resetRecipients() external onlyRole(DEFAULT_ADMIN_ROLE) { + function resetRecipients() external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; + uint256 nativeTokenAmount; // set processedCount to payeeCount -- ignore all pending payments. processedCount = payeeCount; for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { + AirdropContent memory content = airdropContent[i]; + isCancelled[i] = true; + + if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { + nativeTokenAmount += content.amount; + } + } + + if (nativeTokenAmount > 0) { + // refund amount to contract admin address + CurrencyTransferLib.safeTransferNativeToken(msg.sender, nativeTokenAmount); } } @@ -112,6 +124,7 @@ contract AirdropERC20 is function processPayments(uint256 paymentsToProcess) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 totalPayees = payeeCount; uint256 countOfProcessed = processedCount; + uint256 nativeTokenAmount; require(countOfProcessed + paymentsToProcess <= totalPayees, "invalid no. of payments"); @@ -129,11 +142,21 @@ contract AirdropERC20 is if (!success) { indicesOfFailed.push(i); + + if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { + nativeTokenAmount += content.amount; + } + success = false; } emit AirdropPayment(content.recipient, content, !success); } + + if (nativeTokenAmount > 0) { + // refund failed payments' amount to contract admin address + CurrencyTransferLib.safeTransferNativeToken(msg.sender, nativeTokenAmount); + } } /** @@ -146,9 +169,10 @@ contract AirdropERC20 is function airdrop(AirdropContent[] calldata _contents) external payable nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; uint256 nativeTokenAmount; + uint256 refundAmount; for (uint256 i = 0; i < len; i++) { - CurrencyTransferLib.transferCurrency( + bool success = _transferCurrencyWithReturnVal( _contents[i].tokenAddress, _contents[i].tokenOwner, _contents[i].recipient, @@ -157,12 +181,21 @@ contract AirdropERC20 is if (_contents[i].tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { nativeTokenAmount += _contents[i].amount; + + if (!success) { + refundAmount += _contents[i].amount; + } } - emit StatelessAirdrop(_contents[i].recipient, _contents[i]); + emit StatelessAirdrop(_contents[i].recipient, _contents[i], !success); } require(nativeTokenAmount == msg.value, "Incorrect native token amount"); + + if (refundAmount > 0) { + // refund failed payments' amount to contract admin address + CurrencyTransferLib.safeTransferNativeToken(msg.sender, refundAmount); + } } /*/////////////////////////////////////////////////////////////// @@ -193,10 +226,14 @@ contract AirdropERC20 is require(startId <= endId && endId < payeeCount, "invalid range"); uint256 processed = processedCount; + if (processed == payeeCount) { + return contents; + } + if (startId < processed) { startId = processed; } - contents = new AirdropContent[](endId - startId); + contents = new AirdropContent[](endId - startId + 1); uint256 idx; for (uint256 i = startId; i <= endId; i += 1) { @@ -312,7 +349,7 @@ contract AirdropERC20 is require( IERC20(_currency).balanceOf(_from) >= _amount && IERC20(_currency).allowance(_from, address(this)) >= _amount, - "CurrencyTransferBal: insufficient balance or allowance." + "Not balance or allowance" ); success = false; diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 007d13176..1ebe2ebd3 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -150,6 +150,14 @@ contract AirdropERC721 is _contents[i].tokenId ) {} catch { + // revert if failure is due to unapproved tokens + require( + (IERC721(_contents[i].tokenAddress).ownerOf(_contents[i].tokenId) == _contents[i].tokenOwner && + address(this) == IERC721(_contents[i].tokenAddress).getApproved(_contents[i].tokenId)) || + IERC721(_contents[i].tokenAddress).isApprovedForAll(_contents[i].tokenOwner, address(this)), + "Not owner or approved" + ); + failed = true; } @@ -185,15 +193,18 @@ contract AirdropERC721 is require(startId <= endId && endId < payeeCount, "invalid range"); uint256 processed = processedCount; + if (processed == payeeCount) { + return contents; + } + if (startId < processed) { startId = processed; } - contents = new AirdropContent[](endId - startId); + contents = new AirdropContent[](endId - startId + 1); - uint256 idx; + uint256 index; for (uint256 i = startId; i <= endId; i += 1) { - contents[idx] = airdropContent[i]; - idx += 1; + contents[index++] = airdropContent[i]; } } diff --git a/contracts/interfaces/airdrop/IAirdropERC20.sol b/contracts/interfaces/airdrop/IAirdropERC20.sol index 843d60c32..25a1f9cfc 100644 --- a/contracts/interfaces/airdrop/IAirdropERC20.sol +++ b/contracts/interfaces/airdrop/IAirdropERC20.sol @@ -17,7 +17,7 @@ interface IAirdropERC20 { /// @notice Emitted when an airdrop payment is made to a recipient. event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. - event StatelessAirdrop(address indexed recipient, AirdropContent content); + event StatelessAirdrop(address indexed recipient, AirdropContent content, bool failed); /** * @notice Details of amount and recipient for airdropped token. diff --git a/lib/forge-std b/lib/forge-std index 0aa99eb84..4a79aca83 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 0aa99eb8456693c015350c5e6c4f442ebe912f77 +Subproject commit 4a79aca83f8075f8b1b4fe9153945fef08375630 diff --git a/src/test/airdrop/AirdropERC1155.t.sol b/src/test/airdrop/AirdropERC1155.t.sol index b66f17b6b..f08009045 100644 --- a/src/test/airdrop/AirdropERC1155.t.sol +++ b/src/test/airdrop/AirdropERC1155.t.sol @@ -12,7 +12,11 @@ contract AirdropERC1155Test is BaseTest { Wallet internal tokenOwner; - IAirdropERC1155.AirdropContent[] internal _contents; + IAirdropERC1155.AirdropContent[] internal _contentsOne; + IAirdropERC1155.AirdropContent[] internal _contentsTwo; + + uint256 countOne; + uint256 countTwo; function setUp() public override { super.setUp(); @@ -29,8 +33,23 @@ contract AirdropERC1155Test is BaseTest { tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), true); - for (uint256 i = 0; i < 1000; i++) { - _contents.push( + countOne = 1000; + countTwo = 200; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne.push( + IAirdropERC1155.AirdropContent({ + tokenAddress: address(erc1155), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + tokenId: i % 5, + amount: 5 + }) + ); + } + + for (uint256 i = countOne; i < countOne + countTwo; i++) { + _contentsTwo.push( IAirdropERC1155.AirdropContent({ tokenAddress: address(erc1155), tokenOwner: address(tokenOwner), @@ -43,18 +62,183 @@ contract AirdropERC1155Test is BaseTest { } /*/////////////////////////////////////////////////////////////// - Unit tests: `createPack` + Unit tests: `processPayments` //////////////////////////////////////////////////////////////*/ - function test_state_airdrop() public { + function test_state_processPayments_full() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < countOne; i++) { + assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); + } + assertEq(erc1155.balanceOf(address(tokenOwner), 0), 0); + assertEq(erc1155.balanceOf(address(tokenOwner), 1), 1000); + assertEq(erc1155.balanceOf(address(tokenOwner), 2), 2000); + assertEq(erc1155.balanceOf(address(tokenOwner), 3), 3000); + assertEq(erc1155.balanceOf(address(tokenOwner), 4), 4000); + } + + function test_state_processPayments_partial() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); + } + } + + function test_revert_processPayments_notOwner() public { + vm.prank(address(25)); + vm.expectRevert( + abi.encodePacked( + "Permissions: account ", + TWStrings.toHexString(uint160(address(25)), 20), + " is missing role ", + TWStrings.toHexString(uint256(0x00), 32) + ) + ); + drop.addRecipients(_contentsOne); + } + + function test_revert_processPayments_notApproved() public { + tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); + vm.startPrank(deployer); - drop.addRecipients(_contents); - drop.processPayments(_contents.length); + drop.addRecipients(_contentsOne); + vm.expectRevert("Not balance or approved"); + drop.processPayments(_contentsOne.length); vm.stopPrank(); + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: `reset` + //////////////////////////////////////////////////////////////*/ + + function test_state_resetRecipients() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count - for (uint256 i = 0; i < 1000; i++) { - assertEq(erc1155.balanceOf(_contents[i].recipient, i % 5), 5); + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); } + } + + function test_state_resetRecipients_addMore() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + + // add more recipients + vm.prank(deployer); + drop.addRecipients(_contentsTwo); + + // check state + assertEq(drop.getAllAirdropPayments(0, countOne + countTwo - 1).length, countOne + countTwo); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne + countTwo - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne + countTwo - 1).length, countTwo); // pending payments equal to count of new recipients added + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne + countTwo - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne + countTwo); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); + } + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: stateless airdrop + //////////////////////////////////////////////////////////////*/ + + function test_state_airdrop() public { + vm.prank(deployer); + drop.airdrop(_contentsOne); + + for (uint256 i = 0; i < countOne; i++) { + assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); + } + assertEq(erc1155.balanceOf(address(tokenOwner), 0), 0); assertEq(erc1155.balanceOf(address(tokenOwner), 1), 1000); assertEq(erc1155.balanceOf(address(tokenOwner), 2), 2000); @@ -72,16 +256,15 @@ contract AirdropERC1155Test is BaseTest { TWStrings.toHexString(uint256(0x00), 32) ) ); - drop.addRecipients(_contents); + drop.airdrop(_contentsOne); } function test_revert_airdrop_notApproved() public { tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), false); vm.startPrank(deployer); - drop.addRecipients(_contents); vm.expectRevert("Not balance or approved"); - drop.processPayments(_contents.length); + drop.airdrop(_contentsOne); vm.stopPrank(); } } diff --git a/src/test/airdrop/AirdropERC20.t.sol b/src/test/airdrop/AirdropERC20.t.sol index 4b4782090..9290a5565 100644 --- a/src/test/airdrop/AirdropERC20.t.sol +++ b/src/test/airdrop/AirdropERC20.t.sol @@ -12,7 +12,11 @@ contract AirdropERC20Test is BaseTest { Wallet internal tokenOwner; - IAirdropERC20.AirdropContent[] internal _contents; + IAirdropERC20.AirdropContent[] internal _contentsOne; + IAirdropERC20.AirdropContent[] internal _contentsTwo; + + uint256 countOne; + uint256 countTwo; function setUp() public override { super.setUp(); @@ -24,8 +28,22 @@ contract AirdropERC20Test is BaseTest { erc20.mint(address(tokenOwner), 10_000 ether); tokenOwner.setAllowanceERC20(address(erc20), address(drop), type(uint256).max); - for (uint256 i = 0; i < 1000; i++) { - _contents.push( + countOne = 1000; + countTwo = 200; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne.push( + IAirdropERC20.AirdropContent({ + tokenAddress: address(erc20), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + amount: 10 ether + }) + ); + } + + for (uint256 i = countOne; i < countOne + countTwo; i++) { + _contentsTwo.push( IAirdropERC20.AirdropContent({ tokenAddress: address(erc20), tokenOwner: address(tokenOwner), @@ -37,56 +55,319 @@ contract AirdropERC20Test is BaseTest { } /*/////////////////////////////////////////////////////////////// - Unit tests: `createPack` + Unit tests: `processPayments` //////////////////////////////////////////////////////////////*/ - function test_state_airdrop() public { - vm.startPrank(deployer); - drop.addRecipients(_contents); - drop.processPayments(_contents.length); - vm.stopPrank(); + function test_state_processPayments_full() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); - for (uint256 i = 0; i < 1000; i++) { - assertEq(erc20.balanceOf(_contents[i].recipient), _contents[i].amount); + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < countOne; i++) { + assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); } assertEq(erc20.balanceOf(address(tokenOwner)), 0); } - function test_state_airdrop_nativeToken() public { + function test_state_processPayments_partial() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); + } + assertEq(erc20.balanceOf(address(tokenOwner)), 3000 ether); + } + + function test_state_processPayments_nativeToken_full() public { vm.deal(deployer, 10_000 ether); uint256 balBefore = deployer.balance; - for (uint256 i = 0; i < 1000; i++) { - _contents[i].tokenAddress = NATIVE_TOKEN; + for (uint256 i = 0; i < countOne; i++) { + _contentsOne[i].tokenAddress = NATIVE_TOKEN; } - vm.startPrank(deployer); - drop.addRecipients{ value: 10_000 ether }(_contents); - drop.processPayments(_contents.length); - vm.stopPrank(); + vm.prank(deployer); + drop.addRecipients{ value: 10_000 ether }(_contentsOne); - for (uint256 i = 0; i < 1000; i++) { - assertEq(_contents[i].recipient.balance, _contents[i].amount); + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < countOne; i++) { + assertEq(_contentsOne[i].recipient.balance, _contentsOne[i].amount); + } + assertEq(deployer.balance, balBefore - 10_000 ether); + } + + function test_state_processPayments_nativeToken_partial() public { + vm.deal(deployer, 10_000 ether); + + uint256 balBefore = deployer.balance; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne[i].tokenAddress = NATIVE_TOKEN; + } + + vm.prank(deployer); + drop.addRecipients{ value: 10_000 ether }(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(_contentsOne[i].recipient.balance, _contentsOne[i].amount); } assertEq(deployer.balance, balBefore - 10_000 ether); } - function test_revert_airdrop_incorrectNativeTokenAmt() public { + function test_revert_processPayments_incorrectNativeTokenAmt() public { vm.deal(deployer, 11_000 ether); uint256 incorrectAmt = 10_000 ether + 1; for (uint256 i = 0; i < 1000; i++) { - _contents[i].tokenAddress = NATIVE_TOKEN; + _contentsOne[i].tokenAddress = NATIVE_TOKEN; } vm.prank(deployer); vm.expectRevert("Incorrect native token amount"); - drop.addRecipients{ value: incorrectAmt }(_contents); + drop.addRecipients{ value: incorrectAmt }(_contentsOne); + } + + function test_revert_processPayments_notAdmin() public { + vm.prank(address(25)); + vm.expectRevert( + abi.encodePacked( + "Permissions: account ", + TWStrings.toHexString(uint160(address(25)), 20), + " is missing role ", + TWStrings.toHexString(uint256(0x00), 32) + ) + ); + drop.addRecipients(_contentsOne); + } + + function test_revert_processPayments_notApproved() public { + tokenOwner.setAllowanceERC20(address(erc20), address(drop), 0); + + vm.startPrank(deployer); + drop.addRecipients(_contentsOne); + vm.expectRevert("Not balance or allowance"); + drop.processPayments(_contentsOne.length); + vm.stopPrank(); + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: `reset` + //////////////////////////////////////////////////////////////*/ + + function test_state_resetRecipients() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); + } + assertEq(erc20.balanceOf(address(tokenOwner)), 3000 ether); + } + + function test_state_resetRecipients_addMore() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + + // add more recipients + vm.prank(deployer); + drop.addRecipients(_contentsTwo); + + // check state + assertEq(drop.getAllAirdropPayments(0, countOne + countTwo - 1).length, countOne + countTwo); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne + countTwo - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne + countTwo - 1).length, countTwo); // pending payments equal to count of new recipients added + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne + countTwo - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne + countTwo); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); + } + assertEq(erc20.balanceOf(address(tokenOwner)), 3000 ether); + } + + function test_state_resetRecipients_nativeToken() public { + vm.deal(deployer, 10_000 ether); + + uint256 balBefore = deployer.balance; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne[i].tokenAddress = NATIVE_TOKEN; + } + + vm.prank(deployer); + drop.addRecipients{ value: 10_000 ether }(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + + for (uint256 i = 0; i < countOne - 300; i++) { + assertEq(_contentsOne[i].recipient.balance, _contentsOne[i].amount); + } + assertEq(deployer.balance, balBefore - 7_000 ether); // native token amount gets refunded + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: stateless airdrop + //////////////////////////////////////////////////////////////*/ + + function test_state_airdrop() public { + vm.prank(deployer); + drop.airdrop(_contentsOne); + + for (uint256 i = 0; i < countOne; i++) { + assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); + } + assertEq(erc20.balanceOf(address(tokenOwner)), 0); } - function test_revert_airdrop_notAdmin() public { + function test_revert_airdrop_notOwner() public { vm.prank(address(25)); vm.expectRevert( abi.encodePacked( @@ -96,16 +377,15 @@ contract AirdropERC20Test is BaseTest { TWStrings.toHexString(uint256(0x00), 32) ) ); - drop.addRecipients(_contents); + drop.airdrop(_contentsOne); } function test_revert_airdrop_notApproved() public { tokenOwner.setAllowanceERC20(address(erc20), address(drop), 0); vm.startPrank(deployer); - drop.addRecipients(_contents); vm.expectRevert("Not balance or allowance"); - drop.processPayments(_contents.length); + drop.airdrop(_contentsOne); vm.stopPrank(); } } diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index 251d1ea4a..77e680b57 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -12,7 +12,11 @@ contract AirdropERC721Test is BaseTest { Wallet internal tokenOwner; - IAirdropERC721.AirdropContent[] internal _contents; + IAirdropERC721.AirdropContent[] internal _contentsOne; + IAirdropERC721.AirdropContent[] internal _contentsTwo; + + uint256 countOne; + uint256 countTwo; function setUp() public override { super.setUp(); @@ -21,11 +25,25 @@ contract AirdropERC721Test is BaseTest { tokenOwner = getWallet(); - erc721.mint(address(tokenOwner), 1000); + erc721.mint(address(tokenOwner), 1500); tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), true); - for (uint256 i = 0; i < 1000; i++) { - _contents.push( + countOne = 1000; + countTwo = 200; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne.push( + IAirdropERC721.AirdropContent({ + tokenAddress: address(erc721), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + tokenId: i + }) + ); + } + + for (uint256 i = countOne; i < countOne + countTwo; i++) { + _contentsTwo.push( IAirdropERC721.AirdropContent({ tokenAddress: address(erc721), tokenOwner: address(tokenOwner), @@ -37,17 +55,193 @@ contract AirdropERC721Test is BaseTest { } /*/////////////////////////////////////////////////////////////// - Unit tests: `createPack` + Unit tests: `processPayments` //////////////////////////////////////////////////////////////*/ - function test_state_airdrop() public { + function test_state_processPayments_full() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(_contentsOne.length); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < 1000; i++) { + assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); + } + } + + function test_state_processPayments_partial() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + for (uint256 i = 0; i < 700; i++) { + assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); + } + + for (uint256 i = 700; i < 1000; i++) { + assertEq(erc721.ownerOf(i), address(tokenOwner)); + } + } + + function test_revert_processPayments_notOwner() public { + vm.prank(address(25)); + vm.expectRevert( + abi.encodePacked( + "Permissions: account ", + TWStrings.toHexString(uint160(address(25)), 20), + " is missing role ", + TWStrings.toHexString(uint256(0x00), 32) + ) + ); + drop.addRecipients(_contentsOne); + + vm.prank(deployer); + drop.addRecipients(_contentsOne); + vm.expectRevert( + abi.encodePacked( + "Permissions: account ", + TWStrings.toHexString(uint160(address(25)), 20), + " is missing role ", + TWStrings.toHexString(uint256(0x00), 32) + ) + ); + vm.prank(address(25)); + drop.processPayments(countOne); + } + + function test_revert_processPayments_notApproved() public { + tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); + vm.startPrank(deployer); - drop.addRecipients(_contents); - drop.processPayments(_contents.length); + drop.addRecipients(_contentsOne); + vm.expectRevert("Not owner or approved"); + drop.processPayments(_contentsOne.length); vm.stopPrank(); + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: `reset` + //////////////////////////////////////////////////////////////*/ + + function test_state_resetRecipients() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // check state before airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), 0); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + + for (uint256 i = 0; i < 700; i++) { + assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); + } + } + + function test_state_resetRecipients_addMore() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + // perform airdrop + vm.prank(deployer); + drop.processPayments(countOne - 300); + + // do a reset + vm.prank(deployer); + drop.resetRecipients(); + + // check state after reset + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + + // add more recipients + vm.prank(deployer); + drop.addRecipients(_contentsTwo); + + // check state + assertEq(drop.getAllAirdropPayments(0, countOne + countTwo - 1).length, countOne + countTwo); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne + countTwo - 1).length, countOne - 300); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne + countTwo - 1).length, countTwo); // pending payments equal to count of new recipients added + assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne + countTwo - 1).length, 300); // cancelled payments + assertEq(drop.payeeCount(), countOne + countTwo); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < 700; i++) { + assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); + } + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: stateless airdrop + //////////////////////////////////////////////////////////////*/ + + function test_state_airdrop() public { + vm.prank(deployer); + drop.airdrop(_contentsOne); for (uint256 i = 0; i < 1000; i++) { - assertEq(erc721.ownerOf(i), _contents[i].recipient); + assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); } } @@ -61,16 +255,15 @@ contract AirdropERC721Test is BaseTest { TWStrings.toHexString(uint256(0x00), 32) ) ); - drop.addRecipients(_contents); + drop.airdrop(_contentsOne); } function test_revert_airdrop_notApproved() public { tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), false); vm.startPrank(deployer); - drop.addRecipients(_contents); vm.expectRevert("Not owner or approved"); - drop.processPayments(_contents.length); + drop.airdrop(_contentsOne); vm.stopPrank(); } } From 6c7b7df55b0012462e549189c8f509d36548537c Mon Sep 17 00:00:00 2001 From: Yash Date: Tue, 28 Feb 2023 21:01:14 +0530 Subject: [PATCH 10/18] [H-1] addRecipients adds empty airdrops on subsequent calls --- contracts/airdrop/AirdropERC1155.sol | 4 +- contracts/airdrop/AirdropERC20.sol | 4 +- contracts/airdrop/AirdropERC721.sol | 4 +- src/test/airdrop/AirdropERC721.t.sol | 79 ++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 6 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index 25e98370a..2f1e5ef39 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -78,8 +78,8 @@ contract AirdropERC1155 is uint256 currentCount = payeeCount; payeeCount += len; - for (uint256 i = currentCount; i < len; i += 1) { - airdropContent[i] = _contents[i]; + for (uint256 i = 0; i < len; i += 1) { + airdropContent[i + currentCount] = _contents[i]; } emit RecipientsAdded(_contents); diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 2b0648039..86267eb2c 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -82,8 +82,8 @@ contract AirdropERC20 is uint256 nativeTokenAmount; - for (uint256 i = currentCount; i < len; i += 1) { - airdropContent[i] = _contents[i]; + for (uint256 i = 0; i < len; i += 1) { + airdropContent[i + currentCount] = _contents[i]; if (_contents[i].tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { nativeTokenAmount += _contents[i].amount; diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 1ebe2ebd3..0a7bc1c81 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -78,8 +78,8 @@ contract AirdropERC721 is uint256 currentCount = payeeCount; payeeCount += len; - for (uint256 i = currentCount; i < len; i += 1) { - airdropContent[i] = _contents[i]; + for (uint256 i = 0; i < len; i += 1) { + airdropContent[i + currentCount] = _contents[i]; } emit RecipientsAdded(_contents); diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index 77e680b57..de5ad13ea 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -267,3 +267,82 @@ contract AirdropERC721Test is BaseTest { vm.stopPrank(); } } + +contract AirdropERC721AuditTest is BaseTest { + AirdropERC721 internal drop; + + Wallet internal tokenOwner; + + IAirdropERC721.AirdropContent[] internal _contentsOne; + IAirdropERC721.AirdropContent[] internal _contentsTwo; + + uint256 countOne; + uint256 countTwo; + + function setUp() public override { + super.setUp(); + + drop = AirdropERC721(getContract("AirdropERC721")); + + tokenOwner = getWallet(); + + erc721.mint(address(tokenOwner), 1500); + tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), true); + + countOne = 1000; + countTwo = 200; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne.push( + IAirdropERC721.AirdropContent({ + tokenAddress: address(erc721), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + tokenId: i + }) + ); + } + + for (uint256 i = countOne; i < countOne + countTwo; i++) { + _contentsTwo.push( + IAirdropERC721.AirdropContent({ + tokenAddress: address(erc721), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + tokenId: i + }) + ); + } + } + + function test_NewRecipientsAreEmptyPreventingAirdrops() public { + // create a memory array of one as the recipient array to use for this test + IAirdropERC721.AirdropContent[] memory _c = new IAirdropERC721.AirdropContent[](1); + _c[0] = _contentsOne[0]; + // add recipients the first time + vm.prank(deployer); + drop.addRecipients(_c); + // everything should be normal at this point + assertEq(drop.payeeCount(), 1); + assertEq(drop.getAllAirdropPayments(0, 0).length, 1); + assertEq(drop.payeeCount(), 1); + // grab another recipient + _c[0] = _contentsOne[1]; + // add this new one, this is where the issues occur + vm.prank(deployer); + drop.addRecipients(_c); + // payee count is correct, everything seems fine + assertEq(drop.payeeCount(), 2); + // get all the airdrop payments to double check + IAirdropERC721.AirdropContent[] memory _res = drop.getAllAirdropPayments(0, 1); + // length seems fine + assertEq(_res.length, 2); + // first entry is correct + assertEq(_res[0].tokenAddress, _contentsOne[0].tokenAddress); + assertEq(_res[1].tokenAddress, _contentsOne[1].tokenAddress); + assertEq(_res[1].tokenAddress == _contentsOne[1].tokenAddress, true); + + vm.prank(deployer); + drop.processPayments(2); + } +} From 82445bbbe65dc669b6307acf349ec694c0380c2a Mon Sep 17 00:00:00 2001 From: Yash Date: Tue, 28 Feb 2023 21:34:32 +0530 Subject: [PATCH 11/18] [M-1] Fails for non-compliant ERC20 tokens --- contracts/airdrop/AirdropERC20.sol | 8 +- src/test/airdrop/AirdropERC20.t.sol | 73 ++++++++++++++++ src/test/mocks/MockERC20NonCompliant.sol | 104 +++++++++++++++++++++++ 3 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 src/test/mocks/MockERC20NonCompliant.sol diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 86267eb2c..4837c11ff 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -343,16 +343,14 @@ contract AirdropERC20 is // slither-disable-next-line low-level-calls (success, ) = _to.call{ value: _amount }(""); } else { - try IERC20(_currency).transferFrom(_from, _to, _amount) returns (bool success_) { - success = success_; - } catch { + (success, ) = _currency.call(abi.encodeWithSelector(IERC20.transferFrom.selector, _from, _to, _amount)); + + if (!success) { require( IERC20(_currency).balanceOf(_from) >= _amount && IERC20(_currency).allowance(_from, address(this)) >= _amount, "Not balance or allowance" ); - - success = false; } } } diff --git a/src/test/airdrop/AirdropERC20.t.sol b/src/test/airdrop/AirdropERC20.t.sol index 9290a5565..9cc9d3940 100644 --- a/src/test/airdrop/AirdropERC20.t.sol +++ b/src/test/airdrop/AirdropERC20.t.sol @@ -7,6 +7,8 @@ import { AirdropERC20, IAirdropERC20 } from "contracts/airdrop/AirdropERC20.sol" import { Wallet } from "../utils/Wallet.sol"; import "../utils/BaseTest.sol"; +import "../mocks/MockERC20NonCompliant.sol"; + contract AirdropERC20Test is BaseTest { AirdropERC20 internal drop; @@ -389,3 +391,74 @@ contract AirdropERC20Test is BaseTest { vm.stopPrank(); } } + +contract AirdropERC20AuditTest is BaseTest { + AirdropERC20 internal drop; + + Wallet internal tokenOwner; + + IAirdropERC20.AirdropContent[] internal _contentsOne; + IAirdropERC20.AirdropContent[] internal _contentsTwo; + + uint256 countOne; + uint256 countTwo; + + MockERC20NonCompliant public erc20_nonCompliant; + + function setUp() public override { + super.setUp(); + + erc20_nonCompliant = new MockERC20NonCompliant(); + drop = AirdropERC20(getContract("AirdropERC20")); + + tokenOwner = getWallet(); + + erc20_nonCompliant.mint(address(tokenOwner), 10_000 ether); + tokenOwner.setAllowanceERC20(address(erc20_nonCompliant), address(drop), type(uint256).max); + + countOne = 1000; + countTwo = 200; + + for (uint256 i = 0; i < countOne; i++) { + _contentsOne.push( + IAirdropERC20.AirdropContent({ + tokenAddress: address(erc20_nonCompliant), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + amount: 10 ether + }) + ); + } + + for (uint256 i = countOne; i < countOne + countTwo; i++) { + _contentsTwo.push( + IAirdropERC20.AirdropContent({ + tokenAddress: address(erc20_nonCompliant), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + amount: 10 ether + }) + ); + } + } + + function test_process_payments_with_non_compliant_token() public { + vm.prank(deployer); + drop.addRecipients(_contentsOne); + + vm.prank(deployer); + drop.processPayments(countOne); + + // check state after airdrop + assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); + assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); + assertEq(drop.payeeCount(), countOne); + assertEq(drop.processedCount(), countOne); + + for (uint256 i = 0; i < countOne; i++) { + assertEq(erc20_nonCompliant.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); + } + assertEq(erc20_nonCompliant.balanceOf(address(tokenOwner)), 0); + } +} diff --git a/src/test/mocks/MockERC20NonCompliant.sol b/src/test/mocks/MockERC20NonCompliant.sol new file mode 100644 index 000000000..8bc8889b5 --- /dev/null +++ b/src/test/mocks/MockERC20NonCompliant.sol @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +contract MockERC20NonCompliant { + mapping(address => uint256) private _balances; + mapping(address => mapping(address => uint256)) private _allowances; + uint256 private _totalSupply; + + constructor() {} + + function decimals() public view virtual returns (uint8) { + return 18; + } + + function totalSupply() public view virtual returns (uint256) { + return _totalSupply; + } + + function balanceOf(address account) public view virtual returns (uint256) { + return _balances[account]; + } + + function allowance(address owner, address spender) public view virtual returns (uint256) { + return _allowances[owner][spender]; + } + + function approve(address spender, uint256 amount) public virtual returns (bool) { + address owner = msg.sender; + _approve(owner, spender, amount); + return true; + } + + // non-compliant ERC20 as transfer doesn't return a bool + function transfer(address to, uint256 amount) public virtual { + address owner = msg.sender; + _transfer(owner, to, amount); + } + + // non-compliant ERC20 as transferFrom doesn't return a bool + function transferFrom( + address from, + address to, + uint256 amount + ) public { + address spender = msg.sender; + _spendAllowance(from, spender, amount); + _transfer(from, to, amount); + } + + function _transfer( + address from, + address to, + uint256 amount + ) internal virtual { + require(from != address(0), "ERC20: transfer from the zero address"); + require(to != address(0), "ERC20: transfer to the zero address"); + + uint256 fromBalance = _balances[from]; + require(fromBalance >= amount, "ERC20: transfer amount exceeds balance"); + unchecked { + _balances[from] = fromBalance - amount; + } + _balances[to] += amount; + } + + function _mint(address account, uint256 amount) internal virtual { + require(account != address(0), "ERC20: mint to the zero address"); + + _totalSupply += amount; + unchecked { + // Overflow not possible: balance + amount is at most totalSupply + amount, which is checked above. + _balances[account] += amount; + } + } + + function mint(address to, uint256 amount) public { + _mint(to, amount); + } + + function _approve( + address owner, + address spender, + uint256 amount + ) internal virtual { + require(owner != address(0), "ERC20: approve from the zero address"); + require(spender != address(0), "ERC20: approve to the zero address"); + + _allowances[owner][spender] = amount; + } + + function _spendAllowance( + address owner, + address spender, + uint256 amount + ) internal virtual { + uint256 currentAllowance = allowance(owner, spender); + if (currentAllowance != type(uint256).max) { + require(currentAllowance >= amount, "ERC20: insufficient allowance"); + unchecked { + _approve(owner, spender, currentAllowance - amount); + } + } + } +} From fd85a6a9180bf1a143347ba398bb98afd71f11b2 Mon Sep 17 00:00:00 2001 From: Yash Date: Tue, 28 Feb 2023 22:55:57 +0530 Subject: [PATCH 12/18] [Q-1] Unused library SafeERC20 --- contracts/airdrop/AirdropERC20.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 4837c11ff..2fe8708b7 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -10,7 +10,7 @@ import "@openzeppelin/contracts-upgradeable/utils/MulticallUpgradeable.sol"; import "../interfaces/airdrop/IAirdropERC20.sol"; import { CurrencyTransferLib } from "../lib/CurrencyTransferLib.sol"; -import "../openzeppelin-presets/token/ERC20/utils/SafeERC20.sol"; +import "../eip/interface/IERC20.sol"; // ========== Features ========== import "../extension/Ownable.sol"; @@ -24,7 +24,6 @@ contract AirdropERC20 is MulticallUpgradeable, IAirdropERC20 { - using SafeERC20 for IERC20; /*/////////////////////////////////////////////////////////////// State variables //////////////////////////////////////////////////////////////*/ From d222049d4d46f1916455fbb08579b9d070a42feb Mon Sep 17 00:00:00 2001 From: Yash Date: Wed, 1 Mar 2023 03:01:43 +0530 Subject: [PATCH 13/18] [M-2] resetRecipients() can exceed block gas limit --- contracts/airdrop/AirdropERC1155.sol | 91 +++--------------- contracts/airdrop/AirdropERC20.sol | 93 ++++--------------- contracts/airdrop/AirdropERC721.sol | 91 +++--------------- .../interfaces/airdrop/IAirdropERC1155.sol | 20 ++-- .../interfaces/airdrop/IAirdropERC20.sol | 20 ++-- .../interfaces/airdrop/IAirdropERC721.sol | 20 ++-- src/test/airdrop/AirdropERC1155.t.sol | 47 ++++++---- src/test/airdrop/AirdropERC20.t.sol | 67 ++++++------- src/test/airdrop/AirdropERC721.t.sol | 47 ++++++---- 9 files changed, 178 insertions(+), 318 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index 2f1e5ef39..13fee255a 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -37,7 +37,7 @@ contract AirdropERC1155 is mapping(uint256 => AirdropContent) private airdropContent; - mapping(uint256 => bool) private isCancelled; + CancelledPayments[] public cancelledPaymentIndices; /*/////////////////////////////////////////////////////////////// Constructor + initializer logic @@ -86,16 +86,20 @@ contract AirdropERC1155 is } ///@notice Lets contract-owner cancel any pending payments. - function resetRecipients() external onlyRole(DEFAULT_ADMIN_ROLE) { - uint256 totalPayees = payeeCount; + function cancelPendingPayments(uint256 numberOfPaymentsToCancel) external onlyRole(DEFAULT_ADMIN_ROLE) { uint256 countOfProcessed = processedCount; - // set processedCount to payeeCount -- ignore all pending payments. - processedCount = payeeCount; + // increase processedCount by the specified count -- all pending payments in between will be treated as cancelled. + uint256 newProcessedCount = countOfProcessed + numberOfPaymentsToCancel; + require(newProcessedCount <= payeeCount, "Exceeds total payees."); + processedCount = newProcessedCount; - for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { - isCancelled[i] = true; - } + CancelledPayments memory range = CancelledPayments({ + startIndex: countOfProcessed, + endIndex: newProcessedCount - 1 + }); + + cancelledPaymentIndices.push(range); } /// @notice Lets contract-owner send ERC721 NFTs to a list of addresses. @@ -216,42 +220,6 @@ contract AirdropERC1155 is } } - /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents) - { - require(startId <= endId && endId < payeeCount, "invalid range"); - uint256 processed = processedCount; - if (startId >= processed) { - return contents; - } - - if (endId >= processed) { - endId = processed - 1; - } - - uint256 count; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - continue; - } - count += 1; - } - - contents = new AirdropContent[](count); - uint256 index; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - continue; - } - contents[index++] = airdropContent[i]; - } - } - /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents) { uint256 count = indicesOfFailed.length; @@ -262,38 +230,9 @@ contract AirdropERC1155 is } } - /// @notice Returns all airdrop payments cancelled. - function getAllAirdropPaymentsCancelled(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents) - { - require(startId <= endId && endId < payeeCount, "invalid range"); - uint256 processed = processedCount; - if (startId >= processed) { - return contents; - } - - if (endId >= processed) { - endId = processed - 1; - } - - uint256 count; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - count += 1; - } - } - - contents = new AirdropContent[](count); - uint256 index; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - contents[index++] = airdropContent[i]; - } - } + /// @notice Returns all blocks of cancelled payments as an array of index range. + function getCancelledPaymentIndices() external view returns (CancelledPayments[] memory) { + return cancelledPaymentIndices; } /*/////////////////////////////////////////////////////////////// diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 2fe8708b7..ec9d985ef 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -38,7 +38,7 @@ contract AirdropERC20 is mapping(uint256 => AirdropContent) private airdropContent; - mapping(uint256 => bool) private isCancelled; + CancelledPayments[] public cancelledPaymentIndices; /*/////////////////////////////////////////////////////////////// Constructor + initializer logic @@ -95,18 +95,24 @@ contract AirdropERC20 is } ///@notice Lets contract-owner cancel any pending payments. - function resetRecipients() external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { - uint256 totalPayees = payeeCount; + function cancelPendingPayments(uint256 numberOfPaymentsToCancel) external onlyRole(DEFAULT_ADMIN_ROLE) { uint256 countOfProcessed = processedCount; uint256 nativeTokenAmount; - // set processedCount to payeeCount -- ignore all pending payments. - processedCount = payeeCount; + // increase processedCount by the specified count -- all pending payments in between will be treated as cancelled. + uint256 newProcessedCount = countOfProcessed + numberOfPaymentsToCancel; + require(newProcessedCount <= payeeCount, "Exceeds total payees."); + processedCount = newProcessedCount; - for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { - AirdropContent memory content = airdropContent[i]; + CancelledPayments memory range = CancelledPayments({ + startIndex: countOfProcessed, + endIndex: newProcessedCount - 1 + }); + + cancelledPaymentIndices.push(range); - isCancelled[i] = true; + for (uint256 i = countOfProcessed; i < newProcessedCount; i += 1) { + AirdropContent memory content = airdropContent[i]; if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { nativeTokenAmount += content.amount; @@ -241,42 +247,6 @@ contract AirdropERC20 is } } - /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents) - { - require(startId <= endId && endId < payeeCount, "invalid range"); - uint256 processed = processedCount; - if (startId >= processed) { - return contents; - } - - if (endId >= processed) { - endId = processed - 1; - } - - uint256 count; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - continue; - } - count += 1; - } - - contents = new AirdropContent[](count); - uint256 index; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - continue; - } - contents[index++] = airdropContent[i]; - } - } - /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents) { uint256 count = indicesOfFailed.length; @@ -287,38 +257,9 @@ contract AirdropERC20 is } } - /// @notice Returns all airdrop payments cancelled. - function getAllAirdropPaymentsCancelled(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents) - { - require(startId <= endId && endId < payeeCount, "invalid range"); - uint256 processed = processedCount; - if (startId >= processed) { - return contents; - } - - if (endId >= processed) { - endId = processed - 1; - } - - uint256 count; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - count += 1; - } - } - - contents = new AirdropContent[](count); - uint256 index; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - contents[index++] = airdropContent[i]; - } - } + /// @notice Returns all blocks of cancelled payments as an array of index range. + function getCancelledPaymentIndices() external view returns (CancelledPayments[] memory) { + return cancelledPaymentIndices; } /*/////////////////////////////////////////////////////////////// diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 0a7bc1c81..2df12ea5f 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -37,7 +37,7 @@ contract AirdropERC721 is mapping(uint256 => AirdropContent) private airdropContent; - mapping(uint256 => bool) private isCancelled; + CancelledPayments[] public cancelledPaymentIndices; /*/////////////////////////////////////////////////////////////// Constructor + initializer logic @@ -86,16 +86,20 @@ contract AirdropERC721 is } ///@notice Lets contract-owner cancel any pending payments. - function resetRecipients() external onlyRole(DEFAULT_ADMIN_ROLE) { - uint256 totalPayees = payeeCount; + function cancelPendingPayments(uint256 numberOfPaymentsToCancel) external onlyRole(DEFAULT_ADMIN_ROLE) { uint256 countOfProcessed = processedCount; - // set processedCount to payeeCount -- ignore all pending payments. - processedCount = payeeCount; + // increase processedCount by the specified count -- all pending payments in between will be treated as cancelled. + uint256 newProcessedCount = countOfProcessed + numberOfPaymentsToCancel; + require(newProcessedCount <= payeeCount, "Exceeds total payees."); + processedCount = newProcessedCount; - for (uint256 i = countOfProcessed; i < totalPayees; i += 1) { - isCancelled[i] = true; - } + CancelledPayments memory range = CancelledPayments({ + startIndex: countOfProcessed, + endIndex: newProcessedCount - 1 + }); + + cancelledPaymentIndices.push(range); } /// @notice Lets contract-owner send ERC721 NFTs to a list of addresses. @@ -208,42 +212,6 @@ contract AirdropERC721 is } } - /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents) - { - require(startId <= endId && endId < payeeCount, "invalid range"); - uint256 processed = processedCount; - if (startId >= processed) { - return contents; - } - - if (endId >= processed) { - endId = processed - 1; - } - - uint256 count; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - continue; - } - count += 1; - } - - contents = new AirdropContent[](count); - uint256 index; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - continue; - } - contents[index++] = airdropContent[i]; - } - } - /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents) { uint256 count = indicesOfFailed.length; @@ -254,38 +222,9 @@ contract AirdropERC721 is } } - /// @notice Returns all airdrop payments cancelled. - function getAllAirdropPaymentsCancelled(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents) - { - require(startId <= endId && endId < payeeCount, "invalid range"); - uint256 processed = processedCount; - if (startId >= processed) { - return contents; - } - - if (endId >= processed) { - endId = processed - 1; - } - - uint256 count; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - count += 1; - } - } - - contents = new AirdropContent[](count); - uint256 index; - - for (uint256 i = startId; i <= endId; i += 1) { - if (isCancelled[i]) { - contents[index++] = airdropContent[i]; - } - } + /// @notice Returns all blocks of cancelled payments as an array of index range. + function getCancelledPaymentIndices() external view returns (CancelledPayments[] memory) { + return cancelledPaymentIndices; } /*/////////////////////////////////////////////////////////////// diff --git a/contracts/interfaces/airdrop/IAirdropERC1155.sol b/contracts/interfaces/airdrop/IAirdropERC1155.sol index 664445f96..cba2bff15 100644 --- a/contracts/interfaces/airdrop/IAirdropERC1155.sol +++ b/contracts/interfaces/airdrop/IAirdropERC1155.sol @@ -36,6 +36,18 @@ interface IAirdropERC1155 { uint256 amount; } + /** + * @notice Range of indices of a set of cancelled payments. Each call to cancel payments + * stores this range in an array. + * + * @param startIndex First index of the set of cancelled payment indices. + * @param endIndex Last index of the set of cancelled payment indices. + */ + struct CancelledPayments { + uint256 startIndex; + uint256 endIndex; + } + /// @notice Returns all airdrop payments set up -- pending, processed or failed. function getAllAirdropPayments(uint256 startId, uint256 endId) external @@ -48,12 +60,6 @@ interface IAirdropERC1155 { view returns (AirdropContent[] memory contents); - /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents); - /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents); @@ -69,7 +75,7 @@ interface IAirdropERC1155 { /** * @notice Lets contract-owner cancel any pending payments. */ - function resetRecipients() external; + function cancelPendingPayments(uint256 numberOfPaymentsToCancel) external; /** * @notice Lets contract-owner set up an airdrop of ERC1155 tokens to a list of addresses. diff --git a/contracts/interfaces/airdrop/IAirdropERC20.sol b/contracts/interfaces/airdrop/IAirdropERC20.sol index 25a1f9cfc..9c1b9982a 100644 --- a/contracts/interfaces/airdrop/IAirdropERC20.sol +++ b/contracts/interfaces/airdrop/IAirdropERC20.sol @@ -34,6 +34,18 @@ interface IAirdropERC20 { uint256 amount; } + /** + * @notice Range of indices of a set of cancelled payments. Each call to cancel payments + * stores this range in an array. + * + * @param startIndex First index of the set of cancelled payment indices. + * @param endIndex Last index of the set of cancelled payment indices. + */ + struct CancelledPayments { + uint256 startIndex; + uint256 endIndex; + } + /// @notice Returns all airdrop payments set up -- pending, processed or failed. function getAllAirdropPayments(uint256 startId, uint256 endId) external @@ -46,12 +58,6 @@ interface IAirdropERC20 { view returns (AirdropContent[] memory contents); - /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents); - /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents); @@ -65,7 +71,7 @@ interface IAirdropERC20 { /** * @notice Lets contract-owner cancel any pending payments. */ - function resetRecipients() external; + function cancelPendingPayments(uint256 numberOfPaymentsToCancel) external; /** * @notice Lets contract-owner send ERC20 or native tokens to a list of addresses. diff --git a/contracts/interfaces/airdrop/IAirdropERC721.sol b/contracts/interfaces/airdrop/IAirdropERC721.sol index ea59d5cb9..dc04b6df6 100644 --- a/contracts/interfaces/airdrop/IAirdropERC721.sol +++ b/contracts/interfaces/airdrop/IAirdropERC721.sol @@ -34,6 +34,18 @@ interface IAirdropERC721 { uint256 tokenId; } + /** + * @notice Range of indices of a set of cancelled payments. Each call to cancel payments + * stores this range in an array. + * + * @param startIndex First index of the set of cancelled payment indices. + * @param endIndex Last index of the set of cancelled payment indices. + */ + struct CancelledPayments { + uint256 startIndex; + uint256 endIndex; + } + /// @notice Returns all airdrop payments set up -- pending, processed or failed. function getAllAirdropPayments(uint256 startId, uint256 endId) external @@ -46,12 +58,6 @@ interface IAirdropERC721 { view returns (AirdropContent[] memory contents); - /// @notice Returns all pending airdrop processed. - function getAllAirdropPaymentsProcessed(uint256 startId, uint256 endId) - external - view - returns (AirdropContent[] memory contents); - /// @notice Returns all pending airdrop failed. function getAllAirdropPaymentsFailed() external view returns (AirdropContent[] memory contents); @@ -67,7 +73,7 @@ interface IAirdropERC721 { /** * @notice Lets contract-owner cancel any pending payments. */ - function resetRecipients() external; + function cancelPendingPayments(uint256 numberOfPaymentsToCancel) external; /** * @notice Lets contract-owner set up an airdrop of ERC721 tokens to a list of addresses. diff --git a/src/test/airdrop/AirdropERC1155.t.sol b/src/test/airdrop/AirdropERC1155.t.sol index f08009045..81344bf47 100644 --- a/src/test/airdrop/AirdropERC1155.t.sol +++ b/src/test/airdrop/AirdropERC1155.t.sol @@ -71,7 +71,6 @@ contract AirdropERC1155Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -82,7 +81,6 @@ contract AirdropERC1155Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); @@ -103,7 +101,6 @@ contract AirdropERC1155Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -114,7 +111,6 @@ contract AirdropERC1155Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); @@ -148,16 +144,15 @@ contract AirdropERC1155Test is BaseTest { } /*/////////////////////////////////////////////////////////////// - Unit tests: `reset` + Unit tests: `cancelPayments` //////////////////////////////////////////////////////////////*/ - function test_state_resetRecipients() public { + function test_state_cancelPayments() public { vm.prank(deployer); drop.addRecipients(_contentsOne); // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -168,29 +163,31 @@ contract AirdropERC1155Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + IAirdropERC1155.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + for (uint256 i = 0; i < countOne - 300; i++) { assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); } } - function test_state_resetRecipients_addMore() public { + function test_state_cancelPayments_addMore() public { vm.prank(deployer); drop.addRecipients(_contentsOne); @@ -198,33 +195,45 @@ contract AirdropERC1155Test is BaseTest { vm.prank(deployer); drop.processPayments(countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + IAirdropERC1155.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + // add more recipients vm.prank(deployer); drop.addRecipients(_contentsTwo); // check state assertEq(drop.getAllAirdropPayments(0, countOne + countTwo - 1).length, countOne + countTwo); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne + countTwo - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne + countTwo - 1).length, countTwo); // pending payments equal to count of new recipients added - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne + countTwo - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne + countTwo); assertEq(drop.processedCount(), countOne); for (uint256 i = 0; i < countOne - 300; i++) { assertEq(erc1155.balanceOf(_contentsOne[i].recipient, i % 5), 5); } + + // cancel more + vm.prank(deployer); + drop.cancelPendingPayments(100); + + cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 2); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + assertEq(cancelledPayments[1].startIndex, countOne); + assertEq(cancelledPayments[1].endIndex, countOne + 100 - 1); } /*/////////////////////////////////////////////////////////////// diff --git a/src/test/airdrop/AirdropERC20.t.sol b/src/test/airdrop/AirdropERC20.t.sol index 9cc9d3940..c9238d230 100644 --- a/src/test/airdrop/AirdropERC20.t.sol +++ b/src/test/airdrop/AirdropERC20.t.sol @@ -66,7 +66,6 @@ contract AirdropERC20Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -77,7 +76,6 @@ contract AirdropERC20Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); @@ -94,7 +92,6 @@ contract AirdropERC20Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -105,7 +102,6 @@ contract AirdropERC20Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); @@ -130,7 +126,6 @@ contract AirdropERC20Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -141,7 +136,6 @@ contract AirdropERC20Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); @@ -166,7 +160,6 @@ contract AirdropERC20Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -177,7 +170,6 @@ contract AirdropERC20Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); @@ -226,16 +218,15 @@ contract AirdropERC20Test is BaseTest { } /*/////////////////////////////////////////////////////////////// - Unit tests: `reset` + Unit tests: `cancelPayments` //////////////////////////////////////////////////////////////*/ - function test_state_resetRecipients() public { + function test_state_cancelPayments() public { vm.prank(deployer); drop.addRecipients(_contentsOne); // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -246,30 +237,32 @@ contract AirdropERC20Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + IAirdropERC20.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + for (uint256 i = 0; i < countOne - 300; i++) { assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); } assertEq(erc20.balanceOf(address(tokenOwner)), 3000 ether); } - function test_state_resetRecipients_addMore() public { + function test_state_cancelPayments_addMore() public { vm.prank(deployer); drop.addRecipients(_contentsOne); @@ -277,15 +270,13 @@ contract AirdropERC20Test is BaseTest { vm.prank(deployer); drop.processPayments(countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count @@ -295,19 +286,33 @@ contract AirdropERC20Test is BaseTest { // check state assertEq(drop.getAllAirdropPayments(0, countOne + countTwo - 1).length, countOne + countTwo); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne + countTwo - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne + countTwo - 1).length, countTwo); // pending payments equal to count of new recipients added - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne + countTwo - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne + countTwo); assertEq(drop.processedCount(), countOne); + IAirdropERC20.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + for (uint256 i = 0; i < countOne - 300; i++) { assertEq(erc20.balanceOf(_contentsOne[i].recipient), _contentsOne[i].amount); } assertEq(erc20.balanceOf(address(tokenOwner)), 3000 ether); + + // cancel more + vm.prank(deployer); + drop.cancelPendingPayments(100); + + cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 2); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + assertEq(cancelledPayments[1].startIndex, countOne); + assertEq(cancelledPayments[1].endIndex, countOne + 100 - 1); } - function test_state_resetRecipients_nativeToken() public { + function test_state_cancelPayments_nativeToken() public { vm.deal(deployer, 10_000 ether); uint256 balBefore = deployer.balance; @@ -321,7 +326,6 @@ contract AirdropERC20Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -332,23 +336,25 @@ contract AirdropERC20Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + IAirdropERC20.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + for (uint256 i = 0; i < countOne - 300; i++) { assertEq(_contentsOne[i].recipient.balance, _contentsOne[i].amount); } @@ -451,7 +457,6 @@ contract AirdropERC20AuditTest is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index de5ad13ea..06f6a1ef1 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -64,7 +64,6 @@ contract AirdropERC721Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -75,7 +74,6 @@ contract AirdropERC721Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); @@ -91,7 +89,6 @@ contract AirdropERC721Test is BaseTest { // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -102,7 +99,6 @@ contract AirdropERC721Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); @@ -153,16 +149,15 @@ contract AirdropERC721Test is BaseTest { } /*/////////////////////////////////////////////////////////////// - Unit tests: `reset` + Unit tests: `cancelPayments` //////////////////////////////////////////////////////////////*/ - function test_state_resetRecipients() public { + function test_state_cancelPayments() public { vm.prank(deployer); drop.addRecipients(_contentsOne); // check state before airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, 0); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, countOne); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), 0); @@ -173,29 +168,31 @@ contract AirdropERC721Test is BaseTest { // check state after airdrop assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 300); assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + IAirdropERC721.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + for (uint256 i = 0; i < 700; i++) { assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); } } - function test_state_resetRecipients_addMore() public { + function test_state_cancelPayments_addMore() public { vm.prank(deployer); drop.addRecipients(_contentsOne); @@ -203,33 +200,45 @@ contract AirdropERC721Test is BaseTest { vm.prank(deployer); drop.processPayments(countOne - 300); - // do a reset + // cancel payments vm.prank(deployer); - drop.resetRecipients(); + drop.cancelPendingPayments(300); // check state after reset assertEq(drop.getAllAirdropPayments(0, countOne - 1).length, countOne); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne - 1).length, 0); // 0 pending payments after reset - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne); assertEq(drop.processedCount(), countOne); // processed count set equal to total payee count + IAirdropERC721.CancelledPayments[] memory cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 1); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + // add more recipients vm.prank(deployer); drop.addRecipients(_contentsTwo); // check state assertEq(drop.getAllAirdropPayments(0, countOne + countTwo - 1).length, countOne + countTwo); - assertEq(drop.getAllAirdropPaymentsProcessed(0, countOne + countTwo - 1).length, countOne - 300); assertEq(drop.getAllAirdropPaymentsPending(0, countOne + countTwo - 1).length, countTwo); // pending payments equal to count of new recipients added - assertEq(drop.getAllAirdropPaymentsCancelled(0, countOne + countTwo - 1).length, 300); // cancelled payments assertEq(drop.payeeCount(), countOne + countTwo); assertEq(drop.processedCount(), countOne); for (uint256 i = 0; i < 700; i++) { assertEq(erc721.ownerOf(i), _contentsOne[i].recipient); } + + // cancel more + vm.prank(deployer); + drop.cancelPendingPayments(100); + + cancelledPayments = drop.getCancelledPaymentIndices(); + assertEq(cancelledPayments.length, 2); + assertEq(cancelledPayments[0].startIndex, countOne - 300); + assertEq(cancelledPayments[0].endIndex, countOne - 1); + assertEq(cancelledPayments[1].startIndex, countOne); + assertEq(cancelledPayments[1].endIndex, countOne + 100 - 1); } /*/////////////////////////////////////////////////////////////// From 27ab08bad476e2989f98a8758309ec647b43d329 Mon Sep 17 00:00:00 2001 From: Yash Date: Wed, 1 Mar 2023 03:06:45 +0530 Subject: [PATCH 14/18] [L-1] No event emitted within resetRecipients() --- contracts/airdrop/AirdropERC1155.sol | 2 ++ contracts/airdrop/AirdropERC20.sol | 2 ++ contracts/airdrop/AirdropERC721.sol | 2 ++ contracts/interfaces/airdrop/IAirdropERC1155.sol | 2 +- contracts/interfaces/airdrop/IAirdropERC20.sol | 2 +- contracts/interfaces/airdrop/IAirdropERC721.sol | 2 +- 6 files changed, 9 insertions(+), 3 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index 13fee255a..f039b19d4 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -100,6 +100,8 @@ contract AirdropERC1155 is }); cancelledPaymentIndices.push(range); + + emit PaymentsCancelledByAdmin(countOfProcessed, newProcessedCount - 1); } /// @notice Lets contract-owner send ERC721 NFTs to a list of addresses. diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index ec9d985ef..1460c6d05 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -123,6 +123,8 @@ contract AirdropERC20 is // refund amount to contract admin address CurrencyTransferLib.safeTransferNativeToken(msg.sender, nativeTokenAmount); } + + emit PaymentsCancelledByAdmin(countOfProcessed, newProcessedCount - 1); } /// @notice Lets contract-owner send ERC20 or native tokens to a list of addresses. diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 2df12ea5f..76991bfed 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -100,6 +100,8 @@ contract AirdropERC721 is }); cancelledPaymentIndices.push(range); + + emit PaymentsCancelledByAdmin(countOfProcessed, newProcessedCount - 1); } /// @notice Lets contract-owner send ERC721 NFTs to a list of addresses. diff --git a/contracts/interfaces/airdrop/IAirdropERC1155.sol b/contracts/interfaces/airdrop/IAirdropERC1155.sol index cba2bff15..9118393b9 100644 --- a/contracts/interfaces/airdrop/IAirdropERC1155.sol +++ b/contracts/interfaces/airdrop/IAirdropERC1155.sol @@ -13,7 +13,7 @@ interface IAirdropERC1155 { /// @notice Emitted when airdrop recipients are uploaded to the contract. event RecipientsAdded(AirdropContent[] _contents); /// @notice Emitted when pending payments are cancelled, and processed count is reset. - event PaymentsResetByAdmin(); + event PaymentsCancelledByAdmin(uint256 startIndex, uint256 endIndex); /// @notice Emitted when an airdrop payment is made to a recipient. event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. diff --git a/contracts/interfaces/airdrop/IAirdropERC20.sol b/contracts/interfaces/airdrop/IAirdropERC20.sol index 9c1b9982a..eb6ee8da1 100644 --- a/contracts/interfaces/airdrop/IAirdropERC20.sol +++ b/contracts/interfaces/airdrop/IAirdropERC20.sol @@ -13,7 +13,7 @@ interface IAirdropERC20 { /// @notice Emitted when airdrop recipients are uploaded to the contract. event RecipientsAdded(AirdropContent[] _contents); /// @notice Emitted when pending payments are cancelled, and processed count is reset. - event PaymentsResetByAdmin(); + event PaymentsCancelledByAdmin(uint256 startIndex, uint256 endIndex); /// @notice Emitted when an airdrop payment is made to a recipient. event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. diff --git a/contracts/interfaces/airdrop/IAirdropERC721.sol b/contracts/interfaces/airdrop/IAirdropERC721.sol index dc04b6df6..efaa4c86b 100644 --- a/contracts/interfaces/airdrop/IAirdropERC721.sol +++ b/contracts/interfaces/airdrop/IAirdropERC721.sol @@ -13,7 +13,7 @@ interface IAirdropERC721 { /// @notice Emitted when airdrop recipients are uploaded to the contract. event RecipientsAdded(AirdropContent[] _contents); /// @notice Emitted when pending payments are cancelled, and processed count is reset. - event PaymentsResetByAdmin(); + event PaymentsCancelledByAdmin(uint256 startIndex, uint256 endIndex); /// @notice Emitted when an airdrop payment is made to a recipient. event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. From b6bc342e13fcff228e2695fea1726df6f1bf3614 Mon Sep 17 00:00:00 2001 From: Yash Date: Wed, 1 Mar 2023 06:28:48 +0530 Subject: [PATCH 15/18] [M-3] processPayments() can be griefed --- contracts/airdrop/AirdropERC1155.sol | 2 +- contracts/airdrop/AirdropERC20.sol | 2 +- contracts/airdrop/AirdropERC721.sol | 6 +- src/test/airdrop/AirdropERC1155.t.sol | 56 +++++++ src/test/airdrop/AirdropERC20.t.sol | 45 ++++++ src/test/airdrop/AirdropERC721.t.sol | 52 +++++++ src/test/airdrop/AirdropGriefingTest.t.sol | 171 +++++++++++++++++++++ 7 files changed, 331 insertions(+), 3 deletions(-) create mode 100644 src/test/airdrop/AirdropGriefingTest.t.sol diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index f039b19d4..f8ed0be8e 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -118,7 +118,7 @@ contract AirdropERC1155 is bool failed; try - IERC1155(content.tokenAddress).safeTransferFrom( + IERC1155(content.tokenAddress).safeTransferFrom{ gas: 80_000 }( content.tokenOwner, content.recipient, content.tokenId, diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 1460c6d05..336f19f54 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -283,7 +283,7 @@ contract AirdropERC20 is if (_currency == CurrencyTransferLib.NATIVE_TOKEN) { // solhint-disable avoid-low-level-calls // slither-disable-next-line low-level-calls - (success, ) = _to.call{ value: _amount }(""); + (success, ) = _to.call{ value: _amount, gas: 80_000 }(""); } else { (success, ) = _currency.call(abi.encodeWithSelector(IERC20.transferFrom.selector, _from, _to, _amount)); diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 76991bfed..8d6065db9 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -118,7 +118,11 @@ contract AirdropERC721 is bool failed; try - IERC721(content.tokenAddress).safeTransferFrom(content.tokenOwner, content.recipient, content.tokenId) + IERC721(content.tokenAddress).safeTransferFrom{ gas: 80_000 }( + content.tokenOwner, + content.recipient, + content.tokenId + ) {} catch { // revert if failure is due to unapproved tokens require( diff --git a/src/test/airdrop/AirdropERC1155.t.sol b/src/test/airdrop/AirdropERC1155.t.sol index 81344bf47..f3e36fde8 100644 --- a/src/test/airdrop/AirdropERC1155.t.sol +++ b/src/test/airdrop/AirdropERC1155.t.sol @@ -277,3 +277,59 @@ contract AirdropERC1155Test is BaseTest { vm.stopPrank(); } } + +contract AirdropERC1155GasTest is BaseTest { + AirdropERC1155 internal drop; + + Wallet internal tokenOwner; + + function setUp() public override { + super.setUp(); + + drop = AirdropERC1155(getContract("AirdropERC1155")); + + tokenOwner = getWallet(); + + erc1155.mint(address(tokenOwner), 0, 1000); + + tokenOwner.setApprovalForAllERC1155(address(erc1155), address(drop), true); + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: gas benchmarks, etc. + //////////////////////////////////////////////////////////////*/ + + function test_safeTransferFrom_toEOA() public { + vm.prank(address(tokenOwner)); + erc1155.safeTransferFrom(address(tokenOwner), address(0x123), 0, 10, ""); + } + + function test_safeTransferFrom_toContract() public { + vm.prank(address(tokenOwner)); + erc1155.safeTransferFrom(address(tokenOwner), address(this), 0, 10, ""); + } + + function test_safeTransferFrom_toEOA_gasOverride() public { + vm.prank(address(tokenOwner)); + console.log(gasleft()); + erc1155.safeTransferFrom{ gas: 100_000 }(address(tokenOwner), address(this), 0, 10, ""); + console.log(gasleft()); + } + + function test_safeTransferFrom_toContract_gasOverride() public { + vm.prank(address(tokenOwner)); + console.log(gasleft()); + erc1155.safeTransferFrom{ gas: 100_000 }(address(tokenOwner), address(this), 0, 10, ""); + console.log(gasleft()); + } + + function onERC1155Received( + address, + address, + uint256, + uint256, + bytes calldata + ) external returns (bytes4) { + return this.onERC1155Received.selector; + } +} diff --git a/src/test/airdrop/AirdropERC20.t.sol b/src/test/airdrop/AirdropERC20.t.sol index c9238d230..9aaa35164 100644 --- a/src/test/airdrop/AirdropERC20.t.sol +++ b/src/test/airdrop/AirdropERC20.t.sol @@ -467,3 +467,48 @@ contract AirdropERC20AuditTest is BaseTest { assertEq(erc20_nonCompliant.balanceOf(address(tokenOwner)), 0); } } + +contract AirdropERC20GasTest is BaseTest { + AirdropERC20 internal drop; + + Wallet internal tokenOwner; + + function setUp() public override { + super.setUp(); + + drop = AirdropERC20(getContract("AirdropERC20")); + + tokenOwner = getWallet(); + + erc20.mint(address(tokenOwner), 10_000 ether); + tokenOwner.setAllowanceERC20(address(erc20), address(drop), type(uint256).max); + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: gas benchmarks, etc. + //////////////////////////////////////////////////////////////*/ + + function test_transferNativeToken_toEOA() public { + vm.prank(address(tokenOwner)); + address(0x123).call{ value: 1 ether }(""); + } + + function test_transferNativeToken_toContract() public { + vm.prank(address(tokenOwner)); + address(this).call{ value: 1 ether }(""); + } + + function test_transferNativeToken_toEOA_gasOverride() public { + vm.prank(address(tokenOwner)); + console.log(gasleft()); + address(0x123).call{ value: 1 ether, gas: 100_000 }(""); + console.log(gasleft()); + } + + function test_transferNativeToken_toContract_gasOverride() public { + vm.prank(address(tokenOwner)); + console.log(gasleft()); + address(this).call{ value: 1 ether, gas: 100_000 }(""); + console.log(gasleft()); + } +} diff --git a/src/test/airdrop/AirdropERC721.t.sol b/src/test/airdrop/AirdropERC721.t.sol index 06f6a1ef1..6e54c1178 100644 --- a/src/test/airdrop/AirdropERC721.t.sol +++ b/src/test/airdrop/AirdropERC721.t.sol @@ -355,3 +355,55 @@ contract AirdropERC721AuditTest is BaseTest { drop.processPayments(2); } } + +contract AirdropERC721GasTest is BaseTest { + AirdropERC721 internal drop; + + Wallet internal tokenOwner; + + function setUp() public override { + super.setUp(); + + drop = AirdropERC721(getContract("AirdropERC721")); + + tokenOwner = getWallet(); + + erc721.mint(address(tokenOwner), 1500); + tokenOwner.setApprovalForAllERC721(address(erc721), address(drop), true); + + vm.startPrank(address(tokenOwner)); + } + + /*/////////////////////////////////////////////////////////////// + Unit tests: gas benchmarks, etc. + //////////////////////////////////////////////////////////////*/ + + function test_safeTransferFrom_toEOA() public { + erc721.safeTransferFrom(address(tokenOwner), address(0x123), 0); + } + + function test_safeTransferFrom_toContract() public { + erc721.safeTransferFrom(address(tokenOwner), address(this), 0); + } + + function test_safeTransferFrom_toEOA_gasOverride() public { + console.log(gasleft()); + erc721.safeTransferFrom{ gas: 100_000 }(address(tokenOwner), address(0x123), 0); + console.log(gasleft()); + } + + function test_safeTransferFrom_toContract_gasOverride() public { + console.log(gasleft()); + erc721.safeTransferFrom{ gas: 100_000 }(address(tokenOwner), address(this), 0); + console.log(gasleft()); + } + + function onERC721Received( + address, + address, + uint256, + bytes calldata + ) external view returns (bytes4) { + return this.onERC721Received.selector; + } +} diff --git a/src/test/airdrop/AirdropGriefingTest.t.sol b/src/test/airdrop/AirdropGriefingTest.t.sol new file mode 100644 index 000000000..e61aa64d9 --- /dev/null +++ b/src/test/airdrop/AirdropGriefingTest.t.sol @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity ^0.8.0; + +// Test imports +import { Wallet } from "../utils/Wallet.sol"; +import "../utils/BaseTest.sol"; + +import "contracts/interfaces/airdrop/IAirdropERC20.sol"; + +contract GriefingContract { + event YouAreBeingGriefed(); + + receive() external payable { + grief(); + } + + function onERC721Received( + address, + address, + uint256, + bytes calldata + ) external returns (bytes4) { + grief(); + return GriefingContract.onERC721Received.selector; + } + + function onERC1155Received( + address, + address, + uint256, + uint256, + bytes calldata + ) external returns (bytes4) { + grief(); + return GriefingContract.onERC1155Received.selector; + } + + function grief() internal { + while (true) { + emit YouAreBeingGriefed(); + } + } +} + +contract AirdropGriefingTest is BaseTest { + AirdropERC721 internal dropERC721; + AirdropERC1155 internal dropERC1155; + AirdropERC20 internal dropERC20; + + Wallet internal tokenOwner; + + IAirdropERC721.AirdropContent[] internal contentsERC721; + IAirdropERC1155.AirdropContent[] internal contentsERC1155; + IAirdropERC20.AirdropContent[] internal contentsERC20; + + GriefingContract private griefingContract; + + function setUp() public override { + super.setUp(); + + tokenOwner = getWallet(); + + // setup erc721 + dropERC721 = AirdropERC721(getContract("AirdropERC721")); + + erc721.mint(address(tokenOwner), 1500); + tokenOwner.setApprovalForAllERC721(address(erc721), address(dropERC721), true); + + griefingContract = new GriefingContract(); + + // add griefing contract to airdrop + contentsERC721.push( + IAirdropERC721.AirdropContent({ + tokenAddress: address(erc721), + tokenOwner: address(tokenOwner), + recipient: address(griefingContract), + tokenId: 50 + }) + ); + + for (uint256 i = 0; i < 5; i++) { + contentsERC721.push( + IAirdropERC721.AirdropContent({ + tokenAddress: address(erc721), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + tokenId: i + }) + ); + } + + vm.prank(deployer); + dropERC721.addRecipients(contentsERC721); + + // setup erc1155 + dropERC1155 = AirdropERC1155(getContract("AirdropERC1155")); + + erc1155.mint(address(tokenOwner), 1, 1500); + tokenOwner.setApprovalForAllERC1155(address(erc1155), address(dropERC1155), true); + + //add griefing contract to airdrop + contentsERC1155.push( + IAirdropERC1155.AirdropContent({ + tokenAddress: address(erc1155), + tokenOwner: address(tokenOwner), + recipient: address(griefingContract), + tokenId: 1, + amount: 1 + }) + ); + + for (uint256 i = 0; i < 5; i++) { + contentsERC1155.push( + IAirdropERC1155.AirdropContent({ + tokenAddress: address(erc1155), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + tokenId: 1, + amount: 1 + }) + ); + } + + vm.prank(deployer); + dropERC1155.addRecipients(contentsERC1155); + + // setup erc20 + dropERC20 = AirdropERC20(getContract("AirdropERC20")); + + //add griefing contract to airdrop + contentsERC20.push( + IAirdropERC20.AirdropContent({ + tokenAddress: address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE), + tokenOwner: address(tokenOwner), + recipient: address(griefingContract), + amount: 1 + }) + ); + + for (uint256 i = 0; i < 5; i++) { + contentsERC20.push( + IAirdropERC20.AirdropContent({ + tokenAddress: address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE), + tokenOwner: address(tokenOwner), + recipient: getActor(uint160(i)), + amount: 1 + }) + ); + } + + vm.deal(deployer, 10_000 ether); + + vm.prank(deployer); + dropERC20.addRecipients{ value: 6 }(contentsERC20); + } + + function test_GriefingERC721_Exceeds_30M_Gas() public { + vm.prank(deployer); + dropERC721.processPayments(6); + } + + function test_GriefingERC1155_Exceeds_30M_Gas() public { + vm.prank(deployer); + dropERC1155.processPayments(6); + } + + function test_GriefingERC20_Exceeds_30M_Gas() public { + vm.prank(deployer); + dropERC20.processPayments(6); + } +} From 9df9018eeabdd3d145f4dac3727c701dc4422f24 Mon Sep 17 00:00:00 2001 From: Yash Date: Sat, 4 Mar 2023 04:09:46 +0530 Subject: [PATCH 16/18] [G-3] Use unchecked blocks to reduce gas --- contracts/airdrop/AirdropERC1155.sol | 18 +++++++++++++++--- contracts/airdrop/AirdropERC20.sol | 24 ++++++++++++++++++++---- contracts/airdrop/AirdropERC721.sol | 18 +++++++++++++++--- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index f8ed0be8e..c87071f7a 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -78,8 +78,12 @@ contract AirdropERC1155 is uint256 currentCount = payeeCount; payeeCount += len; - for (uint256 i = 0; i < len; i += 1) { + for (uint256 i = 0; i < len; ) { airdropContent[i + currentCount] = _contents[i]; + + unchecked { + i += 1; + } } emit RecipientsAdded(_contents); @@ -113,7 +117,7 @@ contract AirdropERC1155 is processedCount += paymentsToProcess; - for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { + for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); ) { AirdropContent memory content = airdropContent[i]; bool failed; @@ -139,6 +143,10 @@ contract AirdropERC1155 is } emit AirdropPayment(content.recipient, content, failed); + + unchecked { + i += 1; + } } } @@ -152,7 +160,7 @@ contract AirdropERC1155 is function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; - for (uint256 i = 0; i < len; i++) { + for (uint256 i = 0; i < len; ) { bool failed; try IERC1155(_contents[i].tokenAddress).safeTransferFrom( @@ -175,6 +183,10 @@ contract AirdropERC1155 is } emit StatelessAirdrop(_contents[i].recipient, _contents[i], failed); + + unchecked { + i += 1; + } } } diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 336f19f54..d6514bdb8 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -81,12 +81,16 @@ contract AirdropERC20 is uint256 nativeTokenAmount; - for (uint256 i = 0; i < len; i += 1) { + for (uint256 i = 0; i < len; ) { airdropContent[i + currentCount] = _contents[i]; if (_contents[i].tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { nativeTokenAmount += _contents[i].amount; } + + unchecked { + i += 1; + } } require(nativeTokenAmount == msg.value, "Incorrect native token amount"); @@ -111,12 +115,16 @@ contract AirdropERC20 is cancelledPaymentIndices.push(range); - for (uint256 i = countOfProcessed; i < newProcessedCount; i += 1) { + for (uint256 i = countOfProcessed; i < newProcessedCount; ) { AirdropContent memory content = airdropContent[i]; if (content.tokenAddress == CurrencyTransferLib.NATIVE_TOKEN) { nativeTokenAmount += content.amount; } + + unchecked { + i += 1; + } } if (nativeTokenAmount > 0) { @@ -137,7 +145,7 @@ contract AirdropERC20 is processedCount += paymentsToProcess; - for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { + for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); ) { AirdropContent memory content = airdropContent[i]; bool success = _transferCurrencyWithReturnVal( @@ -158,6 +166,10 @@ contract AirdropERC20 is } emit AirdropPayment(content.recipient, content, !success); + + unchecked { + i += 1; + } } if (nativeTokenAmount > 0) { @@ -178,7 +190,7 @@ contract AirdropERC20 is uint256 nativeTokenAmount; uint256 refundAmount; - for (uint256 i = 0; i < len; i++) { + for (uint256 i = 0; i < len; ) { bool success = _transferCurrencyWithReturnVal( _contents[i].tokenAddress, _contents[i].tokenOwner, @@ -195,6 +207,10 @@ contract AirdropERC20 is } emit StatelessAirdrop(_contents[i].recipient, _contents[i], !success); + + unchecked { + i += 1; + } } require(nativeTokenAmount == msg.value, "Incorrect native token amount"); diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 8d6065db9..22bccc452 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -78,8 +78,12 @@ contract AirdropERC721 is uint256 currentCount = payeeCount; payeeCount += len; - for (uint256 i = 0; i < len; i += 1) { + for (uint256 i = 0; i < len; ) { airdropContent[i + currentCount] = _contents[i]; + + unchecked { + i += 1; + } } emit RecipientsAdded(_contents); @@ -113,7 +117,7 @@ contract AirdropERC721 is processedCount += paymentsToProcess; - for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); i += 1) { + for (uint256 i = countOfProcessed; i < (countOfProcessed + paymentsToProcess); ) { AirdropContent memory content = airdropContent[i]; bool failed; @@ -138,6 +142,10 @@ contract AirdropERC721 is } emit AirdropPayment(content.recipient, content, failed); + + unchecked { + i += 1; + } } } @@ -151,7 +159,7 @@ contract AirdropERC721 is function airdrop(AirdropContent[] calldata _contents) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) { uint256 len = _contents.length; - for (uint256 i = 0; i < len; i++) { + for (uint256 i = 0; i < len; ) { bool failed; try IERC721(_contents[i].tokenAddress).safeTransferFrom( @@ -172,6 +180,10 @@ contract AirdropERC721 is } emit StatelessAirdrop(_contents[i].recipient, _contents[i], failed); + + unchecked { + i += 1; + } } } From ea87439cca143ba8e96f46d165bf285212674d86 Mon Sep 17 00:00:00 2001 From: Yash Date: Sat, 4 Mar 2023 04:20:39 +0530 Subject: [PATCH 17/18] [G-4] Unnecessary to emit airdrop contents --- contracts/airdrop/AirdropERC1155.sol | 4 ++-- contracts/airdrop/AirdropERC20.sol | 4 ++-- contracts/airdrop/AirdropERC721.sol | 4 ++-- contracts/interfaces/airdrop/IAirdropERC1155.sol | 4 ++-- contracts/interfaces/airdrop/IAirdropERC20.sol | 4 ++-- contracts/interfaces/airdrop/IAirdropERC721.sol | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contracts/airdrop/AirdropERC1155.sol b/contracts/airdrop/AirdropERC1155.sol index c87071f7a..2b43e0179 100644 --- a/contracts/airdrop/AirdropERC1155.sol +++ b/contracts/airdrop/AirdropERC1155.sol @@ -86,7 +86,7 @@ contract AirdropERC1155 is } } - emit RecipientsAdded(_contents); + emit RecipientsAdded(currentCount, currentCount + len); } ///@notice Lets contract-owner cancel any pending payments. @@ -142,7 +142,7 @@ contract AirdropERC1155 is failed = true; } - emit AirdropPayment(content.recipient, content, failed); + emit AirdropPayment(content.recipient, i, failed); unchecked { i += 1; diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index d6514bdb8..900aeef29 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -95,7 +95,7 @@ contract AirdropERC20 is require(nativeTokenAmount == msg.value, "Incorrect native token amount"); - emit RecipientsAdded(_contents); + emit RecipientsAdded(currentCount, currentCount + len); } ///@notice Lets contract-owner cancel any pending payments. @@ -165,7 +165,7 @@ contract AirdropERC20 is success = false; } - emit AirdropPayment(content.recipient, content, !success); + emit AirdropPayment(content.recipient, i, !success); unchecked { i += 1; diff --git a/contracts/airdrop/AirdropERC721.sol b/contracts/airdrop/AirdropERC721.sol index 22bccc452..2983e765a 100644 --- a/contracts/airdrop/AirdropERC721.sol +++ b/contracts/airdrop/AirdropERC721.sol @@ -86,7 +86,7 @@ contract AirdropERC721 is } } - emit RecipientsAdded(_contents); + emit RecipientsAdded(currentCount, currentCount + len); } ///@notice Lets contract-owner cancel any pending payments. @@ -141,7 +141,7 @@ contract AirdropERC721 is failed = true; } - emit AirdropPayment(content.recipient, content, failed); + emit AirdropPayment(content.recipient, i, failed); unchecked { i += 1; diff --git a/contracts/interfaces/airdrop/IAirdropERC1155.sol b/contracts/interfaces/airdrop/IAirdropERC1155.sol index 9118393b9..ad2e724c0 100644 --- a/contracts/interfaces/airdrop/IAirdropERC1155.sol +++ b/contracts/interfaces/airdrop/IAirdropERC1155.sol @@ -11,11 +11,11 @@ pragma solidity ^0.8.11; interface IAirdropERC1155 { /// @notice Emitted when airdrop recipients are uploaded to the contract. - event RecipientsAdded(AirdropContent[] _contents); + event RecipientsAdded(uint256 startIndex, uint256 endIndex); /// @notice Emitted when pending payments are cancelled, and processed count is reset. event PaymentsCancelledByAdmin(uint256 startIndex, uint256 endIndex); /// @notice Emitted when an airdrop payment is made to a recipient. - event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); + event AirdropPayment(address indexed recipient, uint256 index, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. event StatelessAirdrop(address indexed recipient, AirdropContent content, bool failed); diff --git a/contracts/interfaces/airdrop/IAirdropERC20.sol b/contracts/interfaces/airdrop/IAirdropERC20.sol index eb6ee8da1..bf81e71cc 100644 --- a/contracts/interfaces/airdrop/IAirdropERC20.sol +++ b/contracts/interfaces/airdrop/IAirdropERC20.sol @@ -11,11 +11,11 @@ pragma solidity ^0.8.11; interface IAirdropERC20 { /// @notice Emitted when airdrop recipients are uploaded to the contract. - event RecipientsAdded(AirdropContent[] _contents); + event RecipientsAdded(uint256 startIndex, uint256 endIndex); /// @notice Emitted when pending payments are cancelled, and processed count is reset. event PaymentsCancelledByAdmin(uint256 startIndex, uint256 endIndex); /// @notice Emitted when an airdrop payment is made to a recipient. - event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); + event AirdropPayment(address indexed recipient, uint256 index, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. event StatelessAirdrop(address indexed recipient, AirdropContent content, bool failed); diff --git a/contracts/interfaces/airdrop/IAirdropERC721.sol b/contracts/interfaces/airdrop/IAirdropERC721.sol index efaa4c86b..d0d0e680f 100644 --- a/contracts/interfaces/airdrop/IAirdropERC721.sol +++ b/contracts/interfaces/airdrop/IAirdropERC721.sol @@ -11,11 +11,11 @@ pragma solidity ^0.8.11; interface IAirdropERC721 { /// @notice Emitted when airdrop recipients are uploaded to the contract. - event RecipientsAdded(AirdropContent[] _contents); + event RecipientsAdded(uint256 startIndex, uint256 endIndex); /// @notice Emitted when pending payments are cancelled, and processed count is reset. event PaymentsCancelledByAdmin(uint256 startIndex, uint256 endIndex); /// @notice Emitted when an airdrop payment is made to a recipient. - event AirdropPayment(address indexed recipient, AirdropContent content, bool failed); + event AirdropPayment(address indexed recipient, uint256 index, bool failed); /// @notice Emitted when an airdrop is made using the stateless airdrop function. event StatelessAirdrop(address indexed recipient, AirdropContent content, bool failed); From f2c7180f8f4f35225cece807bdfe36129e545fa5 Mon Sep 17 00:00:00 2001 From: Yash Date: Fri, 10 Mar 2023 22:39:58 +0530 Subject: [PATCH 18/18] [M-1] Fails for non-compliant ERC20 tokens: revised fix --- contracts/airdrop/AirdropERC20.sol | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/contracts/airdrop/AirdropERC20.sol b/contracts/airdrop/AirdropERC20.sol index 900aeef29..bf9680485 100644 --- a/contracts/airdrop/AirdropERC20.sol +++ b/contracts/airdrop/AirdropERC20.sol @@ -301,9 +301,14 @@ contract AirdropERC20 is // slither-disable-next-line low-level-calls (success, ) = _to.call{ value: _amount, gas: 80_000 }(""); } else { - (success, ) = _currency.call(abi.encodeWithSelector(IERC20.transferFrom.selector, _from, _to, _amount)); + (bool success_, bytes memory data_) = _currency.call( + abi.encodeWithSelector(IERC20.transferFrom.selector, _from, _to, _amount) + ); + + success = success_; + if (!success || (data_.length > 0 && !abi.decode(data_, (bool)))) { + success = false; - if (!success) { require( IERC20(_currency).balanceOf(_from) >= _amount && IERC20(_currency).allowance(_from, address(this)) >= _amount,