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: Migration Transaction #7377

Merged
merged 16 commits into from
Jul 26, 2023

Conversation

lightclient
Copy link
Member

This EIP proposes a new transaction type that allows EOAs to submit a one-time upgrade to a smart contract.

@lightclient lightclient requested a review from eth-bot as a code owner July 22, 2023 13:52
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-core labels Jul 22, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 22, 2023

File EIPS/eip-7377.md

Requires 1 more reviewers from @axic, @g11tech, @gcolvin, @Pandapip1, @SamWilsn

@eth-bot eth-bot changed the title Add migration tx eip Add EIP: Migration Transaction Jul 22, 2023
@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels Jul 22, 2023
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated

### Cheaper storage

Sometimes it's better to lead with a carrot instead of a stick.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Sometimes it's better to lead with a carrot instead of a stick.
Cheaper storage incentivizes users to migrate, while maintaining the storage cost means that there is more friction to the migration process. Additionally, since mass adoption of contract wallets is a positive externality, a storage subsidy makes sense.

Should probably use more formal language here.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's no fun though

Copy link
Member

Choose a reason for hiding this comment

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

Okay. You can have your fun. As long as you follow it up with a formal explanation.

Suggested change
Sometimes it's better to lead with a carrot instead of a stick.
Sometimes it's better to lead with a carrot instead of a stick.
Cheaper storage incentivizes users to migrate, while maintaining the storage cost means that there is more friction to the migration process. Additionally, since mass adoption of contract wallets is a positive externality, a storage subsidy makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

I will be blocking this EIP until a formal explanation for this is added.

EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Show resolved Hide resolved
EIPS/eip-7377.md Show resolved Hide resolved
@YaroslavPysanskyi

This comment was marked as off-topic.

@lightclient

This comment was marked as off-topic.

@lightclient
Copy link
Member Author

thanks for the feedback @Pandapip1, ready for review again

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

## Security Considerations

As with all sufficiently sophisticated account designs, if a user can be convinced to sign an arbitrary message, that message could be MigrationTransaction which is owned by a malicious actor instead of the user. This can generally be avoided if wallets treat these transactions with *extreme* care and create as much friction and verification as possible before completing the signature.
Copy link
Member

Choose a reason for hiding this comment

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

What is important? If you're talking about the "reasonable amount of friction" bit, I would be fine if that part is unchanged:

Suggested change
As with all sufficiently sophisticated account designs, if a user can be convinced to sign an arbitrary message, that message could be MigrationTransaction which is owned by a malicious actor instead of the user. This can generally be avoided if wallets treat these transactions with *extreme* care and create as much friction and verification as possible before completing the signature.
If a user can be convinced to sign an arbitrary message by a malicious actor, that message could now set the account code of the victim. This can generally be avoided if wallets treat these transactions with *extreme* care and create as much friction and verification as possible before completing the signature.

I would still argue "as much friction as possible" is NOT what you mean here - this is an EIP, and so hyperbole should not be used.

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

### Cheaper storage

Sometimes it's better to lead with a carrot instead of a stick.
Copy link
Member

Choose a reason for hiding this comment

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

Okay. You can have your fun. As long as you follow it up with a formal explanation.

Suggested change
Sometimes it's better to lead with a carrot instead of a stick.
Sometimes it's better to lead with a carrot instead of a stick.
Cheaper storage incentivizes users to migrate, while maintaining the storage cost means that there is more friction to the migration process. Additionally, since mass adoption of contract wallets is a positive externality, a storage subsidy makes sense.

@lightclient
Copy link
Member Author

@Pandapip1 the PR meets the criteria for draft so I would like to ask that you approve it.

EIPS/eip-7377.md Outdated

### No `to` address field

This transaction is only good for one time use to migrate an EOA to a smart contract. It is designed to immediately call the deployed contract, which is at the sender's address, after deployment to allow the sender to do any kind of further processing.
Copy link
Member

Choose a reason for hiding this comment

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

The first sentence is grammatically incorrect and the phrasing is wrong. I think that what you mean is that "This transaction is designed to perform a one-time migration from an EOA to a smart contrct." It's not "only good" for migration - it's its explicit goal.

Additionally, the second sentence has misplaced commas and the structure makes it hard to read.

EIPS/eip-7377.md Outdated

### Cheaper storage

Sometimes it's better to lead with a carrot instead of a stick.
Copy link
Member

Choose a reason for hiding this comment

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

I will be blocking this EIP until a formal explanation for this is added.


## Security Considerations

As with all sufficiently sophisticated account designs, if a user can be convinced to sign an arbitrary message, that message could be MigrationTransaction which is owned by a malicious actor instead of the user. This can generally be avoided if wallets treat these transactions with *extreme* care and create as much friction and verification as possible before completing the signature.
Copy link
Member

