Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Simplify generate.sh #35

Closed
ggwpez opened this issue Apr 6, 2021 · 6 comments
Closed

Simplify generate.sh #35

ggwpez opened this issue Apr 6, 2021 · 6 comments
Labels
enhancement Improvement of an existing feature

Comments

@ggwpez
Copy link
Contributor

ggwpez commented Apr 6, 2021

Location: generate.sh

Description:
Use the following snippet to get rid of the awk call and simplify the script.

Snippet from @sebastianst, link:

#!/bin/bash

# SPDX-License-Identifier: Apache-2.0

set -e

# Configuration
solpath="../contracts/contracts"
bindpath="../../bindings" # reverse to solpath
solc="solc-static-linux"

# Download solc.
wget -nc "https://github.com/ethereum/solidity/releases/download/v0.8.3/${solc}"
chmod +x $solc
echo -e "⚠ Ensure that the newest version of abigen is installed"

# Compile in solpath
cd "${solpath}"
"${bindpath}/${solc}" --combined-json abi,bin,bin-runtime \
  --allow-paths '.,../node_modules/@openzeppelin' \
  --optimize --optimize-runs 10000 \
  --overwrite -o "${bindpath}" \
  "@openzeppelin=../node_modules/@openzeppelin" \
  *.sol

# Bind
cd "${bindpath}"
abigen --pkg bindings --out bindings.go --combined-json combined.json \
  --exc Address,Bytes,Context,ECDSA,ERC165,ERC20,ERC721,IERC165,IERC721Enumerable,IERC721Metadata,IERC721Receiver,Ownable,Sig,Strings

echo -e "🎉 Bindings generated"
@ggwpez ggwpez added the enhancement Improvement of an existing feature label Apr 6, 2021
@sebastianst
Copy link
Contributor

Did you try it out yet @ggwpez ? I think at least the --exc argument needs adjustment. Although they seem to be ignored right now anyways...

@ggwpez
Copy link
Contributor Author

ggwpez commented Apr 6, 2021

Did you try it out yet @ggwpez ? I think at least the --exc argument needs adjustment. Although they seem to be ignored right now anyways...

I did not try it yet, just wanted to put have it written down as an issue. @sebastianst

@sebastianst
Copy link
Contributor

BTW fixed the behavior of --exc in ethereum/go-ethereum#22620

@matthiasgeihs
Copy link
Contributor

We are currently not using awk in script generate.sh.

To be honest, I don't think our script is very complicated. The most complicated part is generating the BinRuntime.go file for each contract, but I don't think this is solved in the snippet, right?

@sebastianst
Copy link
Contributor

The current generate.sh script has two related issues. The abigen call

    $ABIGEN --pkg $PKG --sol ../contracts/contracts/$FILE.sol --out $PKG/$FILE.go --solc $SOLC

passes the solc compiler via --solc. Unfortunately, this method doesn't allow setting compiler flags. So in particular, no --optimize-runs 10000 can be specified. This means that contracts deployed using the bindings are not optimized, which is currently not an issue, but when we move towards production, the contracts should be optimized to minimize transaction costs.

The second issue is that the generation of the BinRuntime.gos use a second direct solc call, possibly with different options that abigen internally. So the resulting deployed-bytecode checks may fail. It was unclean in the first place to compile the contracts twice, once for bindings generation and a second time for binruntime generation. Using --combined-json is therefore cleaner and more consistent (and faster, because the whole contracts tree is only compiled once). Only thing that would be left to do is extract the bin-runtime from the combined.json, which should be possible with a little jq magic ✨

It's not an urgent issue to fix, but I reopen it as to not forget about it.

@sebastianst sebastianst reopened this Apr 13, 2021
@ggwpez
Copy link
Contributor Author

ggwpez commented Apr 13, 2021

Using --combined-json is therefore cleaner and more consistent (and faster, because the whole contracts tree is only compiled once)

This would also help to verify the Code on Etherscan, right? @seb
I think @matthiasgeihs tried it by combining the contracts but it is needlessly difficult.

PS: Feel free to edit the issue.

@hyperledger-labs hyperledger-labs locked and limited conversation to collaborators Apr 13, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants