Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

v7.0 Breaking Changes #451

Closed
8 of 9 tasks
davidmurdoch opened this issue Jul 15, 2019 · 10 comments
Closed
8 of 9 tasks

v7.0 Breaking Changes #451

davidmurdoch opened this issue Jul 15, 2019 · 10 comments

Comments

@davidmurdoch
Copy link
Member

davidmurdoch commented Jul 15, 2019

This is a list of planned breaking changes in v7.0.0.

  • r, s values will be returned QUANTITY encoded for eth_getTransactionByHash
  • eth_sign should return chainId-relative signature with high-level v values
  • fix bug where ganache treats all transactions as having a chainId of 1, but RPC eth_chainId returns 1337 (we need to just make chainId configurable). We need investigate aligning net_version and chain_id as metamask uses net_version instead of chain_id for signing transactions.
  • move reporting of tx hash on error to error field to prevent poorly-written clients which assume that the existence of the "result" field implies no errors from breaking.
  • relative to above: don't return both result and an err, ever. Maybe. Need to discuss further.
  • flip the VM errors on RPC response flag
  • old db format will no longer be valid. Investigate how to update old dbs to the new one without forcing the user into the new release.
  • Update web3_clientVersion to include Ganache in its return value.
  • improve precision of internal timeAdjustments (this is more of a bug fix that could break test that use specific block times via the time option, evm_increaseTime, or evm_setTime)
@moodysalem
Copy link

Does web3.eth.accounts.sign generate signatures with a v of 27/28? It seems that it does but does not generate correct signatures according to this test

https://travis-ci.org/ethvault/ens-registrar-contract/jobs/561935235#L612

Otherwise is there a workaround for writing tests for code that uses ECDSA.recover?

@mrwillis
Copy link

mrwillis commented Oct 23, 2019

@moodysalem did you find a solution? I'm wondering if there is a workaround as well. Currently cannot verify signatures correctly using Ganache CLI v6.7.0-beta.0 (ganache-core: 2.8.0-beta.0) due to the v parameter.

@davidmurdoch perhaps we could get a special release? Basically if you are using openzeppelin-solidity>=2.2 signature verification will not work :(

@moodysalem
Copy link

moodysalem commented Oct 23, 2019

@mrwillis I reverted to an older version of the ECDSA code.

This is safe if you don’t check signatures are only used once. Instead I associate an expiration timestamp with each signature and multiple calls are idempotent.

@MicahZoltu
Copy link
Contributor

move reporting of tx hash on error to error field to prevent poorly-written clients which assume that the existence of the "result" field implies no errors from breaking.

The JSON-RPC specification explicitly states that

[result] MUST NOT exist if there was an error invoking the method

and

[error] MUST NOT exist if there was no error triggered during invocation

I wouldn't call someone who wrote a JSON-RPC client to-spec "poorly-written".

@davidmurdoch
Copy link
Member Author

Thanks for the clarity on this, @MicahZoltu. This line was copy-pasted from a TODO in our source code: 0ed5d17#diff-1c25f2af06a7bfd6c36abff0f88b9a2bR189-R191

I think the comment in the code there was written because the author is a huge proponent of The Robustness Principle:

Be conservative in what you do, be liberal in what you accept from others (often reworded as "Be conservative in what you send, be liberal in what you accept").

In other words, programs that send messages to other machines (or to other programs on the same machine) should conform completely to the specifications, but programs that receive messages should accept non-conformant input as long as the meaning is clear.

@MicahZoltu
Copy link
Contributor

I am a big fan of the Robustness Principle. However, in JSON-RPC the presence/non-presence of those two variables are the intended mechanism for discriminating between success and failure, and having both is considered an error (per the specification). Because of that, I don't think the robustness principle applies here as the correct way to discriminate is to look for either a success or an error and expect never both.

@balthazar
Copy link

Any progress on this, particularly the chainId hardcoded to 1? Still having the issues with Ganache 2.4.0

@davidmurdoch
Copy link
Member Author

@balthazar yes! You can follow along in the ts branch of this repository.

@frangio
Copy link

frangio commented Apr 20, 2021

Sorry to bump this. Are there any updates on 3.0? I can't find the ts branch.

Edit: Ok I just found that it was merged to the next branch. I would love any updates about the roadmap to get 3.0 released.

@davidmurdoch davidmurdoch changed the title v3.0 Breaking Changes v7.0 Breaking Changes Apr 20, 2021
@davidmurdoch
Copy link
Member Author

@frangio It was merged into develop. We're working on buttoning some things up and porting the last feature: forking.

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

No branches or pull requests

6 participants