-
Notifications
You must be signed in to change notification settings - Fork 0
ERC20Supra Smart Contract #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/multisig
Are you sure you want to change the base?
Conversation
31f1c84 to
c5e6b37
Compare
| } | ||
|
|
||
| /// @notice Allows a user to send native tokens directly. | ||
| receive() external payable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the use-cases of the reveive() in automation context ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aregng my understanding is that this is not automation specific. A user can send native token to a contract via native value transfer, which will result in invocation of payable method of the target contract. @udityadav-supraoracles to confirm. However, question is, why should we have two methods then for Native 2 ERC20 conversion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if an EOA sends native token directly to smart contract addressreceive gets executed and it'll mint ERC20Supra tokens.
Either they can send native token directly or they can invoke function deposit(). We can restrict to only deposit if you say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have any particular opinion on this , so let's keep both unless there is a valid reason not to have any of them.
| if (msg.value == 0) revert InvalidAmount(); | ||
| _mint(msg.sender, msg.value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does msg.value implicitly deducts money from sender balance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since, it's a payable function they're sending the money along with the tx. msg.value only refers to that amount being sent. Here, it'll mint ERC20Supra tokens equivalent to msg.value i.e. 1:1.
| } | ||
|
|
||
| /// @notice Allows a user to send native tokens directly. | ||
| receive() external payable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aregng my understanding is that this is not automation specific. A user can send native token to a contract via native value transfer, which will result in invocation of payable method of the target contract. @udityadav-supraoracles to confirm. However, question is, why should we have two methods then for Native 2 ERC20 conversion?
| assertEq(token.owner(), owner); | ||
| assertEq(token.name(), "ERC20Supra"); | ||
| assertEq(token.symbol(), "SUPRA"); | ||
| assertEq(token.decimals(), 18); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@so-schen , do we have 18 decimal precision for native $SUPRA on EVM? (CC: @udityadav-supraoracles @aregng )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes , according to the following comment and normalization equation between native-coins of VMs.
https://github.com/Entropy-Foundation/smr-moonshot/blob/e2e69c2f7f81b3bc38cf960a133d49f65ba6de97/crates/chainspec/src/lib.rs#L9
| import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; | ||
| import "@openzeppelin/contracts/access/Ownable2Step.sol"; | ||
|
|
||
| contract ERC20Supra is ERC20, ERC20Burnable, Ownable2Step, ERC20Permit { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@udityadav-supraoracles I do not see method to delegate ERC20 to other address etc, are those functionalities handled in Parent contract?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if a user wants to delegate the authority to spend their ERC20 they can call approve which is defined in parent ERC20.
| } | ||
|
|
||
| /// @notice Allows a user to send native tokens directly. | ||
| receive() external payable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not have any particular opinion on this , so let's keep both unless there is a valid reason not to have any of them.
| assertEq(token.owner(), owner); | ||
| assertEq(token.name(), "ERC20Supra"); | ||
| assertEq(token.symbol(), "SUPRA"); | ||
| assertEq(token.decimals(), 18); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes , according to the following comment and normalization equation between native-coins of VMs.
https://github.com/Entropy-Foundation/smr-moonshot/blob/e2e69c2f7f81b3bc38cf960a133d49f65ba6de97/crates/chainspec/src/lib.rs#L9
This PR includes ERC20Supra smart contract and test cases for it. Related to Entropy-Foundation/smr-moonshot#2525