Choose a reason for hiding this comment

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

Why is that an important part of the statement? Isn't a "sufficiently sophisticated account design" implied by a "sign an arbitrary message"?

@lightclient
Copy link
Member Author

@Pandapip1 the formatting is correct. There is nothing in EIP-1 that says you can block merging a well-formatted draft because an explanation isn't formal enough. I again ask that you merge this to draft. I want to share this with the community, but you are delaying it.

EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
EIPS/eip-7377.md Outdated Show resolved Hide resolved
@lightclient
Copy link
Member Author

@g11tech, wrote a new motivation, PTAL

@Pandapip1
Copy link
Member

Pandapip1 commented Jul 25, 2023

@Pandapip1 the formatting is correct. There is nothing in EIP-1 that says you can block merging a well-formatted draft because an explanation isn't formal enough. I again ask that you merge this to draft. I want to share this with the community, but you are delaying it.

Quoting EIP-1:

EIP Editor Responsibilities

For each new EIP that comes in, an editor does the following:

  • Read the EIP to check if it is ready: sound and complete. The ideas must make technical sense, even if they don’t seem likely to get to final status.
  • The title should accurately describe the content.
  • Check the EIP for language (spelling, grammar, sentence structure, etc.), markup (GitHub flavored Markdown), code style

If the EIP isn’t ready, the editor will send it back to the author for revision, with specific instructions.

The majority of my complaints fall under the third option. In fact, sentence structure is explicitly called out as something that I can block an EIP for.

Additionally, my requiring of adding a formal explanation for your "lead with a carrot, not a stick" falls squarely under the first option, since I am blocking the EIP for not containing a sound and complete explanation for that particular section.

I don't want to delay merging your EIP any longer than need be. Either accept my changes, or fix them yourself. But I will not be merging this EIP until you do. Since you are an editor yourself, I will be holding you to a higher standard of quality here.

@lightclient
Copy link
Member Author

@Pandapip1 I've added more formal language. I don't agree with your other change requests and they are not reasonable grounds to block on.

@lightclient lightclient merged commit 7e5caf1 into ethereum:master Jul 26, 2023
streamnft-tech pushed a commit to streamnft-tech/EIPs that referenced this pull request Oct 27, 2023
* add migration tx eip

* add 2718 as requirement

* add eip number

* placate the supreme ruler, our great overlord, and my friend - eipw

* fix duplicate

* fix hierarchy

* fix hierarchy

* markdown lint

* rename storage field

* Apply suggestions from code review

Co-authored-by: Gavin John <[email protected]>

* more code review suggestions

* Nit

* Nit 2: Electric Boogaloo

* addr instead of ptr

* different motivation

* remove fun line

---------

Co-authored-by: Gavin John <[email protected]>
RaphaelHardFork pushed a commit to RaphaelHardFork/EIPs that referenced this pull request Jan 30, 2024
* add migration tx eip

* add 2718 as requirement

* add eip number

* placate the supreme ruler, our great overlord, and my friend - eipw

* fix duplicate

* fix hierarchy

* fix hierarchy

* markdown lint

* rename storage field

* Apply suggestions from code review

Co-authored-by: Gavin John <[email protected]>

* more code review suggestions

* Nit

* Nit 2: Electric Boogaloo

* addr instead of ptr

* different motivation

* remove fun line

---------

Co-authored-by: Gavin John <[email protected]>
just-a-node pushed a commit to connext/EIPs that referenced this pull request Feb 17, 2024
* add migration tx eip

* add 2718 as requirement

* add eip number

* placate the supreme ruler, our great overlord, and my friend - eipw

* fix duplicate

* fix hierarchy

* fix hierarchy

* markdown lint

* rename storage field

* Apply suggestions from code review

Co-authored-by: Gavin John <[email protected]>

* more code review suggestions

* Nit

* Nit 2: Electric Boogaloo

* addr instead of ptr

* different motivation

* remove fun line

---------

Co-authored-by: Gavin John <[email protected]>
GAEAlimited pushed a commit to GAEAlimited/EIPs that referenced this pull request Jun 19, 2024
* add migration tx eip

* add 2718 as requirement

* add eip number

* placate the supreme ruler, our great overlord, and my friend - eipw

* fix duplicate

* fix hierarchy

* fix hierarchy

* markdown lint

* rename storage field

* Apply suggestions from code review

Co-authored-by: Gavin John <[email protected]>

* more code review suggestions

* Nit

* Nit 2: Electric Boogaloo

* addr instead of ptr

* different motivation

* remove fun line

---------

Co-authored-by: Gavin John <[email protected]>
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 e-consensus Waiting on editor consensus e-review Waiting on editor to review s-draft This EIP is a Draft t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants