-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add EIP-6228: Extreme EIP-20 (meta-transaction token MTT) #6228
Conversation
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s): (pass) eip-3561.md
(fail) eip-6228.md
(pass) assets/eip-3561/README.md
(pass) assets/eip-3561/TrustMinimizedProxy.js
(pass) assets/eip-3561/contracts/MockLogic.sol
(pass) assets/eip-3561/contracts/TrustMinimizedProxy.sol
(pass) assets/eip-3561/fixtures.js
|
## Test Cases | ||
|
||
[Unit tests](../assets/eip-3561/). | ||
|
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.
Please make these changes in a separate PR.
title: Extreme ЕRС-20 | ||
description: meta-transaction token (MTT) |
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.
These are both way too vague. You need to give readers some kind of idea about what your proposal is about.
|
||
## Abstract | ||
|
||
This standard proposes a solution in which meta-transaction fees are deducted directly from the user' token transfer amount, no liquidity for keeper incentives required. |
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.
This standard proposes a solution in which meta-transaction fees are deducted directly from the user' token transfer amount, no liquidity for keeper incentives required. | |
This standard proposes a solution in which meta-transaction fees are deducted directly from the user's token transfer amount, no liquidity for keeper incentives required. |
This EIP is based on these assumptions: | ||
|
||
- That meta-transaction signatures need to be public in order to speed up blockchain adoption. The EIP assumes that token dApps will feature a public pool of signatures similar to Coinbase or Etherscan, which will resemble regular Ethereum transactions but with reduced user friction. | ||
- Meta-transactions must become standard procedure when interacting with smart contracts. | ||
- Gas efficiency is in no way less important when it comes to optimistic and zero knowledge roll-ups. If anything, lower network fees enable more automation, which is about to become essential in such a hyper-competitive industry. | ||
- Full automation can easily be too costly for project funding. Therefore, meta-transactions fees should be deducted directly from the transfer amount. | ||
|
||
An example: | ||
Alice has never had a wallet before but is eligible for an airdrop on Arbitrum. All she needs to do is install a wallet, sign the data, and then she has airdrop tokens minus the keeper fee. She does not have to make any transactions or possess any ETH. She is not limited in any way more than Bob, who is transacting on his own. Alice can vote, stake, claim rewards, trade on Uniswap. In fact, she can do more than Bob, such as subscribing to staking rewards to get them automatically every month, a part of these rewards then automatically sent to pay her bills and support her favorite content creators. |
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.
This doesn't belong in the abstract. The abstract should be a terse technical summary of the EIP.
- All existing solutions rely on funding/supporting a relayer/keeper network, which makes innovation more dependent on external funding. | ||
|
||
## Specification | ||
|
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.
You should try to use the MUST/SHOULD/etc. terms and include the blurb from eip-template.md.
|
||
## Specification | ||
|
||
Signatories can create executable signatures for transactions of any kind. As long as signatory `address(this)` token balance or `returnToken`(token returned after after execution) balance is higher than keeper fee of a transaction at hand, and as long as remaining value after keeper fee transaction is above `minOut`, it is executable. |
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.
While I'm generally ok with jumping straight into technical content, reading EIP-20 and the text of this proposal up to this point doesn't give enough background for me to understand all the terms you're using. Are these terms/patterns common in the meta-transaction world? I'd really like to see some definitions and background information included if possible.
uint initGasLeft = gasleft();// gas comes first | ||
uint gasPrice; | ||
assembly { | ||
gasPrice := gasprice() | ||
} | ||
require(!users[reqt.signer].forbidAll, 'forbid all is active'); | ||
_verifySig(reqt); | ||
// check if allowed to use this signature | ||
require(block.number > users[reqt.signer].indexLastClaims[reqt.index] + reqt.interval, 'sig cant be valid yet'); | ||
require(users[reqt.signer].forbiddenSaltsBitMap[reqt.salt / 256] & (1 << reqt.salt % 256) == 0, 'forbidden salt'); | ||
require(reqt.interval >= 86400);// in seconds, this is to avoid griefing | ||
// record state change if any | ||
if (reqt.interval > 0) { | ||
users[reqt.signer].indexLastClaims[reqt.index] = block.number; // its a subscription, track last execution time | ||
} else { | ||
_forbidSalt(reqt.signer, reqt.salt);// it was a one time transaction, forbid this salt | ||
} | ||
// call | ||
// if data is 0, subtract keeper reward from user balance here | ||
// otherwise data should return amount which is then being split between the user and the keeper | ||
bytes memory data; | ||
bool success; | ||
if (reqt.needsHelper) { | ||
(success, data) = I(_ab.helper).handle{value: msg.value}(reqt); | ||
require(success == true, 'helper failed'); | ||
} else { | ||
(success, data) = reqt.callee.call{value: msg.value}(abi.encodeWithSelector(reqt.selector, reqt.callData)); | ||
require(success == true, 'direct call failed'); | ||
} | ||
// take fee and complete | ||
uint amount; // compute amount from data bytes | ||
assembly { | ||
amount := mload(add(data, 0x20)) | ||
} | ||
uint approxGas = (initGasLeft - gasleft() /*+ 21000 */) * gasPrice; | ||
uint keeperReward = _calculateAmountInNativeToken(reqt.returnToken, approxGas + reqt.tip); | ||
require(amount - keeperReward >= reqt.minOut); | ||
if (reqt.returnToken == address(this)) { | ||
users[msg.sender].balance += keeperReward; | ||
users[reqt.signer].balance += amount - keeperReward; | ||
} else { | ||
if (reqt.returnToken == address(0)) { | ||
(success, data) = payable(msg.sender).call{value: keeperReward}(''); | ||
require(success); | ||
(success, data) = payable(reqt.signer).call{value: amount - keeperReward}(''); | ||
require(success); | ||
} else { | ||
I(reqt.returnToken).transfer(msg.sender, keeperReward); | ||
I(reqt.returnToken).transfer(reqt.signer, amount - keeperReward); | ||
} | ||
} |
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.
uint initGasLeft = gasleft();// gas comes first | |
uint gasPrice; | |
assembly { | |
gasPrice := gasprice() | |
} | |
require(!users[reqt.signer].forbidAll, 'forbid all is active'); | |
_verifySig(reqt); | |
// check if allowed to use this signature | |
require(block.number > users[reqt.signer].indexLastClaims[reqt.index] + reqt.interval, 'sig cant be valid yet'); | |
require(users[reqt.signer].forbiddenSaltsBitMap[reqt.salt / 256] & (1 << reqt.salt % 256) == 0, 'forbidden salt'); | |
require(reqt.interval >= 86400);// in seconds, this is to avoid griefing | |
// record state change if any | |
if (reqt.interval > 0) { | |
users[reqt.signer].indexLastClaims[reqt.index] = block.number; // its a subscription, track last execution time | |
} else { | |
_forbidSalt(reqt.signer, reqt.salt);// it was a one time transaction, forbid this salt | |
} | |
// call | |
// if data is 0, subtract keeper reward from user balance here | |
// otherwise data should return amount which is then being split between the user and the keeper | |
bytes memory data; | |
bool success; | |
if (reqt.needsHelper) { | |
(success, data) = I(_ab.helper).handle{value: msg.value}(reqt); | |
require(success == true, 'helper failed'); | |
} else { | |
(success, data) = reqt.callee.call{value: msg.value}(abi.encodeWithSelector(reqt.selector, reqt.callData)); | |
require(success == true, 'direct call failed'); | |
} | |
// take fee and complete | |
uint amount; // compute amount from data bytes | |
assembly { | |
amount := mload(add(data, 0x20)) | |
} | |
uint approxGas = (initGasLeft - gasleft() /*+ 21000 */) * gasPrice; | |
uint keeperReward = _calculateAmountInNativeToken(reqt.returnToken, approxGas + reqt.tip); | |
require(amount - keeperReward >= reqt.minOut); | |
if (reqt.returnToken == address(this)) { | |
users[msg.sender].balance += keeperReward; | |
users[reqt.signer].balance += amount - keeperReward; | |
} else { | |
if (reqt.returnToken == address(0)) { | |
(success, data) = payable(msg.sender).call{value: keeperReward}(''); | |
require(success); | |
(success, data) = payable(reqt.signer).call{value: amount - keeperReward}(''); | |
require(success); | |
} else { | |
I(reqt.returnToken).transfer(msg.sender, keeperReward); | |
I(reqt.returnToken).transfer(reqt.signer, amount - keeperReward); | |
} | |
} |
You shouldn't include any implementation in the specification section, only in the Reference Implementation.
} | ||
``` | ||
|
||
### Method \_calculateAmountInNativeToken(address returnToken, uint amount) |
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.
### Method \_calculateAmountInNativeToken(address returnToken, uint amount) | |
### Method `_calculateAmountInNativeToken(address returnToken, uint amount)` |
use backticks for inline code
As per previous related EIPs, replayable public signatures can be made secure enough for trust-minimized 0-liability use, which is the most economically efficient approach for start-ups. | ||
|
||
Another way to improve the user experience is to reduce the meta-transaction fee by making `address(this)` always indebted to the keepers, i.e. the keeper never receives the full fee after executing. This, however, would require at least one additional `SSTORE`, therefore requires more elaborate argumentation to consider. | ||
|
||
Adding user whitelists of allowed paths is not the best way to increase user security as of now since user whitelists reintroduce the same user friction this EIP tries to avoid. Since if a whitelist can be edited by signature, then it's practically useless, it needs an actual transaction from the user. | ||
|
||
Global path whitelists are also ineffective unless they are made to be extremely restrictive, as a malicious signature can easily bypass them if a user is tricked into signing malicious data. Some major dApps provide functions with alterable destination address. To make the most of this EIP with restrictive global paths, it would be best to block all such dApps by default and give users the choice to enable them for personal use. | ||
|
||
A check for only authorized keepers is a way to greatly enhance security, in cost-efficient manner. However, authorized keepers then would need to KYC, which is not the most optimal solution. With pseudo-anonymous approach it looks a tiny bit better: choosing authorized keepers among those who frequently enough transact sufficient value from/to addresses of KYC-required services such as Coinbase exchange or Binance, it is still easily a questionable design solution too. Another option: only keepers with locked funds are eligible to transact on behalf of the users, however this case still requires trust for non-anonymous centralized governance, which is inefficient in many ways, and of all other things assumes liability, which is too expensive(insurance, support employees). | ||
|
||
Getting rid of method abstraction and restricting meta-transactions to predetermined token-related paths is a way to decrease overall gas cost and to protect user funds from being stolen, as long as associated contracts' functions do not have an option to alter destination address to any address. However, this will make meta-transactions a cellular phenomenon and restrict adoption of DeFi. |
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.
A lot of this reads more like Motivation, and not Rationale. The Rationale section should explain specific technical choices made within the EIP, while the Motivation section should justify the EIP as a whole.
@@ -0,0 +1 @@ | |||
Dedicated repo with hardhat tests and easy install: https://github.com/SamPorter1984/EIP-3561-tests |
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.
Please remove any external links.
Dedicated repo with hardhat tests and easy install: https://github.com/SamPorter1984/EIP-3561-tests |
@@ -0,0 +1,188 @@ | |||
pragma solidity >=0.8.0; //important |
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.
This is from a different EIP.
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment. |
This standard proposes a solution in which meta-transaction fees are deducted directly from the user' token transfer amount, no liquidity for keeper incentives required.