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

Update dependencies to threshold-network and keep-network projects #212

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

michalinacienciala
Copy link
Contributor

There was a couple of changes in the project dependencies that requuired
change of the configuration of package.json and contracts.yml:

  1. coverage-pools became dependent on
    @threshold-network/solidity-contracts package. This was already
    reflected in the package.json, but not in the CI.
  2. Version of the @threshold-network/solidity-contracts got bumped up
    to 1.2.0-dev.x, which needed to be reflected in package.json
  3. We started to prepare our projects for deployment on the goerli
    testnet. This means that we will (or already did) publish packages
    with the -goerli suffix. In order to use the -dev suffixed
    packages in the NPM workflow we need to update the semver ranges so
    that the versions could be resolved to the desired packages.

I hope that those changes will help with the problem with deployment of coverage-pools on ropsten (https://github.com/keep-network/coverage-pools/runs/7299602405?check_suite_focus=true).

There was a couple of changes in the project dependencies that requuired
change of the configuration of `package.json` and `contracts.yml`:
1. `coverage-pools` became dependent on
   `@threshold-network/solidity-contracts` package. This was already
   reflected in the `package.json`, but not in the CI.
2. Version of the `@threshold-network/solidity-contracts` got bumped up
   to `1.2.0-dev.x`, which needed to be reflected in `package.json`
3. We started to prepare our projects for deployment on the `goerli`
   testnet. This means that we will (or already did) publish packages
   with the `-goerli` suffix. In order to use the `-dev` suffixed
   packages in the `NPM` workflow we need to update the semver ranges so
   that the versions could be resolved to the desired packages.
@michalinacienciala
Copy link
Contributor Author

michalinacienciala commented Jul 12, 2022

In https://github.com/keep-network/coverage-pools/actions/runs/2655950601 I'm verifying if the changes help for the ropsten deployment problem

@michalinacienciala
Copy link
Contributor Author

In https://github.com/keep-network/coverage-pools/actions/runs/2655950601 I'm verifying if the changes help for the ropsten deployment problem

Unfortunately, now builds fails with INSUFFICIENT_FUNDS during execution of 04_deploy_underwriter_token.ts... I'm trying to figure out the root cause...

@michalinacienciala
Copy link
Contributor Author

Unfortunately, now builds fails with INSUFFICIENT_FUNDS during execution of 04_deploy_underwriter_token.ts... I'm trying to figure out the root cause...

Deployer account needed a top-up. Verifying again after sending some ETH: https://github.com/keep-network/coverage-pools/actions/runs/2655950601.

@michalinacienciala michalinacienciala force-pushed the ci-update-dependencies branch 3 times, most recently from c4928e6 to 7bbb6f6 Compare July 12, 2022 15:12
@michalinacienciala
Copy link
Contributor Author

Deployer account needed a top-up. Verifying again after sending some ETH: https://github.com/keep-network/coverage-pools/actions/runs/2655950601.

Back to UNPREDICTABLE_GAS_LIMIT error during execution of 05_deploy_asset_pool.ts. Looks that this error appeared after introduction of this piece of code in #206:

initGovernance(_collateralToken);
}
/// @dev Overwrite to empty if collateral token used by the AssetPool
/// does not support DAO checkpoints. Used for tests with KEEP token.
function initGovernance(ICollateralToken _collateralToken)
internal
virtual
{
_collateralToken.delegate(address(this));
}

package.json Outdated
"@thesis/solidity-contracts": "github:thesis/solidity-contracts#4985bcf",
"@threshold-network/solidity-contracts": ">1.1.0-dev <1.1.0-ropsten",
"@threshold-network/solidity-contracts": ">1.2.0-dev <1.2.0-goerli",
Copy link
Member

Choose a reason for hiding this comment

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

Could we use development tag?

Suggested change
"@threshold-network/solidity-contracts": ">1.2.0-dev <1.2.0-goerli",
"@threshold-network/solidity-contracts": "development",

Same for other dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now tag the `@threshold-network/solidity-contracts` package
containing contracts meant for develoment with `development` tag. We can
use it in the `package.json` to refer to the latest package meant for
development.
We're adding a step that configures git to use `https://` protocol
instead of `git://` when downloading files.
We need this step because the `@keep-network/tbtc` which we update in next
steps has a dependency to `@summa-tx/[email protected]` package, which
downloads one of its sub-dependencies via unathenticated `git://`
protocol. That protocol is no longer supported. See
https://github.blog/2021-09-01-improving-git-protocol-security-github/.
We also manually change the unauthenticated git protocol used for this
sub-dependency in the `package.lock`.
@@ -87,12 +114,14 @@ jobs:
query: |
keep-core-contracts-version = github.com/keep-network/keep-core/solidity-v1#version
tbtc-contracts-version = github.com/keep-network/tbtc/solidity#version
solidity-contracts-version = github.com/threshold-network/solidity-contracts#version
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
solidity-contracts-version = github.com/threshold-network/solidity-contracts#version
threshold-contracts-version = github.com/threshold-network/solidity-contracts#version


- name: Resolve latest contracts
run: |
yarn upgrade \
@keep-network/keep-core@${{ steps.upstream-builds-query.outputs.keep-core-contracts-version }} \
@keep-network/tbtc@${{ steps.upstream-builds-query.outputs.tbtc-contracts-version }}
@keep-network/tbtc@${{ steps.upstream-builds-query.outputs.tbtc-contracts-version }} \
@threshold-network/solidity-contracts@${{ steps.upstream-builds-query.outputs.solidity-contracts-version }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@threshold-network/solidity-contracts@${{ steps.upstream-builds-query.outputs.solidity-contracts-version }}
@threshold-network/solidity-contracts@${{ steps.upstream-builds-query.outputs.threshold-contracts-version }}

Comment on lines +38 to +39
"@keep-network/keep-core": ">1.8.0-dev <1.8.0-goerli",
"@keep-network/tbtc": ">1.1.2-dev <1.1.2-goerli",
Copy link
Member

Choose a reason for hiding this comment

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

Could we use the development tag here as well?

Suggested change
"@keep-network/keep-core": ">1.8.0-dev <1.8.0-goerli",
"@keep-network/tbtc": ">1.1.2-dev <1.1.2-goerli",
"@keep-network/keep-core": "development",
"@keep-network/tbtc": "development",

- name: Configure git to don't use unauthenticated protocol
shell: bash
run: git config --global url."https://".insteadOf git://

- name: Resolve latest contracts
run: |
yarn upgrade \
Copy link
Member

Choose a reason for hiding this comment

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

Let's pin to exact dependencies versions. (see keep-network/keep-core#3083 for explaination)

Suggested change
yarn upgrade \
yarn upgrade --exact \

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