Skip to content

Remove compiled contracts from repository#34

Closed
vsbogd wants to merge 1 commit into
singnet:masterfrom
vsbogd:remove-compiled-contracts-from-repo
Closed

Remove compiled contracts from repository#34
vsbogd wants to merge 1 commit into
singnet:masterfrom
vsbogd:remove-compiled-contracts-from-repo

Conversation

@vsbogd

@vsbogd vsbogd commented Sep 24, 2018

Copy link
Copy Markdown
Member

Remove compiled contracts as they should be created during the building process. Add build folder to the .gitignore.

This PR depends on #33

Remove compiled contracts as they should be created during the building
process. Add build folder to the .gitignore.
@tiero

tiero commented Sep 25, 2018

Copy link
Copy Markdown
Contributor

The reason of compiled artifacts into git repo is having a reliable source to fetch from the abi and the contract bytecode.

If we want to explore a better alternative, let's discuss it in an isolated issue.

I propose to close it for now

@vsbogd

vsbogd commented Sep 26, 2018

Copy link
Copy Markdown
Member Author

Ok I agree to close it for now and I will add commit to remove compiled AphaRegistry to the #33 PR.

But I don't understand the reason we need to keep compiled artifacts. What is wrong with compiling them again when we need them?

@tiero

tiero commented Sep 26, 2018

Copy link
Copy Markdown
Contributor

We cannot recompile in every situation, eg. the dapp. Being quite expensive the compilation with solc-js in the browser, we will end up to "vendorize" somehow the contract in that repo, so a this point better distribute artifact from a single source of truth (eg. npm) as we do right now.

I can agree that git/npm it can be the sub-optimal solution for the future, but I strongly believe we need to have the artifacts already compiled somewhere. (swarm, ipfs etc..)

@vsbogd

vsbogd commented Sep 26, 2018

Copy link
Copy Markdown
Member Author

Sure, I agree that client applications should use released binaries instead of compiling sources each time, but I thought that released npm package should solve this problem.

Do you mean that DApp js code includes this artifacts downloading them directly from GitHub?

@tiero

tiero commented Sep 26, 2018

Copy link
Copy Markdown
Contributor

They are included in npm cause they are ALSO included in the git repo.
When you install the npm package you have both sources and artifacts. We can have a more complex and advanced release strategy, but for now, I will not put so much energy and just adopt the approach of the past.

@vsbogd

vsbogd commented Sep 27, 2018

Copy link
Copy Markdown
Member Author

Not sure I understand you correctly probably because I am not very familiar with procedure of Ethereum contracts publishing as npm packages. Excuse me if it is a silly question, but do you mean that npm package referes to GitHub repo which in turn should contain contract binaries in JSON form? What is the reason why npm package cannot include binaries then?

@tiero

tiero commented Sep 27, 2018

Copy link
Copy Markdown
Contributor

The npm package contains the solidity contract source code along with the relative compiled artifact (let's say the binary) as it does here https://github.com/singnet/platform-contracts/blob/master/scripts/package-npm.js

We should ask to @vforvalerio87 for more clue on this decision, but IMO to be more transparent between what we open source and what is inside the npm package, it is in the repo as a proof.

On this matter, I would propose to commit the sha512 hash of the git patch on each git commit message along with a checksum of each release of the npm package. (not urgent tho)

EDIT: the SHA512 thing is because SHA1 it's vulnerable and we, as developers, we can become surface of an attack. More clue here bitcoin/bitcoin#9871

@vsbogd

vsbogd commented Sep 28, 2018

Copy link
Copy Markdown
Member Author

Am I right that you propose to publish reference to the git HEAD within npm package?
It sounds reasonable for me as if someone doesn't trust to npm artifact then one can get the sources by git tag and compile artifact by himself. It is an usual way to do it in open source as far as I know.

If one don't like compiling it by himself one should rely on publisher. To make this work publisher should sign artifacts off and consumer should check the sign. Something like this.

@vsbogd vsbogd deleted the remove-compiled-contracts-from-repo branch October 12, 2018 20:07
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.

2 participants