Huy airdrop

[huy525]

This is my airdrop, any feedback is much appreciated!

[vtd12]

the function depositNativeToken() can be replaced by a fallback function. depositERC20 should have onlyOwner.
Some unchecked functions can lead to unexpected scenarios, but I suppose you checked it in ERC20 before. No withdraw all feature. A small thing is that I don't know the native token has erc20 interface lol. The test cases are new to me when you use attacker contracts to test! You can explicitly calculate the gas for exact testing.

[huy525]

Thanks for your comments!

the function depositNativeToken() can be replaced by a fallback function. depositERC20 should have onlyOwner.

This is kinda intended lol. I wanted to enforce the way to deposit and to give generous ppl a way to deposit if they wanted to haha.

Some unchecked functions can lead to unexpected scenarios, but I suppose you checked it in ERC20 before.

I don't really understand, like which ones?

No withdraw all feature.

Yeah just added lol. Didn't add withdraw for all tokens though.

A small thing is that I don't know the native token has erc20 interface lol.

No...? I don't think it does?

You can explicitly calculate the gas for exact testing.

Ah, nice! Thanks!

[hruiyang]

Overall the code and design is very sound! I don't think there are any major flaws with it. (Consdering the intend function of having donors for the deposit functions!)

Something really good is actually testing using other smart contracts for Reentrancy, etc! The smart contract is simple and sound, which helps a lot!

Some nitpicky stuff I guess:

1. There's a constant repetition of this line: require(totalBalanceERC20(tokenAddress) >= amount, "Insufficient total balance") and require(totalBalanceNativeToken() >= amount, "Insufficient total balance"). Maybe you could add this into _claim. If you would like _claim to be modular and only to be involved in claiming and transferring of nativeToken/ERC20 into the claimant then you could just add another internal function like _claimTo() which has both _claim and the require statement. This way you can save on the lines which are duplicated (and difficult to keep track if you have to change one of them)
2. For the depositNativeToken, maybe one way is to convert it into an internal function _depositNativeToken and then put it into the receive() function. This way, it'll always emit whatever you need, (including if you need to add state checking or state updates etc) and with the receive() function you can just transfer straight to the smart contract instead of having to call the function!
3. You can create an interface for TokenDistributor
4. Correct me if I'm wrong! You deposit native token or ERC20 into the smart contract -> then you call the functions to allocate to the user(or vice versa) -> user comes in and claims it. What if someone fat fingered and gave more ERC20 tokens or ETH into the smart contract? Or if the user doesn't claim it! This means that the tokens/native token will be stuck inside the smart contract forever!

[huy525]

Overall the code and design is very sound! I don't think there are any major flaws with it. (Consdering the intend function of having donors for the deposit functions!)

Something really good is actually testing using other smart contracts for Reentrancy, etc! The smart contract is simple and sound, which helps a lot!

Some nitpicky stuff I guess:

1. There's a constant repetition of this line: require(totalBalanceERC20(tokenAddress) >= amount, "Insufficient total balance") and require(totalBalanceNativeToken() >= amount, "Insufficient total balance"). Maybe you could add this into _claim. If you would like _claim to be modular and only to be involved in claiming and transferring of nativeToken/ERC20 into the claimant then you could just add another internal function like _claimTo() which has both _claim and the require statement. This way you can save on the lines which are duplicated (and difficult to keep track if you have to change one of them)
2. For the depositNativeToken, maybe one way is to convert it into an internal function _depositNativeToken and then put it into the receive() function. This way, it'll always emit whatever you need, (including if you need to add state checking or state updates etc) and with the receive() function you can just transfer straight to the smart contract instead of having to call the function!
3. You can create an interface for TokenDistributor
4. Correct me if I'm wrong! You deposit native token or ERC20 into the smart contract -> then you call the functions to allocate to the user(or vice versa) -> user comes in and claims it. What if someone fat fingered and gave more ERC20 tokens or ETH into the smart contract? Or if the user doesn't claim it! This means that the tokens/native token will be stuck inside the smart contract forever!

Thanks!
1/2/3. Great advices, thanks a lot!
4. I did not think of that lol. But I guess the owner can always get corrupted and airdrop + claim everything to himself/herself (not sure if this is a good thing lol).