Fix problems with failing prepare-dependencies.sh script
#142
Assignees
Labels
deploymentComments
michalinacienciala
added a commit
that referenced
this issue
Jun 6, 2023
…ncies-script Remove prepare dependencies script Refs #142 This PR removes `prepare-dependencies.sh` script that randomly errors out and gives a lot of headaches to developers. The script was added to mitigate the issue with the same naming of `TokenStaking.sol` contracts that are in `keep-core` and `solidity-contracts` repos. In some projects, both repos were added as dependencies and hardhat had a hard time differentiating the contracts during deployment. Since v1 contracts are already deployed and won't be changed, we can now hardcode the artifacts and get rid of the troubled `prepare-dependencies.sh` script and related code. The following repos were checked for any potential issues as a consequence of removing this script. I didn't find any. - threshold-network/token-dashboard - threshold-network/merkle-distribution - keep-network/coverage-pools - keep-network/keep-core (random-beacon and ecdsa) Depends on #144
This was referenced Jun 6, 2023
Merged
|
The problem got fixed by @dimpar in #143. I published new NPM I also removed some no longer needed workarounds in the projects that use We still may hit the issue occasionally when deploying on Mainnet or Goerli (to fix that we would have to redeploy |
This was referenced Jun 7, 2023
|
Another PRs related to the removal of |
dimpar
added a commit
to keep-network/tbtc-v2
that referenced
this issue
Jun 9, 2023
Background: Previously we've been running postinstall script in a separate step from the `yarn install` command. We've been doing that as a workaround for a problem we had with the postinstall script in the `@threshold-network/solidity-contracts` package (the script in that package often randomly failed). Running `yarn upgrade` with `--ignore-scripts` flag prevented the execution of postinstall script in the main project and its depandencies. And running `yarn run postinstall` after did execute the postinstall script in the main project. The change: Recently we have refactored the `@threshold-network/solidity-contracts` project and got rid of the problematic script. We can go back to executing `yarn install` without the `--ignore-scripts` flag. Ref: threshold-network/solidity-contracts#142 threshold-network/solidity-contracts#143 threshold-network/token-dashboard#531
dimpar
added a commit
to keep-network/keep-core
that referenced
this issue
Jun 14, 2023
We have published new `@threshold-network/solidity-contract` and `@keep-network/random-beacon` packages. Those packages no longer include the `prepare-dependencies.sh` script from `@threshold-network/solidity-contracts` which was causing random failures during `yarn install`. As in some CI jobs we install the dependencies based on the lockfile (with the `--frozen-lockfile` flag), we need to update the dependencies in the lockfile so that the new, improved packages would be used. Refs: threshold-network/solidity-contracts#142 threshold-network/token-dashboard#533 keep-network/coverage-pools#233 keep-network/tbtc-v2#631
dimpar
added a commit
to keep-network/tbtc-v2
that referenced
this issue
Jun 14, 2023
We have published new `@keep-network/random-beacon`, `@keep-network/ecdsa` and `@keep-network/tbtc-v2` packages. Those packages no longer include the `prepare-dependencies.sh` script from `@threshold-network/solidity-contracts` which was causing random failures during `yarn install`. As in some CI jobs we install the dependencies based on the lockfile (with the `--frozen-lockfile` flag), we need to update dependencies in the lockfile so that the new, improved packages would be used. Refs: threshold-network/solidity-contracts#142 threshold-network/token-dashboard#533 keep-network/keep-core#3618 keep-network/coverage-pools#233
dimpar
added a commit
to keep-network/tbtc-v2
that referenced
this issue
Jun 15, 2023
We have published new `@keep-network/ecdsa` and `@keep-network/tbtc-v2` packages. Those packages no longer include the `prepare-dependencies.sh` script from `@threshold-network/solidity-contracts` which was causing random failures during `yarn install`. As in some CI jobs we install the dependencies based on the lockfile (with the `--frozen-lockfile` flag), we need to update dependencies in the lockfile so that the new, improved packages would be used. Ref: threshold-network/solidity-contracts#142 threshold-network/solidity-contracts#143 #631
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Background
We are using
keep-network/keep-coredependency in several projects:solidity-contractstoken-dashboardcoverage-poolsThe
keep-network/keep-corepackage contains a KEEP-related contract calledTokenStakingwhich is the same name that we use for staking contract for T token. We wanted to deploy both contracts in the same deployments, so we hit a contract name conflict (see issue wighawag/hardhat-deploy#241). That's why we created aprepare-dependencies.shscript that gets executed as a last step of installation ofthreshold-network/solidity-contractsproject and renames thekeep-core'sTokenStakingartifact toKeepTokenStaking.The issue(s)
The script is used as a
postinstallscript and uses theINIT_CWDvariable as a parameter:solidity-contracts/package.json
Line 29 in f468a7d
The INIT_CWD isn't set by us, it's a variable created by Yarn that should hold the full path of the localization where
yarn run/yarn installwas executed from. Unfortunately, sometimes it does not get configured correctly (it happens seemingly randomly) and causes the failures of the script.Another issue is that in the script we're copying the files from the following location:
solidity-contracts/scripts/prepare-dependencies.sh
Line 20 in f468a7d
This location sometimes may not be available - this is due to the fact that Yarn flattens the structure of dependencies. For example, when
threshold-network/solidity-contractsis a dependency of some project, Yarn may not include@keep-network/keep-coreinthreshold-network/solidity-contracts'snode_modulesif@keep-network/keep-coreis already included innode_modulesof another dependency.Examples:
https://github.com/keep-network/tbtc-v2/actions/runs/4969553945/jobs/8912960954:
https://github.com/keep-network/tbtc-v2/actions/runs/4969553945/jobs/8892795133
https://github.com/keep-network/keep-core/actions/runs/4969527266/jobs/8892744375:
Solutions
There are several approaches we could take to tackle this:
node_modulesstructure)keep-corecontracts that would be the exact copies of the current ones that are in use, with the only difference being the name of the TokenStaking contract (it could be renamed to KeepTokenStaking). This way we would no longer have a name conflict and could get rid of the prepare-dependencies.sh script.TokenStakingorKeepTokenaddresses are not going to change. This may be even quicker time-wise than figuring out what is happening with environment variables. And we'll make build times shorter. Double win.The text was updated successfully, but these errors were encountered: