Adjust 1.4.1 tests for zkSync #625
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
I have read the CLA Document and I hereby sign the CLA |
48429fc to
3a9ec81
Compare
| { gasLimit: 1000000 }, | ||
| ); | ||
|
|
||
| // Reverted reason seems not properly returned by zkSync local node, though it is in fact GS010 when using debug_traceTransaction |
There was a problem hiding this comment.
can this be extracted into a utility function? e.g., assertZksyncRevertWithMessage that would do this under the hood?
There was a problem hiding this comment.
Sorry, I didn't get your point. Do you wanna to show this comment during tests execution?
There was a problem hiding this comment.
The test only checks that the transaction is reverted and doesn't check the revert message. However, a comment says that you can use debug_traceTransaction to verify that it is in GS010. Is there a way to write a utility function that would call debug_traceTransaction under the hood and verify the error message?
| }); | ||
|
|
||
| it("simulate selfdestruct", async () => { | ||
| it("simulate selfdestruct", async function () { |
There was a problem hiding this comment.
All other tests uses arrow function expressions, please adjust this test and make sure it's consistent across the updated code (I was some other test files where this was changed)
There was a problem hiding this comment.
Arrow function has no own context. Function expression was changed to enable such features as inclusive tests on some condition.
There was a problem hiding this comment.
Ah, I see. But it seems there are some places where it was changed not to enable anything? Like https://github.com/safe-global/safe-contracts/pull/625/files#diff-291a4add941d879c99b79d09eed2c4206c6f23edd4f5edbc93d2bdd18e1c91deR110-R153
There was a problem hiding this comment.
oops, indeed
I will revert them back 😉
Some details:
There are some places where we skipped the tests because of .send function usage, it was not properly supported by zksync. However, you have already changed it .call. Therefore, we removed the conditional skipping. But this is a leftover...
387ff23 to
fd1f3b0
Compare
|
Hello @mmv08! I hope you are doing well 😌 I can see only two options here:
Both options are not so good from dev perspective. If you have anything in mind, please let us know. |
As agreed previously #588 (comment)