ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.9k stars 5.28k forks source link

New "unlock to send a transaction" method name discussion #113

Closed frozeman closed 8 years ago

frozeman commented 8 years ago
ERC
Title: unlock and sign transaction name finding
Status: Draft
Type: Informational
Created: 06.2016

Abstract

The following will help finding the right name for the new function to unlock an account and send a transaction in one go.

The function works the same as eth_sendTransaction, except that it accepts a password as last parameter, whcih will autounlock the account for this one transaction. So the steps are:

-> method is called with tx object and password

  1. unlocks account
  2. signs transaction
  3. sends transaction
  4. locks account again

    Motivation

The idea is to find a name which is intuitive enough so that people won't get confused when using the API. API names should be clear, intuitive and distinct from other function names. Currently we have in geth implemented personal_signAndSendTransaction and web3.js uses the name web3.eth.unlockAndSendTransaction.

This ERC is to dicuss and find a proper name, which everybody feels comfortable with and which reduces confusion.

Possible names, so far

This names falls under the personal_ namespace, we currently have:

Please give your input on those names, or ideas for better ones.

And lets make this a quick one..

hiddentao commented 8 years ago

personal_secureSendTransaction - because it's more secure than eth_sendTransaction in that you don't need to have the account already unlocked. It also encourages use by stating that it's secure. Note that calling it personal_sendSecureTransaction would mean something different, hence this choice.

redsquirrel commented 8 years ago

personal_sendAuthenticatedTransaction

bas-vk commented 8 years ago

I would really like to avoid anything with unlocked. This function doesn't unlock the account like personal_unlockAccount or the unlock command line argument does. It finds the private key associated with tx.from, tries do decrypt it with the given password, signs the transaction and zero's the key, all in the context of a RPC call.

I also don't like personal_sendAuthenticatedTransaction. When I see this name I assume that it accepts an already signed transaction just like eth_sendRawTransaction.

As a disadvantage for personal_signAndSendTransaction:

Is not distinct enough from eth_sendTransaction, as this one is signing also, just not unlocking the account first (it has to be unlocked)

personal_sendAuthenticatedTransaction is not very different either?

Personally I don't think that there is anything wrong with personal_signAndSendTransaction and would opt for that name.

frozeman commented 8 years ago

I personally also want to change eth_sendRawTransaction to eth_sendSignedTransaction, as the former one is not clear. (@SilentCicero)

But thats off topic

jimkberry commented 8 years ago

How about just personal_sendTransaction()? The fact that it's part of the personal namespace implies the account connection.

chfast commented 8 years ago

Can't you extend the eth_sendTransaction with an optional param with password?

tcoulter commented 8 years ago

In favor of the last two proposed: personal prefix is enough, or extend eth_sendTransaction to take another parameter.

danfinlay commented 8 years ago

I'd like to point out this API might encourage Ðapp Devs to accept passwords in the UI, which normalizes the act of typing a secret password into a foreign input.

I think passwords are more safely handled at the client level, and a good client should notify the user and prompt for a password in response to a normal sendTransaction call if the account is locked.

SilentCicero commented 8 years ago

@flyswatter agreed.

chfast commented 8 years ago

What @flyswatter proposed is not feasible. Let's consider geth - Mist architecture. geth keeps password protected accounts, but it cannot show up a popup out of nowhere asking for a password on top of Mist UI.

If passing a password in JSON RPC is the issue, maybe we can use some better authorisation scheme. I only know there exist authorisation schemes where a server challenges a client with a puzzle that can be solved only if the client knows the password. Password is not revealed, the challenge is changed every session. I'm not able to name nor explain how it works, any help here?

Bringing that to our JSON RPC protocol, we could keep eth_sendTransaction request as it is, but in the response a node should give the user a challenge. The user should send another request with the answer.

bas-vk commented 8 years ago

I agree that eth_sendTransaction should remain as is.

The problem is that nodes currently encrypt the private key's with the password and need to have it in order to decrypt the private key that is used to sign the transaction. A challenge response system would not allow for the secure exchange of the password.

In the past I have thought of something like "geth-agents". When the node starts you can give it an agent endpoint. The node will connect to this endpoint and forwards (unsigned) transactions to it. The agent asks the user for the password if necessary, sends the signed transaction back to the node which broadcasts it onto the network. In that way the node doesn't need account management and anyone can write a custom agent suitable for their needs. For example, Mist can also act as an agent if it starts geth in the background. But this will break the current eth_sendTransaction since it can take some time before the transaction is signed.

jimkberry commented 8 years ago

This is an interesting discussion for me - I have always thought that eth_sendTransaction() and the idea of node-managed accounts were really just development tools and that production software would always be expected to have a facility for signing transactions and sending them using eth_sendRawTransaction(). Apparently I have been wrong.

danfinlay commented 8 years ago

@bas-vk it sounds like you're describing this with your agents proposal: https://github.com/ethereum/EIPs/pull/107

@chfast what I propose is entirely feasible when the keys are stored in a separate process from the node, like the link I just posted suggests, and looking at geth for security guidance is an awful idea. Keys should not be stored in the node, there should be no unlocked mode where the node signs everything, and if you don't see why, look at Mist's recent $80k theft.

I entirely agree with @jimkberry that this inter-tangling was a development convenience, but it should not be treated as a fact of the platform, and certainly not entrenched with new standards.

danfinlay commented 8 years ago

Oh sorry @jimkberry I partly misrepresented:

It's an interesting point there.

Yes, I have been working on a client that allows secure prompted signing on sendTransaction. SendRawTransaction would be hard to work with alone, because it takes the authorization object entirely out of the web3 object, and in turn out of the responsibility of the Ethereum browser, and now other measures would have to be taken for users to sign txs (copy this into your wallet to sign??)

While I don't think nodes and wallets should share a process, I do think there's value in a web3 method for requesting a user signature, as long as it's coupled with good UX, and requires confirmation.

frozeman commented 8 years ago
  1. personal_sendTransaction sounds like the best idea
  2. This was never meant to be used widely by developers. I only put this here to be discussed, so that we make sure we include something were we at least agree on a good name. But its more a function used by local dapps.
  3. I fully expect key signing to be outside of geth or any node, be it in the dapp itself, javascript, or a key management agent

So i would rather focus here on the name, and i prefer personal_sendTransaction , as through the personal_ namespace, makes sense that it has a added password.

The whole, "how and where to sign" discussion, and as @bas-vk says we were thinking already about that since a while, needs to take place in its own ERC.

for now its still in geth, but we should warn people when using this personal_sendTransaction endpoint that it is insecure over HTTP, or not even allow it there by default. I could also include a console.warn to make sure developers read and understand that.

In the future only eth_sendRawTransaction should be used (which i would rename to eth_sendSignedTransaction)

danfinlay commented 8 years ago

Sorry @frozeman for hijacking, it hadn't occurred to me that naming might be an EIP before the actual feature was up for discussion.

I'll look forward to the actual discussion, I do think as a community we should make sure these interfaces have common understandings around them, which it sounds like they don't.

frozeman commented 8 years ago

personal_sendTransaction it is, thanks for your help.