trufflesuite / ganache

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
2.62k stars 679 forks source link

v7.0 Breaking Changes #451

Closed davidmurdoch closed 3 years ago

davidmurdoch commented 5 years ago

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

moodysalem commented 5 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

Thanks for the clarity on this, @MicahZoltu. This line was copy-pasted from a TODO in our source code: https://github.com/trufflesuite/ganache-core/commit/0ed5d175030137a36bb8d3ee3afb440b213966d9#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 commented 4 years ago

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 commented 4 years ago

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

davidmurdoch commented 4 years ago

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

frangio commented 3 years ago

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 commented 3 years ago

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