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

EIP-2566: Human-Readable Parameters for Contract Function Execution #2566

Conversation

jstoxrocky
Copy link
Contributor

@jstoxrocky jstoxrocky commented Mar 23, 2020


eip: 2566
title: Human-Readable Parameters for Contract Function Execution
author: Joseph Stockermans (@jstoxrocky)
discussions-to: https://ethereum-magicians.org/t/human-readable-parameters-for-contract-function-execution/4154, #2567
status: Draft
type: Standards Track
category: Interface
created: 2020-03-23

@jstoxrocky jstoxrocky changed the title Add eip-draft_send_transaction_to_contract EIP 2566: Human-Readable Parameters for Contract Function Execution Mar 23, 2020
@jstoxrocky jstoxrocky force-pushed the jms/eip-draft_send_transaction_to_contract branch from c65a4e9 to 36fc352 Compare March 23, 2020 18:43
@jstoxrocky jstoxrocky changed the title EIP 2566: Human-Readable Parameters for Contract Function Execution EIP-2566: Human-Readable Parameters for Contract Function Execution Mar 23, 2020
@jstoxrocky jstoxrocky force-pushed the jms/eip-draft_send_transaction_to_contract branch from 36fc352 to 50eee9d Compare March 23, 2020 18:54
@jstoxrocky jstoxrocky force-pushed the jms/eip-draft_send_transaction_to_contract branch 3 times, most recently from ebe7b58 to 9f1b3c4 Compare March 23, 2020 19:07
EIPS/eip-2566.md Outdated
discussions-to: https://ethereum-magicians.org/t/human-readable-parameters-for-contract-function-execution/4154, https://github.com/ethereum/EIPs/issues/2567
status: Draft
type: Standards Track
category (*only required for Standard Track): Interface
Copy link
Member

@lightclient lightclient Jun 26, 2020

Choose a reason for hiding this comment

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

I don't believe that the (*only required for Standard Track) should be included in the PR.

EIPS/eip-2566.md Outdated
---

## Simple Summary
This EIP proposes a new Ethereum RPC method `eth_sendTransactionToContract` that accepts a human readable version of `eth_sendTransaction`'s `data` field. This change will allow ProviderWallets (hybrid Ethereum provider / wallet software) like Metamask and Geth to display confirmation screens with human readable details of a function call to a contract.
Copy link
Member

Choose a reason for hiding this comment

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

s/human readable/human-readable?

EIPS/eip-2566.md Outdated
### Existing Solutions
Much discussion has been made in the past few years on the topic of human readable Ethereum transaction data. Aragon's [Radspec](https://github.com/aragon/radspec) addresses this issue by requiring contract developers to amend their contract functions with human readable comments. ProviderWallets can then use Aragon's Radspec software to parse these comments from the contract code and display them to the end user - substituting in argument values where necessary. Unfortunately, this approach cannot work with contracts that do not have Radspec comments (and may require integration with IPFS(?)).

