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

Abstraction of transaction origin and signature.md #208

Merged
merged 10 commits into from
Mar 27, 2018
Merged

Conversation

vbuterin
Copy link
Contributor

Implements a set of changes that serve the combined purpose of "abstracting out" signature verification and nonce checking, allowing users to create "account contracts" that perform any desired signature/nonce checks instead of using the mechanism that is currently hard-coded into transaction processing.

Implements a set of changes that serve the combined purpose of "abstracting out" signature verification and nonce checking, allowing users to create "account contracts" that perform any desired signature/nonce checks instead of using the mechanism that is currently hard-coded into transaction processing.
@cdetrio
Copy link
Member

cdetrio commented Feb 25, 2017

Can we make the title more descriptive? How about something like "Abstraction of transaction origin and signature"

@pirapira
Copy link
Member

pirapira commented Mar 1, 2017

I see a change about nonce increment in https://github.com/ethcore/parity/pull/4697 (line: https://github.com/ethcore/parity/blob/027b0446efa48622ce166a561dd274332dac96a0/ethcore/src/executive.rs#L178 ) but not in this PR. I don't have a strong opinion, but need to know which to follow. Now I think, nonce increment or nonce checking should be disabled on NULL_SENDER. Otherwise, transaction signers need to compete for the current nonce for no particular benefits.

@pirapira
Copy link
Member

pirapira commented Mar 1, 2017

@yann300 is also working on this.

If `block.number >= METROPOLIS_FORK_BLKNUM`, then:
1. If the signature of a transaction is `(CHAIN_ID, 0, 0)` (ie. `r = s = 0`, `v = CHAIN_ID`), then treat it as valid and set the sender address to `NULL_SENDER`
2. Set the address of any contract created through a creation transaction to equal `sha3(NULL_SENDER + sha3(init code)) % 2**160`, where `+` represents concatenation, replacing the earlier address formula of `sha3(rlp.encode([sender, nonce]))`
3. Create a new opcode at `0xfb`, `CREATE_P2SH`, which sets the creation address to `sha3(sender + sha3(init code)) % 2**160`. If a contract at that address already exists, fails and returns 0 as if the init code had run out of gas.
Copy link
Member

Choose a reason for hiding this comment

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

Does the last sentence ("If a contract at that address already exists,...") apply to CREATE opcode and creation by external accounts?

@rphmeier
Copy link

rphmeier commented Mar 7, 2017

@pirapira

I think, nonce increment or nonce checking should be disabled on NULL_SENDER.

Yes, absolutely. We'll also have to make it work with the dust protection EIP.
Probably nonce checking should also be disabled, or we should make a distinction between transactions abstracted from NULL_SENDER and transactions actually signed by NULL_SENDER

### Specification

If `block.number >= METROPOLIS_FORK_BLKNUM`, then:
1. If the signature of a transaction is `(CHAIN_ID, 0, 0)` (ie. `r = s = 0`, `v = CHAIN_ID`), then treat it as valid and set the sender address to `NULL_SENDER`
Copy link
Member

Choose a reason for hiding this comment

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

Was v = CHAIN_ID really the intention here?
Currently according to #155 v = CHAIN_ID * 2 + 35 or v = CHAIN_ID * 2 + 36
(implemented only as v = CHAIN_ID * 2 + 35 in cpp-ethereum)

So do we really need additional v rule for zero signature instead of following EIP155 here, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two ways in which v is set in EIP 155. The first is the v value which appears in the actual transaction. This is set according to that formula. The second is the value which is put into the v slot when computing the hash. This is set to equal the CHAIN_ID, just as here.

Copy link
Member

@gumb0 gumb0 Apr 28, 2017

Choose a reason for hiding this comment

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

So v here in this EIP refers not to the actual value in Tx RLP, but to the one extracted from it using EIP155 formula, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aah! I see what you mean, sorry I take back my previous comment. I would still prefer v in the transaction being equal to the chain ID, as that's the easiest thing to do. There is no need to set the v value to 35 + chain_id * 2 here.

@winsvega
Copy link
Contributor

winsvega commented Mar 17, 2017

Some questions about zero-sig transaction:

what should be the standart transaction fields (gas, gasPrice, nonce) encoded in RLP?
does this zero-sig transaction follow the rules as all other transactions (except signature)?
this transaction still charge transaction call gasPrice?
what if 0xfffffff has not enough founds?
what if 0xfffffff has enough founds?
could this transaction be sent to other then the fowarding contract?
could this transaction create new contract?
if this transaction should always has gasPrice set to 0, how would miners know to accept it?

### Specification

If `block.number >= METROPOLIS_FORK_BLKNUM`, then:
1. If the signature of a transaction is `(CHAIN_ID, 0, 0)` (ie. `r = s = 0`, `v = CHAIN_ID`), then treat it as valid and set the sender address to `NULL_SENDER`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not formatted correctly.

Copy link
Member

Choose a reason for hiding this comment

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


If `block.number >= METROPOLIS_FORK_BLKNUM`, then:
1. If the signature of a transaction is `(CHAIN_ID, 0, 0)` (ie. `r = s = 0`, `v = CHAIN_ID`), then treat it as valid and set the sender address to `NULL_SENDER`
2. Set the address of any contract created through a creation transaction to equal `sha3(NULL_SENDER + sha3(init code)) % 2**160`, where `+` represents concatenation, replacing the earlier address formula of `sha3(rlp.encode([sender, nonce]))`
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this only applies to transactions with r = s = 0. Is that correct? So we have:
If such a transaction has a to value of zero (i.e is a creation transaction), set the address of the created contract to sha3(NULL_SENDER + sha3(txdata)) % 2**160, .... If an account at that address already exists (TODO correctly specify existence), the transaction is considered invalid.

Copy link
Member

Choose a reason for hiding this comment

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

  • I understood that this applies to all contract creations.
  • I think we should keep allowing creating contracts on existing accounts (e.g. with positive balance).

@chriseth
Copy link
Contributor

Do we want to lower the gas costs for such transactions (i.e. the transaction itself) since ecrecover does not have to be called to check the signature.

@vbuterin
Copy link
Contributor Author

vbuterin commented Mar 21, 2017

So there are two paths to resolving this:

  1. We implement the EIP as is, so we keep the gasPrice mechanism, keep the nonce checking, just make a (0, 0, 0) sig represent a NULL_SENDER with no other changes to the protocol. For practicality we can add a special-case rule that the nonce of NULL_SENDER is always 0 and never increments.
  2. We add extra rules:
  • The nonce of an EIP 86 transaction MUST be 0
  • The gasprice of an EIP 86 transaction MUST be 0
  • The value of an EIP 86 transaction MUST be 0

EIP 86 transactions can create new contracts if the "to" field is empty.

This looks ugly in either case, as we have useless vestigial fields that hang around like an appendix, but IMO we should make a future HF which modifies TX formats more substantially and makes them more logical, for example including a proper field for the network ID and the transaction version number, and past the HF making old-style transactions no longer valid. IMO we have to do this eventually, as if we do not, then every time we make new tx formats or change tx formats we incur more and more technical debt. If we really feel daring, I can make an EIP for doing this now, so we can just not do EIP 86 at all.

@pirapira
Copy link
Member

pirapira commented Mar 21, 2017

@winsvega miners can, for example, whitelist contracts that pay the current miner (by calling COINBASE). The miners can, for example, include EIP86 transactions only if they call the whitelisted contracts. These are example miner strategies and we do not have to specify this in EIPs.

@winsvega
Copy link
Contributor

winsvega commented Mar 21, 2017

if it has gasprice of 0 and can create contracts would not it mean that such transaction could spam contracts?

lets define YP rules for zero sig transactions so we could define the concrete tests for that

@chriseth
Copy link
Contributor

It is not the gas price that limits the spam, it is the block gas limit.

@winsvega
Copy link
Contributor

So anyone will be able to create contracts for free?

@pirapira
Copy link
Member

@winsvega That's already the case before this EIP if you mine a block that includes your transactions with gas price being zero.

@winsvega
Copy link
Contributor

right. but miniers would most likely refuse such transactions.
not the case with zero sig transactions

@chriseth
Copy link
Contributor

@winsvega it is the same thing. If the zero-sig transaction does not send money to the miner, miners will likely refuse to include it.

@winsvega
Copy link
Contributor

winsvega commented Mar 21, 2017

I am confused. so the gasPrice needs to be > 0 ?

@chriseth
Copy link
Contributor

zero-sig transactions send money to the miner as part of their execution and not as part of the surrounding framework, so gas price should be zero.

@pirapira
Copy link
Member

@winsvega When a miner is constructing a block, the miner can freely exclude transactions. One strategy is doing nothing special about EIP86 transactions (then gasPrice = 0 would be rejected). Another strategy is to exclude all EIP86 transactions. Another strategy is to exclude all EIP86 transactions unless they send Ether to the miner (e.g. a contract CALLs the miner with some value). Then, EIP86 transactions with gasPrice = 0 might be included. An approximation of the previous strategy is to whitelist contracts that pay miners (by CALL ing COINBASE with some value), and exclude all EIP86 transactions except on these whitelisted contracts. Miner strategies are not documented in the EIP.

For somebody sending an EIP86 transaction, it does not make much sense to set gasPrice > 0 because any balance on the null-sender would be quickly be sent away somewhere. So the transaction sender needs a way to pay the miners during EIP86 transactions, and persuade miners to include the EIP86 transactions. The transaction signing strategies are not documented in the EIP.

When this kind of coordination takes place between the transaction senders and the miners, we will see EIP86 transactions in blocks. This coordination between the senders and the miners is not part of the protocol. Even local agreements can exist.

@winsvega
Copy link
Contributor

winsvega commented Mar 21, 2017

so the actual payment for the transaction will be done by a contract that decode data of the zero sig transaction, is that right?

we still have the nonce problem that Andrei mentioned if we do not implement nonce = 0 rule.

the practical problem with nonce checking as I see it - when everyone can send zero sig transactions, and we can have several of them in each block, most of them will be invalid becaus of incorrect nonce
you can't guess the correct nonce when constructing the tx, since anyone else can increment it

or like we already did we could just drop nonce checking for zero sig transactions.
then I guess gasPrice and gasLimit could stay and be verified as for regular transactions.

what I mean by YP rules is that we have to define for zero sig transaction
that everyone would agree

  1. the nonce checking
  2. the gasLimit/gasPrice charge/checking
  3. the value transfer (allow/disallow)
  4. contract creation

@pirapira
Copy link
Member

1 and 2: I think Vitalik's two options both work (but the first option with never-increasing nonce)

3 Disallowing the value transfer would enable an undesired course of events. The null sender receives some large amount by accident, but nobody can send it anywhere, so everyone starts using the null sender's wealth to pay for gas. That's undesired. I prefer allowing value transfer from the null sender because the null sender's wealth will be there only for a short period.

I think the point of this EIP is to specify less and less in the protocol and let the transaction senders and the miners decide.

@vbuterin vbuterin mentioned this pull request Mar 21, 2017
@vbuterin
Copy link
Contributor Author

Here's my EIP for fully "cleaning house" and making new tx types. It's much more work, but in the long run IMO something like this will be necessary.

#232

@tomusdrw
Copy link

tomusdrw commented Mar 23, 2017

What if the transaction goes OOG or uses REVERT at the very end?
Such transaction would be included in the block anyway, but miner will not get the fee, cause all state changes are reverted.

EDIT:
Please disregard the question, I've mistakenly thought that a subcall can cause the entire transaction to be reverted, but that's obviously not the case. So if the top-level contract code is sound, miners/nodes can indeed check if the fee is paid in the first few steps of the contract and include/relay the transaction.

prev_nonce = ~sload(-1)
assert tx_nonce == prev_nonce + 1
assert self.balance >= tx_value + tx_gasprice * tx.startgas
assert ~ecrecover(signing_hash, sig_v, sig_r, sig_s) == <pubkey hash here>
Copy link
Contributor

@jamesray1 jamesray1 Oct 6, 2017

Choose a reason for hiding this comment

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

Should we also abstract the pubkey verification? It could even be abstracted internally, by having a number of precompile contracts where you can call one with an opcode sigVerify. The opcode could even have a default option (where you can call it without any argument). However, I guess that there is not as much benefit to abstract the signature verification algorithm compared to abstracting the signature generation algorithm.

@jamesray1
Copy link
Contributor

Should we include EIP 86 in the title?

@cdetrio
Copy link
Member

cdetrio commented Dec 19, 2017

Discussion of account abstraction proposals has continued here https://ethresear.ch/t/tradeoffs-in-account-abstraction-proposals/263

@Arachnid
Copy link
Contributor

This EIP is mentioned as an example EIP in EIP-1. Can we merge it as a draft? @vbuterin, can you add the copyright assignment clause, please?

@Arachnid Arachnid merged commit 64f6248 into master Mar 27, 2018
@Arachnid Arachnid deleted the vbuterin-patch-3 branch March 27, 2018 14:34
@Arachnid
Copy link
Contributor

This is a courtesy notice to let you know that the format for EIPs has been modified slightly. If you want your draft merged, you will need to make some small changes to how your EIP is formatted:

  • Frontmatter is now contained between lines with only a triple dash ('---')
  • Headers in the frontmatter are now lowercase.

If your PR is editing an existing EIP rather than creating a new one, this has already been done for you, and you need only rebase your PR.

In addition, a continuous build has been setup, which will check your PR against the rules for EIP formatting automatically once you update your PR. This build ensures all required headers are present, as well as performing a number of other checks.

Please rebase your PR against the latest master, and edit your PR to use the above format for frontmatter. For convenience, here's a sample header you can copy and adapt:

---
eip: <num>
title: <title>
author: <author>
type: [Standards Track|Informational|Meta]
category: [Core|Networking|Interface|ERC] (for type: Standards Track only)
status: Draft
created: <date>
---

Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 27, 2018
* Create abstraction.md

Implements a set of changes that serve the combined purpose of "abstracting out" signature verification and nonce checking, allowing users to create "account contracts" that perform any desired signature/nonce checks instead of using the mechanism that is currently hard-coded into transaction processing.

* Update abstraction.md

* Update abstraction.md

* Update abstraction.md

* Update abstraction.md

* Update abstraction.md

* Update abstraction.md

* Update abstraction.md

* Rename abstraction.md to eip-86.md, add copyright clause, fix header.
@Taylorman3
Copy link

So, one use case not being described either in the document or in any of the comments so far is improving the atomicity control over an ecdsa account's direct operations. Suppose I want to transfer an ENS domain "awesome.eth" (currently controlled by an ecdsa key) to the ecdsa account resolved by "vitalik.eth" . From a security model perspective, this is very easy to do safely. First, my client software resolves vitalik.eth, and then it builds a transaction that calls the appropriate ENS contracts, passing in that address as a new owner. Then I sign it and send it, right? But this actually has poor atomicity, because between the time I resolve vitalik.eth and when I send the transaction, the ecdsa account it points to may have been stolen, and the owner of vitalik.eth may have had to point it somewhere else (or any number of other things may have caused a valid and relevant repointing), in which case I am accidentally transferring awesome.eth to an address which the properly updated vitalik.eth registration no longer resolves to. A contract has no such problem, because it can atomically resolve-and-then-send in two consecutive operations that both have it as the msg.sender, with a complete guarantee that the destination has not changed since the first call. An EOA could use a forwarding contract to do both of these operations for it, except that in this example the EOA account is the owner of awesome.eth and using a forwarder would change the msg.sender (because an EOA cannot use delegatecall when calling the forwarding contract). In terms of atomicity, ecdsa accounts are currently second class citizens compared to contracts, and there's no reason for this other than limitations of the current transaction format (in particular, the inability to use delegatecall from an EOA). This is only one of many use cases that suffer in the current model with respect to atomicity (hardware wallets that want to the user to visually confirm important details are particularly hard hit for no inherent security model reason, to give one example).

So the question is: post EIP 208 do we expect even ECDSA users to move over to a new ecdsa-verifying contract to solve their atomicity woes? If so it is extremely important that tx.origin be changed to point to the contract of origin, or almost any existing use of tx.origin will break when that transition occurs (in the most abstract we can say that the purpose of tx.origin is to answer the question "which master puppeteer is causing all of this to happen, regardless of who the latest link in the chain was", and there are certainly a ton of valid use cases for that). Even aside from that I think that removing the current ability to answer that question is a significant step backward.

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.