Update EIP-2612: Move to Last Call #5506

frangio · 2022-08-19T14:26:06Z

This EIP is widely deployed and should be finalized. There are ideas for improvement but all would break compatibility with existing implementations so I believe they should be done as a new EIP.

The changes in this PR are editorial only.

eth-bot · 2022-08-19T14:27:02Z

All tests passed; auto-merging...

(pass) eip-2612.md

classification
`updateEIP`

passed!

frangio · 2022-08-19T16:07:37Z

CodeSpell is a great idea but it's highlighting errors in unrelated files. Edit: Ok they show up in the Files tab but they are not affecting the mergeability of the PR.

The validator is complaining about references to non-Final EIPs. I'd like to challenge this rule. Referring to prior art should be encouraged even if that prior art is a non-Final EIP. As far as I can tell the only problem would be to extend or somehow depend on a non-Final EIP. This also touches on the ambiguity of the requires: field which I've been told should be intepreted as "references:" rather than as dependencies. Dependencies should only be allowed to EIPs in later stages, but references should not care.

Pandapip1

There are a lot of external links that need to be removed that I can't comment on. Try to either remove the snippet/sentence/paragraph they're in (if they aren't necessary) or keep the text but remove the link.

EIPS/eip-2612.md

Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>

frangio · 2022-08-25T22:08:21Z

The remaining external links are for test cases and reference implementation. Are those ok?

The discussions link is the GitHub issue, back in June it was said that it's not required to move that to Ethereum Magicians (#2613 (comment)).

Pandapip1 · 2022-08-26T19:42:02Z

The remaining external links are for test cases and reference implementation. Are those ok?

No, those are not okay. Please move them to the assets folder. Sorry for the inconvenience!

EDIT: For your convenience, I put them in the assets folder myself. I am also eager to get this to final.

The discussions link is the GitHub issue, back in June it was said that it's not required to move that to Ethereum Magicians (#2613 (comment)).

That is correct. This will (eventually) require a manual merge.

frangio · 2022-09-14T22:15:37Z

Removed reference implementation and tests.

GitHub doesn't allow me to request reviews from multiple people.

Pandapip1 · 2022-09-15T12:55:50Z

CC @MrChico

frangio · 2022-09-21T22:29:04Z

I've postponed the deadline and reached out to @MrChico for approval. @Pandapip1 Please approve again when you can.

Pandapip1 · 2022-09-22T13:35:11Z

EIPS/eip-2612.md

-title: permit – 712-signed approvals
-description: ERC-20 approvals via secp256k1 signatures
+title: "EIP-20 Permit Extension: Signed Approvals"
+description: EIP-20 approvals via EIP-712 secp256k1 signatures
 author: Martin Lundfall (@Mrchico)
 discussions-to: https://github.com/ethereum/EIPs/issues/2613


Would you mind creating a new thread on Ethereum magicians?

I explicitly asked about this at the beginning of this process and I was told it was okay to keep this link. Please do not put unnecessary hurdles on this PR that has been open for over a month already.

This isn't a hurdle. This was truly a suggestion that would speed up the process later on (hence why I used a comment instead of a request for changes) but one that is optional.

Ok, sorry that I misunderstood.

xinbenlv · 2022-10-04T20:11:07Z

I have some editorial feedback for author @MrChico and FYI @frangio :

Three new functions are added to the EIP-20 ABI:

I'd suggest changing it to

Compliant contracts MUST implement the following 3 new functions in addition to EIP-20:

frangio · 2022-10-04T20:17:49Z

I agree with the suggestion.

To the editors: would it be okay to make the change along with the future PR that will move the EIP to Final? (Just trying to reduce the number of PRs that will need to be approved.)

xinbenlv · 2022-10-04T21:06:27Z

I agree with the suggestion.

To the editors: would it be okay to make the change along with the future PR that will move the EIP to Final? (Just trying to reduce the number of PRs that will need to be approved.)

I am ok

Pandapip1 · 2022-10-19T11:50:27Z

To the editors: would it be okay to make the change along with the future PR that will move the EIP to Final? (Just trying to reduce the number of PRs that will need to be approved.)

If you don't mind, please make it in two separate PRs. I personally think it's okay, but some editors might disagree and say it's a significant enough change to push back the last call deadline.

* Move EIP-2612 to Last Call * replace ERC- with EIP- * Update EIPS/eip-2612.md Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com> * remove external links * ERC -> EIP * remove external links * delay deadline * Create UniswapV2ERC20.spec.ts * Create UniswapV2ERC20.sol * Use files in assets folder * add missing assets and license * edit title and description * Fix YAML Error * fix asset links * push last-call-deadline * remove reference implementation and test cases * postpone deadline Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>

Move EIP-2612 to Last Call

4de66fa

frangio requested a review from eth-bot as a code owner August 19, 2022 14:26

github-actions bot added classification: EIP status change labels Aug 19, 2022

frangio mentioned this pull request Aug 19, 2022

EIP-2612: 712-signed token approvals #2613

Closed

github-actions bot added the waiting: walidator to pass label Aug 19, 2022

replace ERC- with EIP-

7e583fe

Pandapip1 suggested changes Aug 19, 2022

View reviewed changes

EIPS/eip-2612.md Outdated Show resolved Hide resolved

EIPS/eip-2612.md Outdated Show resolved Hide resolved

EIPS/eip-2612.md Outdated Show resolved Hide resolved

EIPS/eip-2612.md Outdated Show resolved Hide resolved

poojaranjan mentioned this pull request Aug 23, 2022

EIP editor apprenticeship meeting 24 ethcatherders/EIPIP#173

Closed

frangio and others added 5 commits August 25, 2022 18:56

Update EIPS/eip-2612.md

692ccab

Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>

remove external links

bb37d5e

ERC -> EIP

23eea16

remove external links

ea1c4f7

delay deadline

574441f

frangio requested review from Pandapip1 and removed request for Pandapip1 August 25, 2022 22:06

Pandapip1 removed classification: EIP status change labels Aug 26, 2022

Pandapip1 added 2 commits August 26, 2022 15:44

Create UniswapV2ERC20.spec.ts

d6f329e

Create UniswapV2ERC20.sol

7df1f3f

github-actions bot added c-status Changes a proposal's status t-erc" labels Aug 26, 2022

Use files in assets folder

50c5572

Ellajoke previously approved these changes Aug 26, 2022

View reviewed changes

frangio dismissed stale reviews from eth-bot and MrChico via 9780d44 September 14, 2022 22:13

Merge branch 'master' into 2612-last-call

56cf891

frangio requested review from MrChico, Pandapip1 and SamWilsn and removed request for MrChico, Pandapip1 and SamWilsn September 14, 2022 22:13

Pandapip1 previously approved these changes Sep 15, 2022

View reviewed changes

postpone deadline

45a6451

frangio dismissed Pandapip1’s stale review via 45a6451 September 21, 2022 22:27

MrChico approved these changes Sep 21, 2022

View reviewed changes

eth-bot enabled auto-merge (squash) September 21, 2022 22:35

eth-bot approved these changes Sep 21, 2022

View reviewed changes

Pandapip1 reviewed Sep 22, 2022

View reviewed changes

Pandapip1 added this to the Manual Merge Queue milestone Sep 22, 2022

SamWilsn disabled auto-merge September 22, 2022 19:28

SamWilsn merged commit 5bcd0c3 into ethereum:master Sep 22, 2022

frangio deleted the 2612-last-call branch September 22, 2022 19:37

frangio mentioned this pull request Oct 12, 2022

Update EIP-2612: Move to Final #5782

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update EIP-2612: Move to Last Call #5506

Update EIP-2612: Move to Last Call #5506

frangio commented Aug 19, 2022 •

edited

Loading

eth-bot commented Aug 19, 2022 •

edited

Loading

frangio commented Aug 19, 2022 •

edited

Loading

Pandapip1 left a comment

frangio commented Aug 25, 2022 •

edited

Loading

Pandapip1 commented Aug 26, 2022 •

edited

Loading

frangio commented Sep 14, 2022

Pandapip1 commented Sep 15, 2022

frangio commented Sep 21, 2022

Pandapip1 Sep 22, 2022

frangio Sep 22, 2022

Pandapip1 Sep 22, 2022 •

edited

Loading

frangio Sep 22, 2022

xinbenlv commented Oct 4, 2022 •

edited

Loading

frangio commented Oct 4, 2022

xinbenlv commented Oct 4, 2022

Pandapip1 commented Oct 19, 2022

Update EIP-2612: Move to Last Call #5506

Update EIP-2612: Move to Last Call #5506

Conversation

frangio commented Aug 19, 2022 • edited Loading

eth-bot commented Aug 19, 2022 • edited Loading

All tests passed; auto-merging...

(pass) eip-2612.md

frangio commented Aug 19, 2022 • edited Loading

Pandapip1 left a comment

Choose a reason for hiding this comment

frangio commented Aug 25, 2022 • edited Loading

Pandapip1 commented Aug 26, 2022 • edited Loading

frangio commented Sep 14, 2022

Pandapip1 commented Sep 15, 2022

frangio commented Sep 21, 2022

Pandapip1 Sep 22, 2022

Choose a reason for hiding this comment

frangio Sep 22, 2022

Choose a reason for hiding this comment

Pandapip1 Sep 22, 2022 • edited Loading

Choose a reason for hiding this comment

frangio Sep 22, 2022

Choose a reason for hiding this comment

xinbenlv commented Oct 4, 2022 • edited Loading

frangio commented Oct 4, 2022

xinbenlv commented Oct 4, 2022

Pandapip1 commented Oct 19, 2022

frangio commented Aug 19, 2022 •

edited

Loading

eth-bot commented Aug 19, 2022 •

edited

Loading

frangio commented Aug 19, 2022 •

edited

Loading

frangio commented Aug 25, 2022 •

edited

Loading

Pandapip1 commented Aug 26, 2022 •

edited

Loading

Pandapip1 Sep 22, 2022 •

edited

Loading

xinbenlv commented Oct 4, 2022 •

edited

Loading