Allow EIP2612, gas-less approvals in the staking flow #50
Conversation
Doy-lee
force-pushed
the
eip-2612-gasless-approval
branch
from
239b498
to
484d626
Compare
| bytes32 r, | ||
| bytes32 s | ||
| ) public onlyOperator { | ||
| IERC20Permit(address(SENT)).permit(msg.sender, address(this), amount, deadline, v, r, s); |
There was a problem hiding this comment.
In theory you should be able to do SENT.permit(...)
Also, I agree with zeppelin's example: I think that "try to use the permit, but continue regardless" is ok. If the meat of "contribute funds" fails, the worst case is the same as if the eth wallet that triggered this already did SENT.approve(...).
There was a problem hiding this comment.
No bueno cause they're just interfaces, SENT is instantiated as IERC20 which lets us use transferFrom, approve e.t.c but not permit since that's part of IERC20Permit interface.
There was a problem hiding this comment.
I've added the try-catch, it took the weekend but I get why they want it, specifically to allow the code to run incase someone front-runs, someone can permanently deny you from interacting with the contract if they copied just the permit input data and constantly front-run someone if they observe the TX in the mempool.
| bytes32 r, | ||
| bytes32 s | ||
| ) public { | ||
| IERC20Permit(address(SENT)).permit(msg.sender, address(this), amount, deadline, v, r, s); |
There was a problem hiding this comment.
If this can be SENT.permit(...) I would prefer that. If not, I'm curious as to why this (what you've put, which I assume works) works but SENT.permit(...) does not work.
contracts/ServiceNodeRewards.sol
Outdated
| bytes32 r, | ||
| bytes32 s | ||
| ) external whenNotPaused { | ||
| IERC20Permit(address(designatedToken)).permit(msg.sender, address(this), _stakingRequirement, deadline, v, r, s); |
There was a problem hiding this comment.
Similar to my question above, it feels like this should be SENT.permit(...)
|
re: The interface problem, the solution is to have an interface for SENT but this turns into busy work because you have to go and replicate the entire interface of ERC20 and ERC20Permit since there's no inheritance for interface, rather than just cast to the necessary interface 3 times. |
Note C++ tests are failing because they need the updated Ethyl branch of which I have a PR siting that I think is still failing and needs to be tweaked more.