Skip to content
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-5748: Approval Expiration for EIP-20 Tokens #5748

Closed
wants to merge 38 commits into from

Conversation

Mr-Lucky
Copy link

@Mr-Lucky Mr-Lucky commented Oct 4, 2022

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:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

@Mr-Lucky Mr-Lucky requested a review from eth-bot as a code owner October 4, 2022 15:31
@github-actions github-actions bot added c-new Creates a brand new proposal e-number Waiting on EIP Number assignment s-draft This EIP is a Draft labels Oct 4, 2022
@eth-bot
Copy link
Collaborator

eth-bot commented Oct 4, 2022

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):


(fail) eip-5748.md

classification
newEIPFile

(pass) assets/eip-5748/eip-5748.sol

classification
ambiguous
  • file assets/eip-5748/eip-5748.sol is associated with EIP 5748; because there are also changes being made to EIPS/eip-5748.md all changes to corresponding assets are also allowed

@Mr-Lucky Mr-Lucky changed the title add eip-draft_erc20_approval_exxpiration add eip-draft_erc20_approval_expiration Oct 4, 2022
@BigBro17on25tolife
Copy link

BigBro17on25tolife commented Oct 4, 2022 via email

EIPS/eip-draft_erc20_approval_expiration.md Outdated Show resolved Hide resolved
EIPS/eip-draft_erc20_approval_expiration.md Outdated Show resolved Hide resolved
eip: <to be assigned>
title: Approval Expirations for ERC20 Tokens
description: This standard introduces a standardized way for users to set expiration limits on ERC-20 token approvals.
author: Go+ Security <@GoPlusSecurity>, Jeff Hall <@JeffHal25193696>, Xavi <@XaaaaavitheFool>, Turan Vural <@turanzv>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github names need to be wrapped in parentheses like this lightclient (@lightclient).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixing

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be reviewed. There is a problem between the account and management account is being reviewed

turanzv and others added 2 commits October 6, 2022 18:19
Approval Expirations for ERC20 Tokens -> Approval Expiration for ERC-20 Tokens

Co-authored-by: lightclient <[email protected]>
Co-authored-by: lightclient <[email protected]>
@github-actions github-actions bot removed the e-number Waiting on EIP Number assignment label Oct 7, 2022
@turanzv
Copy link

turanzv commented Oct 7, 2022

@lightclient we're hitting CI failures with the "EIP Walidator". Our references to ERC-20 are being flagged as violations. Should we be referring to ERC-20 as EIP-20?

@Pandapip1 Pandapip1 changed the title add eip-draft_erc20_approval_expiration Add EIP-5748: Approval Expiration for EIP-20 Tokens Oct 7, 2022
EIPS/eip-5748.md Outdated Show resolved Hide resolved
@Dankyybomb1

This comment was marked as spam.

@@ -0,0 +1,557 @@
// SPDX-License-Identifier: MIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be CC0.


So that existing event statistics are not perturbed by the implementation of this standard, `_approve()` is triggered by the new event `AllowanceExpireTimeUpdated()`.

This standard is absolutely compatible with the traditional [EIP-20](./eip-20.md) standard. `DEFAULT_EXPIRATION` is added so that the original `approve(address spender, uint256 amount)` method header can remain for backwards compatibility and ease of programming. `DEFAULT_EXPIRATION` can be set to a constant or variable according to the developer's needs.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we know this is 'absolutely compatible' ?


## Abstract

This standard adds an expiration time to the `_approve()` function of token contracts to automatically recall approvals within a certain time period. This functionality as a standard will prevent vulnerabilty exploits due to outstanding token approvals.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this _approve coming from?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect _approve comes from a well known EIP-721 implementation? The standard itself doesn't define it.

@github-actions
Copy link

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.

@github-actions github-actions bot added the w-stale Waiting on activity label Jan 25, 2023
@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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.

@github-actions github-actions bot closed this Mar 8, 2023
@amirkhan7javi
Copy link

Should be reviewed. There is a problem between the account and management account is being reviewed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-new Creates a brand new proposal s-draft This EIP is a Draft t-interface w-stale Waiting on activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.