add erc transfer from native #987
Conversation
|
🛑 |
|
The commit 8380220 (as a parent of 9a7b129) contains errors. |
| status: Draft | ||
| type: Standards | ||
| category: ERC | ||
| created: <TODO> |
There was a problem hiding this comment.
This should contain the date on which the proposal was initiated, regardless of its state.
| MUST emit the following event on success: | ||
|
|
||
| ```solidity | ||
| event TransferFromNative(address indexed from, address indexed to, uint256 value); | ||
| ``` |
There was a problem hiding this comment.
Could clarify on what happens if amount = 0 re the event.
I think skip the emit and just return true?
|
|
||
| MUST verify whether `msg.sender` (the caller) is authorized to execute the Native transfer. | ||
|
|
||
| MUST handle the Native transfer and return false on failure. |
There was a problem hiding this comment.
Could clarify on revert or return false (or perhaps either).
Imo, leaning more towards reverting, cleaner to implement.
Add rationale for choice.
| ```solidity | ||
| interface ERC165 { | ||
| /// @notice Query if a contract implements an interface | ||
| /// @param interfaceID The interface identifier, as specified in ERC-165 | ||
| /// @dev Interface identification is specified in ERC-165. This function | ||
| /// uses less than 30,000 gas. | ||
| /// @return `true` if the contract implements `interfaceID` and | ||
| /// `interfaceID` is not 0xffffffff, `false` otherwise | ||
| function supportsInterface(bytes4 interfaceID) external view returns (bool); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
imo, to avoid inheritance headaches, would prefer if there's some way to recycle one of the existing functions. Then no need for 165.
I think if we have some nativeAllowance(address spender), we can use it as a signaling mechanism. If nativeAllowance is to be implemented, maybe nativeAllowanceTransient should be implemented to differentiate? Or maybe nativeAllowance could be the saturating sum of the transient allowance and regular allowance.
Technically, transferFromNative with a zero amount could be used to signal too (if it doesn't emit any event), but implementing a staticcall to a non-view function in plain Solidity is ugly.
|
|
||
| ### Optional Transient Approval | ||
|
|
||
| For cases where approval should only persist within a single transaction, a transient approval mechanism MAY be used. This allows a user to grant temporary approval to a `spender` for a single transaction, without modifying persistent storage. |
There was a problem hiding this comment.
I think need clarity on which native allowance to deduct first. Would prefer transient to be deducted first. Cuz TLOAD costs less than SLOAD.
Also not sure if it is the saturating sum of transient and regular allowances. Or perhaps some other formula.
There was a problem hiding this comment.
Ah that is interesting. Yeah definitely needs some outline for order of operations. My gut impl would be to default first to transient like you said.
I'm not sure how we should handle the case then if there is more requested than the transient amount allowed. Do you then pull from the persisted allotment? It feels like that may be user error of the transient allotment if the caller didn't request enough to be allowed transiently?
There was a problem hiding this comment.
i think it would be cleaner if we just separated them - transferFromNative and transferFromNativeTransient. that way it's easier to know which allowance to subtract from
| To modify this approval, the following function MUST be implemented. | ||
|
|
||
| ```solidity | ||
| function approveNative(address spender, uint256 amount) external returns (bool); |
There was a problem hiding this comment.
would also be good to include allowance(address spender) and transientAllowance(address spender) in the interface
| - `recipient`: The recipient of the Native transfer. | ||
| - `amount`: The amount of Native to transfer. | ||
|
|
||
| SHOULD early return when amount == 0, otherwise anyone can execute any fallback function on any contract on behalf of the user without explicit approval. |
There was a problem hiding this comment.
Should early return with true, same for transient function
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: