From d45bb4b8406aa9ff15598ad00e5524ce767b7ced Mon Sep 17 00:00:00 2001 From: Sam Bugs Date: Thu, 15 Feb 2024 19:17:27 -0300 Subject: [PATCH] refactor: expose internal state better --- solidity/contracts/OngoingCampaigns.sol | 56 +++++++++++++------- solidity/contracts/test/OngoingCampaigns.sol | 4 +- solidity/interfaces/IOngoingCampaigns.sol | 16 +++--- test/unit/ongoing-campaigns.spec.ts | 28 +++------- test/utils/bdd.ts | 2 +- 5 files changed, 53 insertions(+), 53 deletions(-) diff --git a/solidity/contracts/OngoingCampaigns.sol b/solidity/contracts/OngoingCampaigns.sol index 026b7b4..4c8747a 100644 --- a/solidity/contracts/OngoingCampaigns.sol +++ b/solidity/contracts/OngoingCampaigns.sol @@ -13,12 +13,9 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { /// @inheritdoc IOngoingCampaigns mapping(bytes32 => bytes32) public roots; - /// @inheritdoc IOngoingCampaigns - mapping(bytes32 => uint256) public amountClaimedByCampaignTokenAndClaimee; - /// @inheritdoc IOngoingCampaigns - mapping(bytes32 => uint256) public totalAirdroppedByCampaignAndToken; - /// @inheritdoc IOngoingCampaigns - mapping(bytes32 => uint256) public totalClaimedByCampaignAndToken; + mapping(bytes32 => uint256) internal _amountClaimedByCampaignTokenAndClaimee; + mapping(bytes32 => uint256) internal _totalAirdroppedByCampaignAndToken; + mapping(bytes32 => uint256) internal _totalClaimedByCampaignAndToken; constructor(address _superAdmin, address[] memory _initialAdmins) AccessControlDefaultAdminRules(3 days, _superAdmin) { for (uint256 _i = 0; _i < _initialAdmins.length; ++_i) { @@ -26,6 +23,25 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { } } + /// @inheritdoc IOngoingCampaigns + function amountClaimed( + bytes32 _campaign, + IERC20 _token, + address _claimee + ) external view returns (uint256) { + return _amountClaimedByCampaignTokenAndClaimee[_getIdOfCampaignTokenAndClaimee(_campaign, _token, _claimee)]; + } + + /// @inheritdoc IOngoingCampaigns + function totalAirdropped(bytes32 _campaign, IERC20 _token) external view returns (uint256) { + return _totalAirdroppedByCampaignAndToken[_getIdOfCampaignAndToken(_campaign, _token)]; + } + + /// @inheritdoc IOngoingCampaigns + function totalClaimed(bytes32 _campaign, IERC20 _token) external view returns (uint256) { + return _totalClaimedByCampaignAndToken[_getIdOfCampaignAndToken(_campaign, _token)]; + } + /// @inheritdoc IOngoingCampaigns function updateCampaign( bytes32 _campaign, @@ -41,10 +57,10 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { TokenAmount memory _tokenAllocation = _tokensAllocation[_i]; // Build our unique ID for campaign and token address. - bytes32 _campaignAndTokenId = getIdOfCampaignAndToken(_campaign, _tokenAllocation.token); + bytes32 _campaignAndTokenId = _getIdOfCampaignAndToken(_campaign, _tokenAllocation.token); // Move storage var to memory. - uint256 _currentTotalAirdropped = totalAirdroppedByCampaignAndToken[_campaignAndTokenId]; + uint256 _currentTotalAirdropped = _totalAirdroppedByCampaignAndToken[_campaignAndTokenId]; // We can not lower the amount of total claimable on a campaign since that would break // the maths for the "ongoing airdrops". @@ -59,7 +75,7 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { } // Update total claimable reward on campaign - totalAirdroppedByCampaignAndToken[_campaignAndTokenId] = _tokenAllocation.amount; + _totalAirdroppedByCampaignAndToken[_campaignAndTokenId] = _tokenAllocation.amount; // Refill contract with the ERC20 tokens _tokenAllocation.token.safeTransferFrom(msg.sender, address(this), _refillNeeded); @@ -117,17 +133,17 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { TokenAmount memory _tokenAmount = _tokensAmounts[_i]; // Build our unique ID for campaign, token and claimee address. - bytes32 _campaignTokenAndClaimeeId = getIdOfCampaignTokenAndClaimee(_campaign, _tokenAmount.token, _claimee); + bytes32 _campaignTokenAndClaimeeId = _getIdOfCampaignTokenAndClaimee(_campaign, _tokenAmount.token, _claimee); // Calculate to claim - _claimed[_i] = _tokenAmount.amount - amountClaimedByCampaignTokenAndClaimee[_campaignTokenAndClaimeeId]; + _claimed[_i] = _tokenAmount.amount - _amountClaimedByCampaignTokenAndClaimee[_campaignTokenAndClaimeeId]; _tokens[_i] = _tokenAmount.token; if (_claimed[_i] > 0) { // Update the total amount claimed of the token and campaign for the claimee - amountClaimedByCampaignTokenAndClaimee[_campaignTokenAndClaimeeId] = _tokenAmount.amount; + _amountClaimedByCampaignTokenAndClaimee[_campaignTokenAndClaimeeId] = _tokenAmount.amount; // Update the total claimed of a token on a campaign - totalClaimedByCampaignAndToken[getIdOfCampaignAndToken(_campaign, _tokenAmount.token)] += _claimed[_i]; + _totalClaimedByCampaignAndToken[_getIdOfCampaignAndToken(_campaign, _tokenAmount.token)] += _claimed[_i]; // Send the recipient the claimed tokens _tokenAmount.token.safeTransfer(_recipient, _claimed[_i]); } @@ -151,14 +167,14 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { IERC20 _token = _tokens[_i]; // Build our unique ID for campaign and token address. - bytes32 _campaignAndTokenId = getIdOfCampaignAndToken(_campaign, _token); + bytes32 _campaignAndTokenId = _getIdOfCampaignAndToken(_campaign, _token); // Understand how much is still available - _unclaimed[_i] = totalAirdroppedByCampaignAndToken[_campaignAndTokenId] - totalClaimedByCampaignAndToken[_campaignAndTokenId]; + _unclaimed[_i] = _totalAirdroppedByCampaignAndToken[_campaignAndTokenId] - _totalClaimedByCampaignAndToken[_campaignAndTokenId]; // We remove unecessary data so we get a little bit of gas back - delete totalClaimedByCampaignAndToken[_campaignAndTokenId]; - delete totalAirdroppedByCampaignAndToken[_campaignAndTokenId]; + delete _totalClaimedByCampaignAndToken[_campaignAndTokenId]; + delete _totalAirdroppedByCampaignAndToken[_campaignAndTokenId]; if (_unclaimed[_i] > 0) { // Transfer it out to recipient @@ -169,15 +185,15 @@ contract OngoingCampaigns is AccessControlDefaultAdminRules, IOngoingCampaigns { emit CampaignShutDown(_campaign, _tokens, _unclaimed, _recipient); } - function getIdOfCampaignAndToken(bytes32 _campaign, IERC20 _token) public pure returns (bytes32) { + function _getIdOfCampaignAndToken(bytes32 _campaign, IERC20 _token) internal pure returns (bytes32) { return keccak256(abi.encodePacked(_campaign, _token)); } - function getIdOfCampaignTokenAndClaimee( + function _getIdOfCampaignTokenAndClaimee( bytes32 _campaign, IERC20 _token, address _claimee - ) public pure returns (bytes32) { + ) internal pure returns (bytes32) { return keccak256(abi.encodePacked(_campaign, _token, _claimee)); } diff --git a/solidity/contracts/test/OngoingCampaigns.sol b/solidity/contracts/test/OngoingCampaigns.sol index e444fc3..849722c 100644 --- a/solidity/contracts/test/OngoingCampaigns.sol +++ b/solidity/contracts/test/OngoingCampaigns.sol @@ -35,7 +35,7 @@ contract OngoingCampaignsMock is OngoingCampaigns { IERC20 _token, uint256 _amount ) external { - totalAirdroppedByCampaignAndToken[getIdOfCampaignAndToken(_campaign, _token)] = _amount; + _totalAirdroppedByCampaignAndToken[_getIdOfCampaignAndToken(_campaign, _token)] = _amount; } function setTotalClaimedByCampaignAndToken( @@ -43,7 +43,7 @@ contract OngoingCampaignsMock is OngoingCampaigns { IERC20 _token, uint256 _amount ) external { - totalClaimedByCampaignAndToken[getIdOfCampaignAndToken(_campaign, _token)] = _amount; + _totalClaimedByCampaignAndToken[_getIdOfCampaignAndToken(_campaign, _token)] = _amount; } function _claim( diff --git a/solidity/interfaces/IOngoingCampaigns.sol b/solidity/interfaces/IOngoingCampaigns.sol index 037770c..aba8a52 100644 --- a/solidity/interfaces/IOngoingCampaigns.sol +++ b/solidity/interfaces/IOngoingCampaigns.sol @@ -77,27 +77,25 @@ interface IOngoingCampaigns { /** * @notice Exposes token amount claimed by user on a given campaign - * @dev This value cannot be modified - * @param campaignTokenAndClaimeeId Id built by hashing concatenated values of: Campaign name, token address and user address. * @return Amount claimed */ - function amountClaimedByCampaignTokenAndClaimee(bytes32 campaignTokenAndClaimeeId) external view returns (uint256); + function amountClaimed( + bytes32 campaign, + IERC20 token, + address claimee + ) external view returns (uint256); /** * @notice Total sum of all airdropped amounts of a given token and campaign. - * @dev This value cannot be modified - * @param campaignAndTokenId Id built by hashing concatenated values of: Campaign name and token address. * @return Sum of all airdropped amounts */ - function totalAirdroppedByCampaignAndToken(bytes32 campaignAndTokenId) external view returns (uint256); + function totalAirdropped(bytes32 campaign, IERC20 token) external view returns (uint256); /** * @notice Total amount claimed of a given token and campaign. - * @dev This value cannot be modified - * @param campaignAndTokenId Id built by hashing concatenated values of: Campaign name and token address. * @return Total amount claimed */ - function totalClaimedByCampaignAndToken(bytes32 campaignAndTokenId) external view returns (uint256); + function totalClaimed(bytes32 campaign, IERC20 token) external view returns (uint256); /** * @notice Updates campaign information: Tokens allocation and merkle root. diff --git a/test/unit/ongoing-campaigns.spec.ts b/test/unit/ongoing-campaigns.spec.ts index 8442823..4cce216 100644 --- a/test/unit/ongoing-campaigns.spec.ts +++ b/test/unit/ongoing-campaigns.spec.ts @@ -280,18 +280,14 @@ describe('OngoingCampaigns', () => { }); then('total amount claimed by campaign, token and claimee is updated', async () => { for (let i = 0; i < campaignTokens.length; i++) { - expect( - await ongoingCampaigns.amountClaimedByCampaignTokenAndClaimee( - getIdOfCampaignTokenAndClaimee(campaign, campaignTokens[i].address, claimees[0]) - ) - ).to.be.equal(claimeesAllocations[0][i].amount); + expect(await ongoingCampaigns.amountClaimed(campaign, campaignTokens[i].address, claimees[0])).to.be.equal( + claimeesAllocations[0][i].amount + ); } }); then('total claimed by campaign and token is updated', async () => { for (let i = 0; i < campaignTokens.length; i++) { - expect( - await ongoingCampaigns.totalClaimedByCampaignAndToken(getIdOfCampaignAndToken(campaign, campaignTokens[i].address)) - ).to.be.equal(claimeesAllocations[0][i].amount); + expect(await ongoingCampaigns.totalClaimed(campaign, campaignTokens[i].address)).to.be.equal(claimeesAllocations[0][i].amount); } }); then('sends correct amount of tokens to recipient', async () => { @@ -388,9 +384,7 @@ describe('OngoingCampaigns', () => { }); then('updates total airdropped amount by campaign and token', async () => { for (let i = 0; i < previousAllocations.length; i++) { - expect(await ongoingCampaigns.totalAirdroppedByCampaignAndToken(getIdOfCampaignAndToken(campaign, tokens[i].address))).to.be.equal( - newAllocations[i] - ); + expect(await ongoingCampaigns.totalAirdropped(campaign, tokens[i].address)).to.be.equal(newAllocations[i]); } }); then('transfers the correct amount to the contract', () => { @@ -439,12 +433,12 @@ describe('OngoingCampaigns', () => { }); then('total claimed by campaign and token gets removed', async () => { for (let i = 0; i < totalAirdropped.length; i++) { - expect(await ongoingCampaigns.totalClaimedByCampaignAndToken(getIdOfCampaignAndToken(campaign, tokens[i].address))).to.be.equal(0); + expect(await ongoingCampaigns.totalClaimed(campaign, tokens[i].address)).to.be.equal(0); } }); then('total airdropped by campaign and token gets removed', async () => { for (let i = 0; i < totalAirdropped.length; i++) { - expect(await ongoingCampaigns.totalAirdroppedByCampaignAndToken(getIdOfCampaignAndToken(campaign, tokens[i].address))).to.be.equal(0); + expect(await ongoingCampaigns.totalAirdropped(campaign, tokens[i].address)).to.be.equal(0); } }); then('transfers out the correct amount to the recipient', () => { @@ -506,12 +500,4 @@ describe('OngoingCampaigns', () => { root, }; } - - function getIdOfCampaignTokenAndClaimee(campaign: string, tokenAddress: string, claimeeAddress: string): string { - return ethers.utils.keccak256(`${campaign}${tokenAddress.slice(2)}${claimeeAddress.slice(2)}`); - } - - function getIdOfCampaignAndToken(campaign: string, tokenAddress: string): string { - return ethers.utils.keccak256(`${campaign}${tokenAddress.slice(2)}`); - } }); diff --git a/test/utils/bdd.ts b/test/utils/bdd.ts index bbfa3b6..cd47620 100644 --- a/test/utils/bdd.ts +++ b/test/utils/bdd.ts @@ -1,4 +1,4 @@ -import { Suite, SuiteFunction, it, beforeEach, TestFunction, Func } from 'mocha'; +import { Suite, SuiteFunction, Func } from 'mocha'; export const then = (title: string, fn?: Func) => { if (typeof title === 'string') {