Initial Thoughts
address _owner;
This can be removed as well as setting it in the constructor, as you can query the Ownable contract for the contract owner.
uint256 public counter;
This line can be removed because 'counter' isn't used anywhere in your code.
uint _minammount = 5 * (10**18);
I would place this with your other state variables near the top, as it will enable you to adjust this value later down the road, if needed.
require(_ammount >= _minammount,"Amount less than minimum amount" );
This line will sufficiently enforce a minimum amount.
IERC20(token).transferFrom(msg.sender, _owner, _ammount);
This will attempt to make a transfer for the token you included in your constructor, and that token only.
Recommendation
I would make a deposit function that takes any token address, checks it against a mapping that will act as an allowlist, check for allowance, and then processes the transfer.
Here's my implementation:
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
contract XYZ is Ownable{
mapping(address => bool) public allowedTokens;
uint public _minAmount;
constructor(uint minAmount){
_minAmount = minAmount;
}
function allowAddress(address _token) public onlyOwner {
allowedTokens[_token] = true;
}
function deposit(address _token, uint _amount) public payable {
require(allowedTokens[_token] == true, "Token is not allowed/supported");
require(_amount >= _minAmount, "Amount less than minimum amount");
require(IERC20(token).allowance(msg.sender, address(this)), "Not approved to send balance requested");
bool success = IERC20(token).transferFrom(msg.sender, address(this), _amount);
require(success, "Transaction was not successful");
}
}
You can always reference your code's contract address with 'address(this)'. It is actually a gas optimization to set address(this) to an immutable address state variable and use that instead.
I added an additional function that would allow you to allow more addresses in the future. I would also add a function that allows you to remove them, if that fits the scope of your project. You can optionally set the token addresses in the constructor.
Checking if the supplied token is in your allowlist allows you to write open code where you can easily add additional tokens later on. Additionally, checking for stuff like allowance is a good idea as it will basically tell you if the transfer is even possible before you run it.
The transferFrom ERC20 function returns a boolean. You can check this boolean to double check the transfer was successful.