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: Automatic Transaction Representation #7023

Closed
wants to merge 12 commits into from

Conversation

Pandapip1
Copy link
Member

@Pandapip1 Pandapip1 commented May 12, 2023

No description provided.

@Pandapip1 Pandapip1 requested a review from eth-bot as a code owner May 12, 2023 15:56
@github-actions github-actions bot added c-new Creates a brand new proposal t-erc labels May 12, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented May 12, 2023

File EIPS/eip-7023.md

Requires 1 more reviewers from @axic, @SamWilsn, @xinbenlv

@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels May 12, 2023
ghost
ghost previously approved these changes May 12, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label May 12, 2023
ghost
ghost previously approved these changes May 12, 2023
@abcoathup
Copy link
Contributor

FULL DISCLOSURE: This EIP has an irregular state transition (No EIP -> Living)

Shouldn't this go to draft first?

Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
@Pandapip1 Pandapip1 dismissed stale reviews from ghost and ghost via c8981c4 May 14, 2023 14:07
@Pandapip1 Pandapip1 dismissed a stale review via d240ecd May 16, 2023 14:52
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
@github-actions github-actions bot added s-draft This EIP is a Draft w-ci Waiting on CI to pass and removed w-ci Waiting on CI to pass labels May 16, 2023
ghost
ghost previously approved these changes May 27, 2023
@Pandapip1 Pandapip1 changed the title Add EIP: Automatic Transaction Representation Add EIP: Automatic Transaction Representation V1 May 29, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label May 29, 2023
@eth-bot eth-bot changed the title Add EIP: Automatic Transaction Representation V1 Add EIP: Automatic Transaction Representation v1 May 29, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label May 29, 2023
EIPS/eip-draft_transaction_representation.md Outdated Show resolved Hide resolved
EIPS/eip-draft_transaction_representation.md Outdated Show resolved Hide resolved
EIPS/eip-draft_transaction_representation.md Outdated Show resolved Hide resolved
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@eth-bot eth-bot changed the title Add EIP: Automatic Transaction Representation v1 Add EIP: Automatic Transaction Representation Jul 22, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Jul 22, 2023
@Pandapip1 Pandapip1 requested a review from SamWilsn July 22, 2023 16:08
@github-actions
Copy link

The commit 6a4f26d (as a parent of 90764ac) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Jul 22, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Jul 22, 2023
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

I have two blocking concerns with this EIP.

First, it doesn't seem to have anything to do with ERC-712 as mentioned in the description.

Second, I'm not sure a set of UX guidelines is appropriate for an EIP. It's likely that these recommendations will change over time, leading to a bunch of Final proposals that aren't the latest best practices. I know you try to handle this by introducing versioning, but it still feels...inappropriate.

Comment on lines +35 to +36
- If a request comes from a non-URI source (e.g. a QR code), the wallet MUST indicate the source of the request to the user.
- If a request comes from a URI source, the wallet MUST indicate the source of the request to the user, including the URI scheme, host, and port. If the transaction originates from an unsafe (as determined by the wallet) embedded frame (e.g. an iframe or webview), the wallet MUST indicate this to the user and add a warning that the request may not be trustworthy.
Copy link
Contributor

Choose a reason for hiding this comment

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

These two bullet points feel a little weird. I might suggest:

Suggested change
- If a request comes from a non-URI source (e.g. a QR code), the wallet MUST indicate the source of the request to the user.
- If a request comes from a URI source, the wallet MUST indicate the source of the request to the user, including the URI scheme, host, and port. If the transaction originates from an unsafe (as determined by the wallet) embedded frame (e.g. an iframe or webview), the wallet MUST indicate this to the user and add a warning that the request may not be trustworthy.
- Wallets MUST indicate the source of the request to the user.
- If a request comes from a URI source, this indication MUST indicate the source of the request to the user, including the URI scheme, host, and port. If the transaction originates from an unsafe (as determined by the wallet) embedded frame (e.g. an iframe or webview), this indication MUST indicate this to the user and add a warning that the request may not be trustworthy.

- The verified source code of any referenced contracts with verified source code MUST be made available to the user.
- Users MUST be able to permanently dismiss verified source code warnings by address, chain ID, or address/chain ID pair, and a link to verify the source code of any contract MUST be provided.
- If the wallet can determine if a contract has been audited:
- For each contract referenced in a transaction:
Copy link
Contributor

Choose a reason for hiding this comment

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

This wording is a bit ambiguous. Does this mean in the to field of the transaction, or any contract called while executing? Does this also include contract deployments?

@SamWilsn
Copy link
Contributor

I am closing this pull request because we are in the process of separating EIPs and ERCs into distinct repositories. Unfortunately, as far as we are aware, GitHub does not provide any tools to ease this migration, so every pull request will need to be re-opened manually.

As this is a PR to create / modify an ERC, I will kindly ask you to redirect this to the new repository at ethereum/ERCs. We have prepared a guide to help with the process.

If there is relevant history here, please link to this PR from the new pull request.

On behalf of the EIP Editors, I apologize for this inconvenience.

@SamWilsn SamWilsn closed this Oct 25, 2023
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-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants