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

change of tx.origin for account abstraction #637

Closed
frozeman opened this issue May 24, 2017 · 22 comments
Closed

change of tx.origin for account abstraction #637

frozeman opened this issue May 24, 2017 · 22 comments

Comments

@frozeman
Copy link
Contributor

This is a an ERC concerning the account abstraction proposed by @vbuterin and currently as a PR for review here: #208

The reason for this ERC is to gather community feedback concerning a change of tx.origin for transactions from 0xFFFFF...

Abstract

Initially #86 proposed a changes where transaction can happen from a un-owned account named 0xFFFFFF... The reasoning is that this allows for custom signature schemes, as well as 0 gas transactions, where the verifier contract pays for the gas.

This feature comes with the fact that tx.origin or the ORIGIN opcode will point to 0xFFFFF...

My proposal is to make tx.origin point to the tx.origin+1 for those type of transactions, to prevent the loss of the knowledge of the origin of a transaction.

tx.origin+1 will be the contract verifying the signature coming from the 0xFFFFF... account, where everybody can send a transaction from.

Those verifier contracts, could also implement much more features, like representing a identity with verified properties about the identity holder etc.

Discussion and Problems

There is a short discussion here: #208 (comment), which i recommend to read, as well as a length discussion in the AllCoreDevs Gitter channel.

The two sides are:

  1. Not moving tx.origin+1 removes the ability to know from where transactions originated, which might be useful in some cases.
    While allowing automatically verification of identity properties, if the verifier contract, is more than just a signature verifier. This can have all kind of benefits for KYC etc.
  2. BUT, it comes with some uncertainty, as middle men contracts could alter the actions of the origin and therefore slip in malicious behaviour, or transfer call data wrongly.

I would like to invite the community to bring use cases where pointing tx.origin to the verifier contract, rather than the 0xFFFF... address. Or examples why that is not useful.

Suggested solution for 2.

One solution for the middle men problem, would be to send the actual call data along with the signature, so that the receiving contract can verify the calldata - know that it was not manipulated, and execute it. While still be able to check the tx.origin identity/verifier contract for properties a user needs to fullfill (e.g. user is human, has good reputation, etc.)

Request

Please lets discuss the use of tx.origin and its usefulness, so that we either get rid of it completely, or move tx.origin+1 for transactions originating from 0xFFFF... As we are doing the Metropolis hard fork changes now, it its crucial to get to an agreement now, rather than later, and eventually require a second hard fork to solve this issue.

@frozeman frozeman added the ERC label May 24, 2017
@holiman
Copy link
Contributor

holiman commented May 24, 2017

Some context; the Allcoredevs-discussion starts around here: https://gitter.im/ethereum/AllCoreDevs?at=5923f82000efc2bb3e966db1.

@tjayrush
Copy link

I come at this from a different perspective, I think. That of off-chain, after-the-fact forensic analysis or accounting. I don't really understand the issue, but to the extent that this abstraction makes it more difficult to distinguish between contract and external accounts in the data it makes analyzing the data more difficult.

There are certain optimizations one can make if one knows an account is external (for example, there's no need to search if the account participated in an 'internal' transaction' as initiator).

My comment is the ability to distinguish between external and contract accounts is beneficial to off-chain, after-the-fact analysis tools.

@frozeman
Copy link
Contributor Author

In a way external accounts and contract based accounts are similar, in the sense that both require a signature. Your contact account still needs some kind of signature passed into the verifier account. Just thew which signature it is is more flexible than before. The signer account knows which signatures it will allow to work through it, and additional this contract can contain a lot more then just the signature verification.

@alex-miller-0
Copy link

Why not use tx.origin to represent what it always has (for backwards compatibility) and tx.verifier to represent this new abstracted verifier contract?

Is there some reason why losing information is a good thing?

@rabbit
Copy link

rabbit commented May 24, 2017

