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 ERC: Temporary Approval Extension for ERC-20 #358

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions ERCS/erc-7674.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
eip: 7674
title: Temporary Approval Extension for ERC-20
description: An interface for ephemeral ERC-20 approvals
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
description: An interface for ephemeral ERC-20 approvals
description: An interface for ERC-20 approvals lasting a single transaction

author: Xenia Shape (@byshape), Mikhail Melnik (@ZumZoom), Hadrien Croubois (@Amxx)
discussions-to: https://ethereum-magicians.org/t/add-erc-7674-transient-approval-extension-for-erc-20/19521
status: Draft
type: Standards Track
category: ERC
created: 2024-04-02
requires: 20, 1153
---

## Abstract

This specification defines the minimum interface required to temporarily approve [ERC-20](./eip-20.md) tokens for spending within the same transaction.
ZumZoom marked this conversation as resolved.
Show resolved Hide resolved

## Motivation

User are often require to set token approval that will only be used once. It is common to leave unexpected approvals after these interactions. [EIP-1153](./eip-1153.md) introduces a cheap way to handle temporarily allowances.
Copy link
Collaborator

Choose a reason for hiding this comment

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

will only be used once

This is a bit misleading. This approval can be used multiple times, as long as it's in a single transaction.


## Specification

The key words "MUST", "SHOULD", "MAY" in this document are to be interpreted as described in RFC 2119 and RFC 8174.

Compliant contracts MUST implement 1 new function in addition to [ERC-20](./eip-20.md):
ZumZoom marked this conversation as resolved.
Show resolved Hide resolved
```solidity
function temporaryApprove(address spender, uint256 value) public returns (bool success)
```
Call to `temporaryApprove(spender, value)` allows `spender` to withdraw within the same transaction from `msg.sender` multiple times, up to the `value` amount. This temporary allowance is to be considered in addition to the normal (persistent) [ERC-20](./eip-20.md) allowance, the total value that spender is able to spend during the transaction is thus capped by the sum of the temporary and the normal (persistent) allowances.
Copy link
Contributor

Choose a reason for hiding this comment

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

This temporary allowance is to be considered in addition to the normal (persistent) ERC-20 allowance

This forces an SLOAD of the current allowance, resulting in higher costs than one would want from this feature. I think it should be a temporary allowance override instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree.

If the temporary allowance takes priority (this should be mentionned somewhere if it isn't already), and if the temporary allowance is sufficient to cover the need of some function that consumes allowance (transferFrom), then the allowance can be consumed from the temporary part without having to go consume anything in the persistent part.

You only need to lean (and update) the persistent part if the temporary part is not sufficient.

An analogy would be:

  • you want to cook rice,
  • you have 2kg of rice in your kitchen (fast access - eq to temporary)
  • you have 50kg of rice stored in you basement (slow access - eq to persistent)

it is true that:

  • you have 52kg available
  • to measure how much you have available, you have to check both the kitchen and the basement
  • if you need to cook diner, and that diner requires 500g of rice, the kitchen is sufficient, and you don't need to go to the basement. Rice in the basement is available if you need, but you don't need to access it unless there isn't enough in the kitchen for what you are planning to cook.

Copy link
Contributor

@Amxx Amxx Jun 10, 2024

Choose a reason for hiding this comment

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

What do you think of

Suggested change
Call to `temporaryApprove(spender, value)` allows `spender` to withdraw within the same transaction from `msg.sender` multiple times, up to the `value` amount. This temporary allowance is to be considered in addition to the normal (persistent) [ERC-20](./eip-20.md) allowance, the total value that spender is able to spend during the transaction is thus capped by the sum of the temporary and the normal (persistent) allowances.
Call to `temporaryApprove(spender, value)` allows `spender` to withdraw within the same transaction from `msg.sender` multiple times, up to the `value` amount. This temporary allowance is to be considered in addition to the normal (persistent) [ERC-20](./eip-20.md) allowance, the total value that spender is able to spend during the transaction is thus capped by the sum of the temporary and the normal (persistent) allowances. While it SHOULD be possible for a `transferFrom` operation to consume both types of allowance, the consumption of the temporary allowance SHOULD take priority over the consumption of the persistent allowance. Therefore, if the temporary allowance is sufficient for executing a `transferFrom` operation, the persistent allowance SHOULD not be loaded/updated from the storage. Consumption of persistent allowance, which implies storage accesses, SHOULD be performed only if the temporary allowance is not sufficient for the operation being executed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, my confusion was because of the effect on the allowance() function. If the allowances have to be added, this forces allowance to read from both. However, in the context of transferFrom it should not be necessary to do this, as you've described.

I am not sure if the effect on allowance is something that we should care about.

ZumZoom marked this conversation as resolved.
Show resolved Hide resolved

The storage for the temporary allowances MUST be different to that of the regular allowance. Compliant contracts MAY use the transient storage [EIP-1153](./eip-1153.md) to keep the temporary allowance. For each `owner` and `spender`, the slot MUST be uniquely selected to avoid slot collision. Each slot index SHOULD be derived from the base slot index for temporary allowances, `owner` and `spender` addresses. Slot MAY be derived as `keccak256(spender . keccak256(owner . p))` where `.` is concatenation and `p` is `keccak256` from the string uniquely defining temporary allowances in the namespace of the implementing contract.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is all still implementation detail. You should only specify the externally visible behaviour in the Specification section. You can put recommendations in the Reference Implementation section. How about:

Suggested change
The storage for the temporary allowances MUST be different to that of the regular allowance. Compliant contracts MAY use the transient storage [EIP-1153](./eip-1153.md) to keep the temporary allowance. For each `owner` and `spender`, the slot MUST be uniquely selected to avoid slot collision. Each slot index SHOULD be derived from the base slot index for temporary allowances, `owner` and `spender` addresses. Slot MAY be derived as `keccak256(spender . keccak256(owner . p))` where `.` is concatenation and `p` is `keccak256` from the string uniquely defining temporary allowances in the namespace of the implementing contract.
Each temporary allowance MUST persist until the end of the transaction that created it (unless overwritten by another call to `temporaryApprove`.) Each temporary allowance MUST be cleared at the end of the transaction that created it. See [Using Transient Storage](#using-transient-storage) for an example.

Then moving this content to the Reference Implementation section under a "Using Transient Storage" heading.

Copy link
Contributor

@Amxx Amxx Jun 11, 2024

Choose a reason for hiding this comment

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

(unless overwritten by another call to temporaryApprove or consumed by a call to transferFrom).

ZumZoom marked this conversation as resolved.
Show resolved Hide resolved

Compliant contracts MUST add a temporary allowance check to the `transferFrom` function. The permanent allowance SHOULD only be spent after the temporary allowance has been exhausted.

Compliant contracts MUST add a temporary allowance to the permanent one when returning the allowed amount to spend in the `allowance` function. In case the sum of the temporary and permanent allowance overflow, `type(uint256).max` MUST be returned.

No event is required when setting a temporary allowance, but compliant contracts MAY emit a specific event.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should specify the event, or clarify what "specific" means here. The easiest would be to define the event, but leave it optional.


## Rationale

It was decided to make minimal interface extension to allow easier integration of a compliant contract into the existing infrastructure. This affects the backward compatibility of the `allowance` function. However, the required changes to the `transferFrom` function implementation satisfy the requirement to explicitly authorize the spender to transfer tokens.

## Backwards Compatibility

All functionality of the [ERC-20](./eip-20.md) standard is backward compatible except for the `allowance` function.
ZumZoom marked this conversation as resolved.
Show resolved Hide resolved

## Reference Implementation

TBD <!-- TODO -->

## Security Considerations

The method of deriving slot identifiers to store temporary allowances must avoid collision with other slots in the same space (e.g. transient storage).

## Copyright

Copyright and related rights waived via [CC0](../LICENSE.md).
Loading