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

Add support for deployment on Goerli testnet #215

Draft
wants to merge 1 commit into
base: goerli-deploy
Choose a base branch
from

Conversation

michalinacienciala
Copy link
Contributor

@michalinacienciala michalinacienciala commented Aug 4, 2022

Görli became a recommended test network after Ropsten's deprecation
notice (https://blog.ethereum.org/2022/06/21/testnet-deprecation/).
We're modifying GitHub Actions workflow for deploying coverage-pools
contracts to support the deployment on Görli.

TODO:

  • Set up the access to the GOERLI_ETH_HOSTNAME_HTTP and GOERLI_ETH_CONTRACT_OWNER_PRIVATE_KEY secrets for coverage-pools repo
  • Verify if the deployment and workflow behave correctly (so far the deployment fails, see my comment in #212).
  • Remove testing configuration from the workflow

Görli became a recommended test network after Ropsten's deprecation
notice (https://blog.ethereum.org/2022/06/21/testnet-deprecation/).
We're modifying GitHub Actions workflow for deploying `coverage-pools`
contracts to support the deployment on Görli.

NOTE: We're temporarily using some testing configuration in the
workflow, which needs to be removed before merge to `main`.
@michalinacienciala
Copy link
Contributor Author

michalinacienciala commented Aug 4, 2022

I run the deployment on goerli and it failed (twice in a row) with the following error:

Error: ERROR processing /home/runner/work/coverage-pools/coverage-pools/deploy/05_deploy_asset_pool.ts:
Error: cannot estimate gas; transaction may fail or may require manual gas limit [ See: https://links.ethers.org/v5-errors-UNPREDICTABLE_GAS_LIMIT ] (reason="execution reverted", method="estimateGas", transaction={"from":"0x68ad60CC5e8f3B7cC53beaB321cf0e6036962dBc","data":"0x60e0604052621baf806002556202a

Workflow run: https://github.com/keep-network/coverage-pools/runs/7670017006?check_suite_focus=true.
Not sure what's causing it. The deployer account should be in line with the one used to create keep-core and tbtc packages...

[EDIT]: Verified again, on a ci-test branch that combines ci-goerli and ci-update-dependencies (from PR #212). Still fails with the same error. Run: https://github.com/keep-network/coverage-pools/runs/7863518824?check_suite_focus=true.

@nkuba
Copy link
Member

nkuba commented Aug 17, 2022

I run the deployment on goerli and it failed (twice in a row) with the following error:

Error: ERROR processing /home/runner/work/coverage-pools/coverage-pools/deploy/05_deploy_asset_pool.ts:
Error: cannot estimate gas; transaction may fail or may require manual gas limit [ See: https://links.ethers.org/v5-errors-UNPREDICTABLE_GAS_LIMIT ] (reason="execution reverted", method="estimateGas", transaction={"from":"0x68ad60CC5e8f3B7cC53beaB321cf0e6036962dBc","data":"0x60e0604052621baf806002556202a

Workflow run: https://github.com/keep-network/coverage-pools/runs/7670017006?check_suite_focus=true. Not sure what's causing it. The deployer account should be in line with the one used to create keep-core and tbtc packages...

[EDIT]: Verified again, on a ci-test branch that combines ci-goerli and ci-update-dependencies (from PR #212). Still fails with the same error. Run: https://github.com/keep-network/coverage-pools/runs/7863518824?check_suite_focus=true.

We use the V1's KeepToken deployed to goerli.
We pass the KeepToken in the constructor to the AssetPool contract:

args: [KeepToken.address, UnderwriterToken.address, rewardManager],

The first argument is expected to implement ICollateralToken interface:
ICollateralToken _collateralToken,

Unfortunatelly the KeepToken from V1, doesn't implement the ICollateralToken interface.

I'm not sure what are the plans regarding the latest AssetPool implementation and deployment arguments. @pdyraga when you find a second could you please clarify?

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