Skip to content
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.

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

Standardjson #569

Merged
merged 36 commits into from
Jan 11, 2021
Merged

Standardjson #569

merged 36 commits into from
Jan 11, 2021

Conversation

MrChico
Copy link
Member

@MrChico MrChico commented Dec 10, 2020

Addresses #168, #572 and accommodates for Solidity deprecating --combined-json.
Refactors build-dapp-package.nix to use dapp-build instead of handcrafted solc invocation.
Some TODOs, might be more things that I have not yet thought of:

  • dapp create
  • dapp --make-library-state.
  • seth combined-json & seth bundle-source
    • fixed and renamed to seth solc
    • make bundle-source work with a standard json etherscan example
      - [ ] hevm needs to interpret the sources in more detail, differentiating between "urls" and "content"
      - [ ] then we can check that seth run-tx/debug work against both flattened and standard json examples
      EDIT: url and content is not available in the compiler output, we would need to parse the input file as well. Postponing this.
  • dapp build could interpret and render the solc errors in the json

mapfile -t sources < <(find "${DAPP_SRC?}" "$DAPP_LIB" -name '*.sol')
grep -EH '^library\s+\w+' "${sources[@]}" \
| sed -E 's/:library[[:blank:]]+([_[:alnum:]]+)[[:blank:]]+(\{|is).*/:\1/' \
| sort | while read -r line; do
# Only include libraries that have actually been built.
if [[ -f $(binfile <<<"$line") ]]; then echo "$line"; fi
echo "$line";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you tested this with some projects that use libraries?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a toy example, which I will add to dapp-test. Still needs --make-library-state to work tho

"evmVersion": "byzantium",
// Optional: Change compilation pipeline to go through the Yul intermediate representation.
// This is a highly EXPERIMENTAL feature, not to be used for production. This is false by default.
"viaIR": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set these to sane defaults and then actually use them rather than popping?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe... The template right now is mainly used to be informative and demonstrate the capabilities of the std json.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One problem would be compatibility with older solc versions. Are options just ignored if they aren't used by solc? The evmVersion could be especially problematic, does solc just fill this with a default if you don't supply it?

MrChico and others added 7 commits December 16, 2020 13:27
Co-authored-by: rain <rainbreak@riseup.net>
instead of handcrafted building script

A few minor tweaks to `dapp-build` and `dapp-mk-standard-json`
We have to specify gas prices like `seth --gas-price 100gwei` as we
can't have spaces in these arguments, unless we provide a quoted
argument like `seth --gas-price "100 gwei"`.
introduces the implicit parameter DappContext, which can be used in
`showAbiValue` to e.g. render addresses with their contract name.

We now render addresses as `ContractName@0x....` when the address is
associated with a contract and a name, and `@0x...` otherwise.
This allows us to create the build artefacts for contracts on etherscan
that have been verified by the standard json method (as opposed to the
flattened file method).

N.B. hevm does not actually support this yet as it does not interpret
the "content" field in the sources
@MrChico MrChico marked this pull request as ready for review January 4, 2021 18:45
@MrChico
Copy link
Member Author

MrChico commented Jan 4, 2021

This is starting to get pretty ready. It's a fairly broad change, so I would like to see this get some testing before we make a release out of this. But things seem to work quite well now, might even be a bit more robust when it comes to libraries.

@MrChico MrChico requested a review from d-xo January 4, 2021 18:48
@MrChico
Copy link
Member Author

MrChico commented Jan 5, 2021

Actually, the data we need to find the sources from the json output are available in the metadata section

@MrChico
Copy link
Member Author

MrChico commented Jan 7, 2021

This now also supports hevm parsing of file content embedded directly in the standard json. Only missing now is support for standard json in seth bundle-source, but that could also wait until a future PR

@MrChico MrChico mentioned this pull request Jan 7, 2021
@ethernomad
Copy link

I can test pragma solidity ^0.8.0; code with this branch.

src/dapp/CHANGELOG.md Outdated Show resolved Hide resolved
src/dapp/CHANGELOG.md Outdated Show resolved Hide resolved
src/dapp/libexec/dapp/dapp---make-library-state Outdated Show resolved Hide resolved
### files from the solc json into $DAPP_OUT. Beware of contract
### name collisions. This is provided for compatibility with older
### workflows.
### --optimize: activate the solidity optimizer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we allow users to specify the runs from the command line as well?

src/dapp/libexec/dapp/dapp-build Show resolved Hide resolved
src/dapp/libexec/dapp/dapp-build Show resolved Hide resolved
src/dapp/libexec/dapp/dapp-build Outdated Show resolved Hide resolved
src/hevm/src/EVM/Format.hs Outdated Show resolved Hide resolved
MrChico and others added 2 commits January 8, 2021 13:42
Co-authored-by: David Terry <xwvvvvwx@users.noreply.github.com>
Comment on lines +32 to +33
- `SOLC_FLAGS`. To modify the compiler settings, use a custom standard json and set
the filename as argument to `DAPP_STANDARD_JSON`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `SOLC_FLAGS`. To modify the compiler settings, use a custom standard json and set
the filename as argument to `DAPP_STANDARD_JSON`.
- `SOLC_FLAGS`. To modify the compiler settings, use `--optimize` or set `DAPP_STANDARD_JSON` to the location of a hand written standard json.

src/dapp/README.md Outdated Show resolved Hide resolved
src/dapp/README.md Outdated Show resolved Hide resolved
Co-authored-by: David Terry <xwvvvvwx@users.noreply.github.com>
@MrChico
Copy link
Member Author

MrChico commented Jan 11, 2021

I say we merge this and try it out on master for a bit before release

@MrChico
Copy link
Member Author

MrChico commented Jan 12, 2021

hmm, I think dapp create is still needed, no it seems fine

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

Successfully merging this pull request may close these issues.

hevm pretty prints address arguments with leading zeros missing Output compact AST when running dapp build
4 participants