I don't entirely understand the tradeoffs but, in terms of usefulness, every situation I thought tx.origin would be useful ended up being better served by redesigning for msg.sender instead. I'm more concerned with the abstract action of introducing a breaking change to what feels like a system constant.

@veox
Copy link
Contributor

veox commented May 24, 2017

@frozeman Could you edit the OP to have tx.{origin+1} (or similar)? Was confused on first encounter.

EDIT: Compare to (tx.origin)+1.

@alex-miller-0
Copy link

every situation I thought tx.origin would be useful ended up being better served by redesigning for msg.sender instead

I personally have never used tx.origin either, but I won't presume to understand the design decisions of every app developer in the ecosystem and would suggest we don't break backward compatibility of contracts that can't be updated.

@holiman
Copy link
Contributor

holiman commented May 24, 2017

There are some misconceptions here and on Reddit. , I'll call them origin (the proposed one) and origin+1 for @frozeman 's version.

Regarding backward compatibility.

  • msg.sender == tx.origin checks in use today, will continue to work with origin, but will fail with
    origin+1. Fail, in the sense that the check can no longer verify that the caller is a non-contract-account.
  • origin and origin+1 both kind of destroys backwards compatibility, since multiple accounts can get the same result. However, this is an artefact of the underlying EIP, not the particulars of the opcode.

And I don't see that either way makes any difference for off-chain analysis.

@Arachnid
Copy link
Contributor

Arachnid commented May 25, 2017

@tjayrush I don't see how having this opcode or not will affect that; can you elaborate?

@alex-miller-0

Is there some reason why losing information is a good thing?

Because I, and others, have never seen a use-case for tx.origin that isn't dangerous and broken.

@tjayrush
Copy link

@Arachnid I wasn't really saying this would or would not make it more difficult to distinguish contracts from non-contracts. I was just saying that if it does make distinguishing them off-chain more difficult (i.e. by doing a getCode call to the RPC) that would negatively affect my work.

@coder5876
Copy link

@frozeman I'm also not convinced of the need to keep tx.origin around and I share a lot of the concerns expressed by @Arachnid and @holiman about man-in-the-middle attacks.

I think the identity usecase in the linked gitter convo is not that good of an example since IMO best practices is not to link identities directly to keys (which can/will be lost) but instead to contract addresses or other non-key identifiers (which allows for key rotation etc).

So I'm also in favor of deprecating tx.origin.

I also think that the "origin" word introduces some preconceptions. tx.origin is defined as the hash of the pubkey that signed the transaction. If this signature is supposed to convey some authorization then IMO this is something that should be explicitly passed in with the transaction data and/or function parameters. If the signature is passed in that way it doesn't matter who is actually "sending" the transaction.

@coder5876
Copy link

An example/proposed standard of passing in the signature explicitly is #191 by @holiman

@frozeman
Copy link
Contributor Author

Maybe I don't explain correctly, but the identity/verifier can have key rotation, as it decides which keys to accept.
Sure data can be passed as signature, like I said in the example, it it still
Helps if you can check for WHO send the signature, then you don't need to put the address in the signed data itself.

@Arachnid
Copy link
Contributor

@tjayrush This sounds like an objection to the account abstraction change, rather than to the tx.origin change.

@oojr
Copy link

oojr commented Dec 7, 2017

@frozeman I am trying to catch up why tx.origin is being deprecated? Approved addresses mapping will solve all middleman problems, having the user approve and then transferFrom in two different contracts is okay but having another transfer function that can check tx.origin should be fine, if people send money to a malicious contract tx.origin or msg.sender will not matter. What am I missing?

@axic axic removed the type: ERC label Jul 6, 2019
@CodeMustRule
Copy link

Most contract creators rely on standard require( msg.sender == owner ) making their contracts vulnerable to a highly unlike but not impossible address collision. I personally use require( msg.sender == owner && tx.origin != msg.sender ) ensuring that a random EOA will NEVER gain control of an important contract and compromise user funds. I believetx.origin is useful in its current state for that purpose.

@CodeMustRule
Copy link

@tjayrush I don't see how having this opcode or not will affect that; can you elaborate?

@alex-miller-0

Is there some reason why losing information is a good thing?

Because I, and others, have never seen a use-case for tx.origin that isn't dangerous and broken.

The reasoning behind... I never have used it or I don't know how to use it correctly, doesn't mean something is useless or that no one knows how to use it correctly.

Passing tx.origin up the call chain as msg.sender increases gas usage and blockchains must provide all tools available to program contracts efficiently, not expending precious computing time passing constants from one call to another.

Breaking backwards compatibility out of personal taste in a blockchain full of contracts using tx.origin may cause many non expert users to lose funds due to usage of contracts that will just fail using such feature after you remove or change it.

Stop believing that tx.origin is used only to incorrectly verify authorization.

@MicahZoltu
Copy link
Contributor

Most contract creators rely on standard require( msg.sender == owner ) making their contracts vulnerable to a highly unlike but not impossible address collision.

This collision isn't possible as of recently: https://eips.ethereum.org/EIPS/eip-3607

@CodeMustRule
Copy link

Most contract creators rely on standard require( msg.sender == owner ) making their contracts vulnerable to a highly unlike but not impossible address collision.

This collision isn't possible as of recently: https://eips.ethereum.org/EIPS/eip-3607

This is exactly what I am trying to say, some programmers thought of this before EIP-3607 was proposed and used tx.origin for protecting their logic or making other kinds of logic decisions, now that you try to fix that issue for those with poor skills and that was addressed by good programmers with something similar to msg.sender != tx.origin. You will support the introduction of a breaking change to such well though programmers logic changing the behavior of tx.origin because some bad programmers don't envision good usages of such feature or believe that other programmers can't handle the usage of it. That is my point, don't break code that is immutable because some programmers are bad at their job of handling security, or will you solve reentrancy, economic attacks, randomness... for them, breaking how solidity was supposed to work for making it easier to use by bad programmers.

@MicahZoltu
Copy link
Contributor

In case you missed it, this issue is from 2017, long before any of the things you mentioned. We (core devs and security community) have been telling people to not use tx.origin checks for a very long time, but unfortunately no one listens. 😢

@CodeMustRule
Copy link

In case you missed it, this issue is from 2017, long before any of the things you mentioned. We (core devs and security community) have been telling people to not use tx.origin checks for a very long time, but unfortunately no one listens. 😢

I must insist that my opinion is that changing the behavior of tx.origin is inconvenient and could break contracts that rely in a simple idea, it gives you the address that initiated a transaction, simple and no more than that, you use it considering that logic and the fact that a third party can fool a user to execute a contract call making it useless for authentication purposes. The reasoning that users can be fool in a phishing attack is the same danger as the the fact that users give unlimited approval on ERC20 tokens to any contract they feel the need, if a user executes a compromised contract is not the failure of the language is the user's failure. From my perspective is clear that using msg.sender for security reasons is a must, but breaking tx.origin as it was implemented is useless and hurts the language, I personally love solidity, love blockchain but for me, programming solidity contracts relies in the important premise of immutability. This proposed change or removal of tx.origin gives more reasons for people that feel the main problem of ethereum is centralization in the hands of their developers. I strongly insist that keeping the premise of immutability unless critical is a must (tx.origin is not critical), and as in good politics the principle is that you don't change basic rules, you can implement new ones but without breaking what already many programmers rely or relied on.

In other topics... I wish anyone that reads this comment a ¡happy new year! 😄

@MicahZoltu
Copy link
Contributor

Closing this issue for housekeeping purposes. People are welcome to continue discussing in this thread, but for additional visibility an EIP should be created or the conversation should be migrated to https://ethereum-magicians.org/

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

No branches or pull requests