celo-org
CIP 35 - Supporting Ethereum-compatible Transactions - Discussions
An open question is whether it may be better to do without adding the ethCompatible field to the JSON representation, as it is not strictly speaking necessary.
Downsides of including it:
- The implementation is more complex, requiring adding and handling the new field when dealing with transactions in JSON
- It constitutes a change to the JSON representation, though its presence shouldn't break compatibility with existing software
Upsides of including it:
- It makes explicit the kind of transaction it is, when received in JSON format (which is the format in which transactions are output by the RPC, such as in
eth_getTransactionoreth_getBlock). Without this field, for a transaction which has the zero-values for the Celo-only fields (which is quite common), the only way to tell whether it's an Ethereum-compatible transaction (and, therefore, to know what its signing-data and its RLP representation should be) would be trial and error: you would first try to verify the signature assuming it's a normal Celo transaction, and if that fails you would then try to verify it assuming it's an Ethereum-compatible transaction. - It makes Ethereum-compatible transactions first-class citizens, in that RPC methods such
eth_signTransactionandeth_sendTransactionwould be able to create such transactions. If there were noethCompatiblefield, these methods would have to either (a) never produce Ethereum-compatible transactions or (b) decide which type to use based on whether the Celo-only fields have their zero-values, meaning they would always produce Ethereum-compatible transactions in such cases. Though there is no practical need to generate such transactions via the RPC (except perhaps for the purpose of testing), the inability to do so would mark this transaction type as less than fully supported.
I'm a big believer in explicit being better than implicit, and I don't like the idea of introducing a new transaction type without having full support for it through RPC, so my preference is to include the new field. But it is not a hard requirement in order to achieve the goal of this CIP, which is enabling the use in Celo of transactions generated by Ethereum tooling without special support for Celo's transaction format.
Oh, quick note, have we considered how our implementation of EIP1559 affects this? Our semantics of the gas fields are slightly different, right? I'm not super clear on the details
Oh, quick note, have we considered how our implementation of EIP1559 affects this? Our semantics of the gas fields are slightly different, right? I'm not super clear on the details
The only difference in their semantics compared to Ethereum is that if you use a non-CELO fee currency (which is a case that doesn't exist in Ethereum), then the gas price is denominated in that fee currency.
Regarding EIP1559, what we have in Celo isn't closely aligned with EIP1559, but rather consists of a smart contract that's responsible for adjusting the minimum gas price (which is then used in the blockchain client). The semantics of the gas price and the transaction's gas limit are not affected (the client just checks that the gas price is at least as high as the minimum gas price for the fee currency being used and that the account can afford to pay for it).
This does tangentially raise the question of whether adoption of this CIP will in the future force or nudge our hands to having to adopt expected changes such as https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2718.md, which slipped my mind when writing the CIP. I will open up a PR to add a section about that.
I would like to voice my concern that while this CIP makes it easier to use Ethereum tooling to access Celo, it also eschews explicit Celo features and possibly encourages a class of dapps to not support them and inadvertently making them inaccessible to many target users of Celo, namely the ones primarily holding stable tokens.
EDIT: While having that concern, I personally still am in favor of passing this CIP as the benefits are large
From @dukejones in the ACD call:
The ability to define a user account default feeCurrency could solve [eth-compatibility users being unable to pay fees in cUSD] in practice
One thing I'm wondering how big of an issue is the different deviation path for Ethereum vs. Celo. I.e. if seed A produces address X on Ethereum, but address Y on Celo, will that be confusing? Does this get more complicated if I use a ledger with Metamask (I'd assume that it would use the Ethereum app?)
I would like to voice my concern that while this CIP makes it easier to use Ethereum tooling to access Celo, it also eschews explicit Celo features and possibly encourages a class of dapps to not support them and inadvertently making them inaccessible to many target users of Celo, namely the ones primarily holding stable tokens.
We should definitely be wary of this. But I think that currently our users are missing out entirely on a lot of compelling applications. I think we can work on addressing this for cUSD users by first ensuring it is as easy as possible to create an applications, and then helping resource and build mobile interfaces for these contracts
From @dukejones in the ACD call:
The ability to define a user account default feeCurrency could solve [eth-compatibility users being unable to pay fees in cUSD] in practice
Is that possible with upstream Metamask? Also, IIUC, gasPrices are priced in feeCurrency, and thus the dapp (or Metamask?) would need to be aware of that?
One thing I'm wondering how big of an issue is the different deviation path for Ethereum vs. Celo. I.e. if
seed Aproducesaddress Xon Ethereum, butaddress Yon Celo, will that be confusing? Does this get more complicated if I use a ledger with Metamask (I'd assume that it would use the Ethereum app?)
The emerging standard for eth-compatible network tooling is to use the same address on each network. Metamask users have the same addressed on Eth, BSC, Optimism, Polygon, etc.. This does cause some complication for us in the future deriving addresses from seed phrases, as a user importing a seed phrase that has been used in another wallet may have Celo in the Eth path AND in the Celo path
From @dukejones in the ACD call:
The ability to define a user account default feeCurrency could solve [eth-compatibility users being unable to pay fees in cUSD] in practice
Is that possible with upstream Metamask? Also, IIUC, gasPrices are priced in
feeCurrency, and thus the dapp (or Metamask?) would need to be aware of that?
The frontend would need to incorporate Celo-specific logic to instruct metamask on the exact gasprice and gas limit to use. Metamask would misreport the fee asset to the user. So it is not perfectly compatible. Power user solution only 👎
While I don't think there are security concerns with a tx being entirely valid on both Celo and ETH, I did find myself spending a good amount of time thinking this through. Some commentary in the proposal as to why this is not a concern could be beneficial for the broader community as this works its way towards a vote/adoption.
While I don't think there are security concerns with a tx being entirely valid on both Celo and ETH, I did find myself spending a good amount of time thinking this through. Some commentary in the proposal as to why this is not a concern could be beneficial for the broader community as this works its way towards a vote/adoption.
Good feedback. We should at least reference EIP-155 explicitly in the CIP. It neatly prevents replay attacks across EVM-compatible networks
While I don't think there are security concerns with a tx being entirely valid on both Celo and ETH, I did find myself spending a good amount of time thinking this through. Some commentary in the proposal as to why this is not a concern could be beneficial for the broader community as this works its way towards a vote/adoption.
Good feedback. We should at least reference EIP-155 explicitly in the CIP. It neatly prevents replay attacks across EVM-compatible networks
I will open up a PR for this.
[Edit: PR opened as #174]
On the topic of whether or not to include an ethCompatible field in non-RLP representations, I'm now in favor of including it overall. I'm not aware of any other supporters of excluding it, so I imagine that settles the issue 😅
@prestwich @YazzyYaz Coming back to the topic that came up in #174 about replay-protection being optional in EIP-155, I realized that the branch as is would not work for non-replay-protected transactions because I didn't updated the Homestead/Frontier signer. I added a commit to the oneeman/cip35 branch that adds this, but then it occurred to me that the CIP spec is incomplete as it doesn't mention the non-replay-protected case and in fact assumes replay-protection is mandatory, which isn't the case. Not sure how we should proceed, whether we can do a PR at this point to add that case.
We should do a PR to adjust. I would suggest that we do make EIP-155 protection mandatory. At this point it seems that 155 adoption is essentially 100% and that there is no use case for supporting unprotected txns.
On Thu, Mar 11, 2021, 14:38 Or Neeman @.***> wrote:
@prestwich https://github.com/prestwich @YazzyYaz https://github.com/YazzyYaz Coming back to the topic that came up in #174 https://github.com/celo-org/celo-proposals/pull/174 about replay-protection being optional in EIP-155, I realized that the branch as is would not work for non-replay-protected transactions because I didn't updated the Homestead/Frontier signer. I added a commit to the oneeman/cip35 branch that adds this, but then it occurred to me that the CIP spec is incomplete as it doesn't mention the non-replay-protected case and in fact assumes replay-protection is mandatory, which isn't the case. Not sure how we should proceed, whether we can do a PR at this point to add that case.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/celo-org/celo-proposals/issues/166#issuecomment-797096807, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNN4MJ4AN5CJ3WA3N6WZ5LTDFA6FANCNFSM4YBIZHQA .
Will do. I had a look at the code and it's trivial to require replay protection starting at Donut activation, so I will make that change and add tests for it. As far as the CIP, the specs as they currently are do (as an oversight) require replay protection, so I will open a PR clarifying in the spec that this is a change compared to the pre-Donut behavior.
Cool. Can we easily sync a node with mandatory replay protection on the current chain, to see if any non-eip155 txns exist already?
On Fri, Mar 12, 2021, 09:22 Or Neeman @.***> wrote:
Will do. I had a look at the code and it's trivial to require replay protection starting at Donut activation, so I will make that change and add tests for it. As far as the CIP, the specs as they currently are do (as an oversight) require replay protection, so I will open a PR clarifying in the spec that this is a change compared to the pre-Donut behavior.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/celo-org/celo-proposals/issues/166#issuecomment-797636958, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNN4MKUBOM6J5ZUQELREJDTDJEVZANCNFSM4YBIZHQA .
Yeah, I can create a branch that panics if it sees a block with a non-protected transaction and try to sync it to mainnet. I'm guessing there wouldn't be any, since a person would have to roll their own software to generate a non-protected Celo transaction, but we'll see.
I implemented the change to make replay protection mandatory once Donut is activated, and added e2e tests to check taht part as well. Here is the PR to update the specs: https://github.com/celo-org/celo-proposals/pull/194 .