Skip to content

Display revert location #225

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 27 commits into from
Closed

Display revert location #225

wants to merge 27 commits into from

Conversation

@Jozwiaczek
Copy link
Contributor

@Jozwiaczek Jozwiaczek commented Apr 20, 2020 •
edited
Loading

image

TODO

  • Try to hook-up error overriding in a simpler way
  • Add a lot more validations so it doesn't crash if something is wrong (eg. bytecode doesn't match)
    • Finds source location for a simple revert in the called method
    • Finds source location for a revert in a method called from the called method
    • Finds source location for a revert in an inherited contract
    • Finds source location for a revert in an inherited contract from node_modules
    • Finds source location for a revert in a library
    • Does not find a revert location for contracts called by the called contract. (Note that there shouldn't be any error output in the console - this must be a silent fail)
  • Have a default build dir (./build)
  • Use relative paths in source output (currently absolute - result in nondeterministic builds)
  • Support more RPCs
    • eth_estimateGas
    • eth_sendTransaction
    • eth_call
  • (separate PR) Support combined output JSON
  • (optional) Figure out a way to support internal transactions (evm_trace maybe?)

@Jozwiaczek Jozwiaczek added the Work in progress Work in progress label Apr 20, 2020
@marekkirejczyk
Copy link
Contributor

marekkirejczyk commented Apr 21, 2020

I propose to first fix linter, than tests, than keep refactoring

Jozwiaczek added 18 commits April 21, 2020 12:38
@Jozwiaczek
fix linter problems and add test for finding contract uri
9ae8424
@Jozwiaczek
add tests for getting instruction index
562e769
@Jozwiaczek
wip
5372fdd
@Jozwiaczek
add test build dir
49bbac1
@Jozwiaczek
wip
a9d798f
@Jozwiaczek
add test-sources dir
6812f4c
@Jozwiaczek
Merge branch 'master' into revert-location
958f623 
# Conflicts:
#	waffle-provider/src/MockProvider.ts
#	waffle-provider/test/MockProvider.test.ts
@Jozwiaczek
Merge commit
9e78887
@Jozwiaczek
wip
f260ed5
@Jozwiaczek
wip
5e99f7a
@Jozwiaczek
Revert "wip"
00633f6 
This reverts commit 5e99f7a
@Jozwiaczek
fix code style
d5cb5b0
@Jozwiaczek
Add separate repo for e2e tests with revertLocalizer
9aae3e8
@Jozwiaczek
Add mock contracts into waffle-e2e
ecb959c
@Jozwiaczek
change dir folders into src and dist
01ad0be
@Jozwiaczek
Merge branch 'master' into revert-location
33c6093 
# Conflicts:
#	waffle-compiler/src/compiler.ts
@Jozwiaczek
Add default build path and prevent error during revert location for c…
0b92fce 
…ontracts called by the called contract
@Jozwiaczek
Merge branch 'master' into revert-location
210d3b4 
# Conflicts:
#	waffle-compiler/src/findInputs.ts
#	waffle-provider/test/MockProvider.test.ts
@sz-piotr
Copy link
Contributor

sz-piotr commented May 8, 2020

Let's make sure the CI is green

Jozwiaczek added 3 commits May 8, 2020 16:21
@Jozwiaczek
Integrate with ethers v5
391cb09
@Jozwiaczek
Merge branch 'master' into revert-location
8605114 
# Conflicts:
#	waffle-e2e/.gitignore
#	waffle-e2e/buildTestContracts.ts
#	waffle-e2e/package.json
#	waffle-e2e/src/HelloContract.sol
#	waffle-e2e/test/index.ts
@Jozwiaczek
Merge with e2e and refactor revertLocalizer
4e4074e
@sz-piotr sz-piotr mentioned this pull request May 11, 2020
Closed
33 tasks
@marekkirejczyk
Copy link
Contributor

marekkirejczyk commented Feb 25, 2021

Seem to be stalled, closing for now. Feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Work in progress Work in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Jozwiaczek @marekkirejczyk @sz-piotr @dmaretskyi