Skip to content

Commit

Permalink
fix: fixing comments in PR
Browse files Browse the repository at this point in the history
  • Loading branch information
@EduardoMelo00
EduardoMelo00 committed Sep 17, 2024
1 parent dbd6b60 commit 0807f1e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 54 deletions.
10 changes: 4 additions & 6 deletions contracts/ERC20Splitter.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract ERC20Splitter is ReentrancyGuard {
uint16[][] shares,
address[][] recipients
);
event Withdraw(address indexed user, uint256[] amounts);
event Withdraw(address indexed user, address[] tokenAddresses, uint256[] amounts);

uint16 public constant MAX_SHARES = 10000;

Expand Down Expand Up @@ -62,7 +62,7 @@ contract ERC20Splitter is ReentrancyGuard {
/// Tokens are automatically determined based on previous deposits.
function withdraw() external nonReentrant {
address payable to = payable(msg.sender);
address[] storage senderTokens = userTokens[to];
address[] storage senderTokens = userTokens[msg.sender];

if (senderTokens.length == 0) {
return;
Expand All @@ -77,8 +77,7 @@ contract ERC20Splitter is ReentrancyGuard {
balances[tokenAddress][to] = 0;

if (tokenAddress == address(0)) {
(bool success, ) = to.call{ value: amount }('');
require(success, 'ERC20Splitter: Failed to send Ether');
to.transfer(amount);
} else {
require(
IERC20(tokenAddress).transferFrom(address(this), to, amount),
Expand All @@ -90,10 +89,9 @@ contract ERC20Splitter is ReentrancyGuard {

delete hasToken[to][tokenAddress];
}
emit Withdraw(to, userTokens[msg.sender], withdrawnAmounts);

delete userTokens[to];

emit Withdraw(to, withdrawnAmounts);
}

/** Internal Functions **/
Expand Down
61 changes: 13 additions & 48 deletions test/SplitterContract.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,19 +197,6 @@ describe('ERC20Splitter', () => {
)
})

it('Should revert when ERC20 transferFrom fails during deposit', async () => {
const tokenAmount = ethers.parseEther('100')
const tokenAddresses = [await mockERC20.getAddress()]
const amounts = [tokenAmount]
const shares = [[10000]] // 100% share
const recipients = [[recipient1.address]]

await mockERC20.transferReverts(true, 0)
await expect(splitter.connect(owner).deposit(tokenAddresses, amounts, shares, recipients)).to.be.revertedWith(
'ERC20Splitter: Transfer failed',
)
})

it('Should handle multiple native token (ETH) deposits in a single transaction', async () => {
const ethShares = [
[5000, 5000],
Expand Down Expand Up @@ -291,9 +278,9 @@ describe('ERC20Splitter', () => {
it('Should allow a recipient to withdraw their split ERC20 tokens without specifying token addresses', async () => {
await expect(splitter.connect(recipient1).withdraw())
.to.emit(splitter, 'Withdraw')
.withArgs(recipient1.address, [ethers.parseEther('50')])
.withArgs(recipient1.address, [await mockERC20.getAddress()], [ethers.parseEther('50')])

expect(await splitter.balances(await mockERC20.getAddress(), recipient1.address)).to.equal(0)
expect(await splitter.balances(mockERC20.getAddress(), recipient1.address)).to.equal(0)
})

it('Should allow a recipient to withdraw their split native tokens (ETH) and ERC20 tokens', async () => {
Expand All @@ -308,47 +295,18 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient1.address,
[await mockERC20.getAddress(), AddressZero],
[ethers.parseEther('50'), ethers.parseEther('0.5')], // 50 ERC20 tokens and 0.5 ETH
)

expect(await splitter.balances(await mockERC20.getAddress(), recipient1.address)).to.equal(0)
expect(await splitter.balances(AddressZero, recipient1.address)).to.equal(0)
expect(await splitter.balances(mockERC20.getAddress(), recipient1.address)).to.equal(0)
})

it('Should handle withdraw() when user has no tokens', async () => {
await splitter.connect(anotherUser).withdraw()
})

it('Should revert when sending Ether to a recipient fails', async () => {
const malicious = await maliciousRecipient.getAddress()

await network.provider.request({
method: 'hardhat_impersonateAccount',
params: [malicious],
})

const ethAmount = ethers.parseEther('1')
const tokenAddresses = [ethers.ZeroAddress] // Ether represented by address zero
const amounts = [ethAmount]
const shares = [[10000]] // 100% share
const recipients = [[maliciousRecipient.getAddress()]]

await splitter.connect(owner).deposit(tokenAddresses, amounts, shares, recipients, {
value: ethAmount,
})

const maliciousSigner = await ethers.getSigner(malicious)

await network.provider.send('hardhat_setBalance', [
malicious,
ethers.toQuantity(ethers.parseEther('1')), // Setting 2 Ether
])

// Attempt to withdraw as the malicious recipient
await expect(splitter.connect(maliciousSigner).withdraw()).to.be.revertedWith(
'ERC20Splitter: Failed to send Ether',
)
})
it('Should revert when ERC20 transferFrom fails during withdraw', async () => {
const mockERC20false = await mockERC20.getAddress()

Expand Down Expand Up @@ -391,7 +349,7 @@ describe('ERC20Splitter', () => {
it('Should allow a recipient to withdraw their split ERC20 tokens without specifying token addresses', async () => {
await expect(splitter.connect(recipient1).withdraw())
.to.emit(splitter, 'Withdraw')
.withArgs(recipient1.address, [ethers.parseEther('50')])
.withArgs(recipient1.address, [await mockERC20.getAddress()], [ethers.parseEther('50')])

expect(await splitter.balances(mockERC20.getAddress(), recipient1.address)).to.equal(0)
})
Expand All @@ -408,6 +366,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient1.address, // Expect both ERC-20 and native token
[await mockERC20.getAddress(), AddressZero],
[ethers.parseEther('50'), ethers.parseEther('0.5')], // 50 ERC20 tokens and 0.5 ETH
)

Expand All @@ -431,6 +390,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient1.address,
[AddressZero],
[ethers.parseEther('0.5')], // Expect 0.5 ETH (50% of 1 ETH)
)

Expand Down Expand Up @@ -462,6 +422,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient1.address,
[AddressZero],
[ethers.parseEther('1')], // Full 1 ETH
)

Expand All @@ -471,6 +432,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient2.address,
[await mockERC20.getAddress()],
[ethers.parseEther('50')], // 50% of ERC-20 tokens
)

Expand All @@ -480,6 +442,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient3.address,
[await mockERC20.getAddress()],
[ethers.parseEther('50')], // 50% of ERC-20 tokens
)

Expand Down Expand Up @@ -520,6 +483,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient1.address,
[AddressZero],
[ethers.parseEther('0.5')], // 50% of 1 ETH
)

Expand All @@ -531,6 +495,7 @@ describe('ERC20Splitter', () => {
.to.emit(splitter, 'Withdraw')
.withArgs(
recipient2.address,
[AddressZero, await mockERC20.getAddress()],
[ethers.parseEther('0.5'), ethers.parseEther('60')], // 50% of 1 ETH and 60 ERC-20 tokens
)

Expand All @@ -541,7 +506,7 @@ describe('ERC20Splitter', () => {
it('Should allow recipient3 to withdraw only ERC-20 tokens', async () => {
await expect(splitter.connect(recipient3).withdraw())
.to.emit(splitter, 'Withdraw')
.withArgs(recipient3.address, [ethers.parseEther('40')])
.withArgs(recipient3.address, [await mockERC20.getAddress()], [ethers.parseEther('40')])

expect(await splitter.balances(mockERC20.getAddress(), recipient3.address)).to.equal(0)
})
Expand Down

0 comments on commit 0807f1e

Please sign in to comment.