[EIP 1138](https://github.com/ethereum/EIPs/issues/1138) also addresses this issue directly but contains serious security issues - allowing untrusted dapps to generate the human readable message displayed to users. In a similar train of thought, Geth's #2940 PR](https://github.com/ethereum/go-ethereum/pull/2940) and [EIPs 191](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-191.md), [712](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md) all highlight the Ethereum community's desire for ProviderWallets to better inform users about what data they are actually acting upon.
Copy link
Member

Choose a reason for hiding this comment

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

The link to the Geth PR is malformed.

EIPS/eip-2566.md Outdated

[EIP 1138](https://github.com/ethereum/EIPs/issues/1138) also addresses this issue directly but contains serious security issues - allowing untrusted dapps to generate the human readable message displayed to users. In a similar train of thought, Geth's #2940 PR](https://github.com/ethereum/go-ethereum/pull/2940) and [EIPs 191](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-191.md), [712](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md) all highlight the Ethereum community's desire for ProviderWallets to better inform users about what data they are actually acting upon.

Finally, The ProviderWallet Metamask already includes some built-in magic for interactions with ERC20 contracts that allows confirmation prompts to display the intended *token* recipient and *token* value. Although this is accomplished in an ad hoc fashion for ERC20-like contracts only, the motivation is the same: users deserve better information about the execution of contract functions they are relying on ProviderWallets to perform.
Copy link
Member

@lightclient lightclient Jun 26, 2020

Choose a reason for hiding this comment

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

s/The/the

EIPS/eip-2566.md Outdated
}

## Rationale
With backwards compatibility in mind, this EIP proposes augmenting the set of Ethereum RPC methods with an additional method instead of mutating the existing paralleled method. Precedent for adding a new RPC method comes from [EIP 712](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md) in which adding the method `eth_signTypedData` is proposed for confirmation prompt security. As an alternate approach, the `eth_sendTransaction` method could be changed to accept its `data` argument in a human readable format, but this would break all existing code attempting to execute a contract function. Breaking this backwards compatibility would be a far bigger issue than the readability of an `data` argument so this EIP does not recommended this approach.
Copy link
Member

Choose a reason for hiding this comment

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

s/paralleled//

EIPS/eip-2566.md Outdated
* `gas`: `QUANTITY` - (optional, default: 90000) Integer of the gas provided for the transaction execution. It will return unused gas.
* `gasPrice`: `QUANTITY` - (optional, default: To-Be-Determined) Integer of the gasPrice used for each paid gas
* `value`: `QUANTITY` - (optional) Integer of the value sent with this transaction
* `function`: `Object`
Copy link
Member

Choose a reason for hiding this comment

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

Is there any functional reason for making function an object with two children? Why not maintain the data field and add a new abi field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that make sense, guess I initially thought it was cleaner this way but your suggestion is simpler. Will make the change.

EIPS/eip-2566.md Outdated
This EIP does not break backwards compatibility and addresses that point above.

## Implementation
The `data` field from `eth_sendTransaction` can be reconstructed in its non-human readable format from the `abi` and `args` fields in `eth_sendTransactionToContractFunction`. This allows implementations to broadcast a transaction with data in the correct format for the Ethereum virtual machine. The function signature can be built and hashed from the function ABI, and the arguments can be ABI-encoded according to the data types specified in the function ABI.
Copy link
Member

@lightclient lightclient Jun 26, 2020

Choose a reason for hiding this comment

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

As stated above, what is the benefit of having an new way of encoding the args? Given the abi, eth_sendTransactionToContractFunction should be able to parse args just fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup you're correct - will make the change.

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Hi @jstoxrocky, I left a few comments that I think will expedite the process of getting your draft merged. One minor inconsistency I saw through out is how human-readable is written. In the title its hyphenated, but throughout the text it appears not hyphenated. It's probably best to choose one way or the other and stick with it.

@github-actions
Copy link

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Oct 27, 2020
@jstoxrocky
Copy link
Contributor Author

jstoxrocky commented Nov 2, 2020

@lightclient I appreciate the feedback. @axic I have addressed all required feedback and am ready for a re-review.

@github-actions github-actions bot removed the stale label Nov 2, 2020
@jstoxrocky jstoxrocky force-pushed the jms/eip-draft_send_transaction_to_contract branch from cfd6c7d to 62fab2e Compare November 3, 2020 15:33
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

The discussions-to change is required prior to merge as draft, but I do recommend reviewing the other feedback. Also, consider championing #719 which provides much stronger security guarantees and doesn't introduce new scam attack vectors as this proposal currently does.

EIPS/eip-2566.md Outdated
eip: 2566
title: Human Readable Parameters for Contract Function Execution
author: Joseph Stockermans (@jstoxrocky)
discussions-to: https://ethereum-magicians.org/t/human-readable-parameters-for-contract-function-execution/4154, https://github.com/ethereum/EIPs/issues/2567
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pick a single location for discussion to occur. We generally prefer Ethereum Magicians, so I recommend that one but either is valid.

EIPS/eip-2566.md Outdated
---

## Simple Summary
This EIP proposes a new Ethereum RPC method `eth_sendTransactionToContract` that accepts a human readable version of `eth_sendTransaction`'s `data` field. This change will allow ProviderWallets (hybrid Ethereum provider / wallet software) like Metamask and Geth to display confirmation screens with human readable details of a function call to a contract.
Copy link
Contributor

Choose a reason for hiding this comment

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

Simple summary should be of similar length/content to an email subject line or a forum title or a GitHub PR title. Currently, this is too long for a Simple Summary and should be shortened up a lot.

Also, the latter portion of this summary is more appropriate for motivation than summary.

Suggested change
This EIP proposes a new Ethereum RPC method `eth_sendTransactionToContract` that accepts a human readable version of `eth_sendTransaction`'s `data` field. This change will allow ProviderWallets (hybrid Ethereum provider / wallet software) like Metamask and Geth to display confirmation screens with human readable details of a function call to a contract.
This EIP proposes a new Ethereum RPC method `eth_sendTransactionToContract` that accepts a human readable version of `eth_sendTransaction`'s `data` field.

* `gasPrice`: `QUANTITY` - (optional, default: To-Be-Determined) Integer of the gasPrice used for each paid gas
* `value`: `QUANTITY` - (optional) Integer of the value sent with this transaction
* `data`: `DATA` - The arguments to the function (not ABI encoded)
* `abi`: `DATA` - The function ABI
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be defined more clearly. Do you mean this should be the canonical signature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better comment below.

EIPS/eip-2566.md Outdated
* `gas`: `QUANTITY` - (optional, default: 90000) Integer of the gas provided for the transaction execution. It will return unused gas.
* `gasPrice`: `QUANTITY` - (optional, default: To-Be-Determined) Integer of the gasPrice used for each paid gas
* `value`: `QUANTITY` - (optional) Integer of the value sent with this transaction
* `data`: `DATA` - The arguments to the function (not ABI encoded)
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be defined more clearly. How are the arguments encoded if not ABI encoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good callout - I agree this needs to be more specific. I will update with a slightly different approach. The data field remains the same as eth_sendTransaction and the only difference between the two functions is the abi field. The data field allows the EVM to understand what were trying to do. The abi field gives us the function name and the argument types. We can parse the argument values from the data field and then given that we know the types, implementors can display them in the appropriate formats.

EIPS/eip-2566.md Outdated
The `data` field from `eth_sendTransaction` can be reconstructed in its non-human readable format from the `abi` and `data` fields in `eth_sendTransactionToContractFunction`. This allows implementations to broadcast a transaction with data in the correct format for the Ethereum virtual machine. The function signature can be built and hashed from the function ABI, and the arguments can be ABI-encoded according to the data types specified in the function ABI.

## Security Considerations
Displaying the contract address, function name, and argument name/values can provide additional security to users but it is not a guarantee that a function will execute as the user expects. A poorly implemented contract can still name its function `transfer` and accept `address` and `uint256` arguments - but there is nothing short of contract examination that will let a user know that this contract is indeed a valid ERC20 contract. This EIP does not intend to solve the larger problem around trust in a contract's code, but instead intends to give users better tools to understand exactly what is contained within the data they are broadcasting to the Ethereum network.
Copy link
Contributor

Choose a reason for hiding this comment

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

Function selectors are only 4 bytes which means it is pretty trivial to create collisions for them. Also, parameter names are not validatable at all so a malicious application can trivially rename the variables to whatever they want.

Consider checking out and championing something like #719 if you want to introduce a mechanism for informed consent that doesn't require trusting the application.

Copy link
Contributor Author

@jstoxrocky jstoxrocky Jan 10, 2021

Choose a reason for hiding this comment

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

@MicahZoltu Trying to think up an example for your collision comment. Are you suggesting that a contract could have a function with signature doABadThingXYZ123(address,unit256) where the name doABadThingXYZ123 is constructed in such a way that the first 4 bytes of this signature hash could be the same as something else like doAGoodThing. Then the malicious Dapp can provide doAGoodThing in the ABI and trick the end user to actually execute doABadThingXYZ123?

Copy link
Contributor Author

@jstoxrocky jstoxrocky Jan 10, 2021

Choose a reason for hiding this comment

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

If this scenario is what you are suggesting - I think that I was approaching the problem from this perspective: we can trust contracts since their source code is public and auditable. We cannot trust dapps (I'm using the word here to mean websites that interact with the chain) since most of their source code is hidden from users. For example, picture a sketchy dapp that is interacting with a well known and audited contract such as OMG ERC20. We can trust that the contract doesn't have any functions like doABadThingXYZ123(address,unit256) since it's a public and audited contract. However, we cannot trust that the dapp is actually sending tokens to who the user intends to send them to. They could be swapping around addresses as in my example in the Motivation section. I feel like it's rare that someone examines the data field.

If we are in a situation where we cannot trust the contract...then that is not a situation that this EIP is attempting to provide a solution for. I feel like that is a different situation where the user should not even be interacting with that untrusted contract.

Basically, I'm thinking about a situation with a trusted contract but untrusted dapp.

Copy link
Contributor Author

@jstoxrocky jstoxrocky Jan 10, 2021

Choose a reason for hiding this comment

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

Cheers for linking #719 I have been aware of your work there for a while. I will give it a closer examination.

EIPS/eip-2566.md Outdated
Comment on lines 20 to 51
## Motivation
### ProviderWallet Definition
ProviderWallets like Metamask and Geth are hybrid software that combine an Ethereum API provider with an Ethereum wallet. This allows them to sign transactions on behalf of their users and also broadcast those signed transactions to the Ethereum network. ProviderWallets are used for both convenience and for the protection they give users through human readable confirmation prompts.

### Existing Solutions
Much discussion has been made in the past few years on the topic of human readable Ethereum transaction data. Aragon's [Radspec](https://github.com/aragon/radspec) addresses this issue by requiring contract developers to amend their contract functions with human readable comments. ProviderWallets can then use Aragon's Radspec software to parse these comments from the contract code and display them to the end user - substituting in argument values where necessary. Unfortunately, this approach cannot work with contracts that do not have Radspec comments (and may require integration with IPFS).

[EIP 1138](https://github.com/ethereum/EIPs/issues/1138) also addresses this issue directly but contains serious security issues - allowing untrusted dapps to generate the human readable message displayed to users. In a similar train of thought, [Geth's #2940 PR](https://github.com/ethereum/go-ethereum/pull/2940) and [EIPs 191](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-191.md), [712](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md) all highlight the Ethereum community's desire for ProviderWallets to better inform users about what data they are actually acting upon.

Finally, the ProviderWallet Metamask already includes some built-in magic for interactions with ERC20 contracts that allows confirmation prompts to display the intended *token* recipient and *token* value. Although this is accomplished in an ad hoc fashion for ERC20-like contracts only, the motivation is the same: users deserve better information about the execution of contract functions they are relying on ProviderWallets to perform.

### Background
At one point or another, a dapp will ask a user to interact with a contract. The interaction between dapps and contracts is a large part of the Ethereum ecosystem and is most commonly brokered by a ProviderWallet. When a dapp asks a user to interact with a contract, it will do so by sending the `eth_sendTransaction` method name to the Ethereum API exposed by a ProviderWallet along with the relevant transaction data. The `data` field of the transaction data contains the information necessary for the Ethereum virtual machine to identify and execute the contract's function. This field has a specific formatting that is both non-human readable and non-recoverable to its human readable state.

The accepted format for `eth_sendTransaction`'s `data` field is the hexadecimal encoding of the first four bytes of the keccak256 digest of the function signature. This abbreviated hash is then concatenated with the ABI encoded arguments to the function. Since the keccak256 digest of the function signature cannot be converted back into the function signature, the `data` field is not only non-human readable, its non-recoverable as well. On top of this, additional insight into the concatenated argument values is further obfuscated as information about their data types are held in the function signature preimage.

### Illustrated Example
When a ProviderWallet receives an `eth_sendTransaction` method call, it has the opportunity to display an information-rich confirmation prompt before signing and broadcasting the transaction to the Ethereum network. Unfortunately, as mentioned above, no human readable contract function-execution data is available at this stage. To illustrate, below we see a Metamask confirmation prompt. For context, this transaction executes the function `transferTokens(address,uint256)` at contract address `0xe44127f6fA8A00ee0228730a630fc1F3162C4d52`. The `from` address is aliased as `Account`, the value for the `address` argument passed to the function is `0x6Aa89e52c9a826496A8f311c1a9db62fd477E256`, and the `uint256` value is `100000000000`.

<img src="../assets/eip-2566/tx.png" style="padding-bottom: 20px; padding-top: 20px" width="35%" />

There are a two main issues with what we are seeing. First, the recipient address shown is the destination contract - not the token recipient. While this is technically correct, users might not think about their actions in this way. In their minds, they might think of the recipient as the address to which they are attempting to transfer tokens to. This distinction can become clear with human readable function-execution data. Second, the hexadecimal encoded function-execution data is not very readable. In fact it's very unlikely that a regular user will even look at this information - let alone parse any insight from it.

Let's compare what we see in the images above with what a user would see if a malicious dapp swaps out the address argument for one of their own. We will leave all other information the same but replace `0x6Aa89e52c9a826496A8f311c1a9db62fd477E256` with `0x9e0A8333984D617562a29a935123F17257aD4dEc`.

<img src="../assets/eip-2566/compromised_tx.png" style="padding-bottom: 20px; padding-top: 20px" width="35%" />

You can see how quiet this slight-of-hand is. The hexadecimal encoded function-execution data has changed to include the new address but it is very unlikely that this non-human readable data field would cause any user to suspect that a dapp is stealing from them. Although this attack would likely be obvious to many examining the dapps front end source code, we cannot assume that all users have the ability to identify such an attack. We should strive to give end users much better security when interacting with contracts. Something like the following can be easily imagined with the `eth_sendTransactionToContractFunction` method.

<img src="../assets/eip-2566/human_readable_tx.png" style="padding-bottom: 20px; padding-top: 20px" width="35%" />

The Ethereum community is trending toward tools that better inform users about their actions. ProviderWallets will remain an integral part of the Ethereum community - serving as the backbone for dapp interconnectivity. The Ethereum RPC API specs should be augmented with methods like `eth_sendTransactionToContract` that allow ProviderWallets to better inform users.
Copy link
Contributor

Choose a reason for hiding this comment

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

While there are no strict limits on the length of the Motivation section, this one feels a bit too long. Consider simplify this to a very brief overview of the problem being solved and some high level summaries of use cases. An EIP is a technical specification and not an essay or an argument document, and its focus should be mostly around the specification itself. We want the motivation section so someone can get a quick idea for why they may want to implement this, but the purpose of this section isn't to "sell" anyone on the EIP, just inform them quickly.

@github-actions
Copy link

github-actions bot commented Jan 4, 2021

There has been no activity on this pull request for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jan 4, 2021
@jstoxrocky jstoxrocky force-pushed the jms/eip-draft_send_transaction_to_contract branch from cfa8b71 to 17582bd Compare January 10, 2021 23:41
@jstoxrocky jstoxrocky force-pushed the jms/eip-draft_send_transaction_to_contract branch from 17582bd to 93eae7b Compare January 10, 2021 23:41
@jstoxrocky
Copy link
Contributor Author

@MicahZoltu I appreciate the feedback. I will check out #719. In the meantime, I have addressed all required feedback and am ready for a re-review.

@github-actions github-actions bot removed the stale label Jan 11, 2021
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in getting this merged, was out sick for a couple days and apparently the bus factor for PR reviews is 1. 😬

@MicahZoltu MicahZoltu merged commit b10b8fd into ethereum:master Jan 13, 2021
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
…thereum#2566)

New Ethereum RPC method `eth_sendTransactionToContractFunction` that parallels `eth_sendTransaction` but allows for human-readable contract function execution data to be displayed to users.
@Amit0617
Copy link

Why is this still stagnant? Is this waiting for an implementation pull request in clients?

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

Successfully merging this pull request may close these issues.

5 participants