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: Interoperable Delegated Accounts #663

Conversation

PowerStream3604
Copy link
Contributor

This proposal outlines the interfaces and behavior for delegated accounts, for better interoperability and storage management.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Oct 1, 2024

✅ All reviewers have approved.

@eip-review-bot eip-review-bot changed the title Add ERC: Interoperable Delegated Account Add ERC: Interoperable Delegated Accounts Oct 1, 2024
@github-actions github-actions bot added the w-ci label Oct 1, 2024
@PowerStream3604 PowerStream3604 force-pushed the add-erc-interoperable-delegated-account branch from 372e22a to c66616e Compare October 1, 2024 18:33
@PowerStream3604 PowerStream3604 force-pushed the add-erc-interoperable-delegated-account branch from c66616e to d9dd8db Compare October 1, 2024 18:41
@github-actions github-actions bot removed the w-ci label Oct 2, 2024
@github-actions github-actions bot added the w-ci label Oct 2, 2024
@github-actions github-actions bot added w-ci and removed w-ci labels Oct 2, 2024
@github-actions github-actions bot removed the w-ci label Oct 7, 2024
@github-actions github-actions bot added the w-ci label Oct 7, 2024
ERCS/erc-7779.md Outdated Show resolved Hide resolved
ERCS/erc-7779.md Outdated Show resolved Hide resolved
ERCS/erc-7779.md Outdated Show resolved Hide resolved
ERCS/erc-7779.md Outdated Show resolved Hide resolved
ERCS/erc-7779.md Outdated Show resolved Hide resolved
ERCS/erc-7779.md Outdated Show resolved Hide resolved
assets/erc-7779/diagram.png Outdated Show resolved Hide resolved
@eip-review-bot eip-review-bot changed the title Add ERC-7779: Interoperable Delegated Accounts Add ERC: Interoperable Delegated Accounts Oct 29, 2024
@PowerStream3604
Copy link
Contributor Author

@SamWilsn could you review this PR again?
I resolved all comments from your previous review.

Thank you

Copy link

github-actions bot commented Nov 4, 2024

The commit a907688 (as a parent of c79a9e9) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci label Nov 4, 2024
@github-actions github-actions bot removed the w-ci label Nov 4, 2024
ERCS/erc-7779.md Outdated Show resolved Hide resolved
ERCS/erc-7779.md Outdated Show resolved Hide resolved
@eip-review-bot eip-review-bot enabled auto-merge (squash) December 9, 2024 17:20
Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eip-review-bot eip-review-bot merged commit 31462fd into ethereum:master Dec 9, 2024
10 of 13 checks passed

However, there is a need to help facilitate storage management for redelegation, as invalid management of storage may incur storage collisions that can lead to unexpected behavior of accounts (e.g., account getting locked, security vulnerabilities, etc)

The interface `InteroperableDelegatedAccount` suggests the interfaces for delegated EOAs to be interoperable and facilitate a better environment for redelegation.
Copy link
Contributor

Choose a reason for hiding this comment

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

The interface InteroperableDelegatedAccount suggests the interfaces

I would change the second occurrence of 'interface' with 'methods' or 'functions'


`onRedelegation()` was designed to be optional to lower the barrier of being compliant with this standard. Also, there could be accounts that does not functionally require a hook-logic to be in place before redelegation, or accounts that does not suite with the design principle of the `onRedelegation()` e.g., excessive use of mapping or data types that's hard to uninitialize.

It is worth noting that, `onRedelegation()` does not obligate the account to completely whipe out it's storage. It's more of a best effort function to leave the cleanest stage for the future use of the EOA in a new wallet. Or to execute a function to prepare for redelegation.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in ' ... whipe out ...'


Also, the account should make sure the initializer cannot be triggered by an arbitrary entity after `onRedelegation()` is called.

4. `onRedelegation()` should not reset the replay protection considering that it could incur a vulnerability(e.g., signature reply attack).
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in 'signature reply attack'

@filmakarov
Copy link
Contributor

Have you guys considered accepting bytes calldata data param in onRedelegation()?
It can be useful, for example for ERC-7579 accounts, to call onUnistall methods on modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants