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

Deploy Protocol Contracts on LightLink Mainnet and Testnet #1924

Merged
merged 11 commits into from
Jan 13, 2024

Conversation

vanshwassan
Copy link
Contributor

Copy link
Contributor

@Ashar2shahid Ashar2shahid left a comment

Choose a reason for hiding this comment

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

Looks good to me, Lighlink recently reached out to integrate QRNG and we will have the providers redeploy to support the chain. We cannot merge this until @api3dao/chains issue is resolved.

api3dao/chains#137 .

@vanshwassan vanshwassan marked this pull request as ready for review January 6, 2024 14:31
Copy link
Contributor

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

Can you address the references.json merge conflict? The file / export changed recently in #1929. It may only need merging master then rerunning of:

"deploy:generate-references": "hardhat run scripts/generate-references.ts",

I'm still confused on the path forward with api3dao/chains#137. Should that be reopened and merged, followed by a 4.3.0 release (4.2.0 is already released)?

@Ashar2shahid
Copy link
Contributor

I'm still confused on the path forward with api3dao/chains#137. Should that be reopened and merged, followed by a 4.3.0 release (4.2.0 is already released)?

Currently the chains CI runs on every chain in the package, there needs to be a flag to disable CI operation on a chain (for more fringe chains)

@bbenligiray
Copy link
Member

These deployments shouldn't be pushed here unless the same chain is already published in @api3/chains. Non-dAPI related chain additions to @api3/chains will create overhead for the dAPI operations. I thought you were going to do this completely off-repo.

@Ashar2shahid
Copy link
Contributor

Ashar2shahid commented Jan 9, 2024

I think RRP deployments should live in this repo for completeness sake, the dependency on @api3/chains should be figured out IMO rather than have RRP deployments live in some other repo. Alternatively, we can have another repo "qrng-deployments" where we do all the rrp deployments if we do not want to support this.

@dcroote
Copy link
Contributor

dcroote commented Jan 10, 2024

Non-dAPI related chain additions to @api3/chains will create overhead for the dAPI operations

I thought api3/chains was the single source of truth for all chains and I lean this way:

@api3/chains should be figured out IMO rather than have RRP deployments live in some other repo

Otherwise, we then have another source of chains and chain IDs to deal with. What could work is more streamlined or automated api3/chains release process to minimize the effort involved in adding a chain.

@andreogle
Copy link
Member

andreogle commented Jan 10, 2024

I generally don't really follow what's going on with new chain integrations, but I'm happy to help streamline any processes with @api3/chains to make it easy to work with. I tried to make the repo as developer friendly as possible to encourage contributions/PRs. The guard rails (linting checks) are also fairly strict to prevent anything from going wrong. Adding a new chain should be a fairly straightforward exercise at this point. I'm also happy to do same day releases assuming I'm available (otherwise someone else can help with this or we can automate it).

I assume this comment is the problematic one? I don't have much capacity at the moment to get too involved here, but it should be fairly straightforward to implement a new field that skips the chain in the CI. Happy to accept PRs if it's a blocker

@dcroote
Copy link
Contributor

dcroote commented Jan 10, 2024

@andreogle - I find api3/chains quite dev friendly- didn't mean otherwise and apologies if it came off sounding that way. I created api3dao/chains#153 and assigned myself to work on now so this can unblock folks.

@andreogle
Copy link
Member

didn't mean otherwise and apologies if it came off sounding that way

I didn't take it that way at all 😄 Thanks for implementing the PR to skip provider checks 👍🏻 (cc @vanshwassan).

I also just meant that I'm happy to try to help make the repo and/or process as streamlined as possible - whatever that might look like. I'm not a primary user of it so I don't always understand where the issues/blockers are.

@dcroote
Copy link
Contributor

dcroote commented Jan 11, 2024

@vanshwassan - the PR for skipping a chain in CI is merged so now api3dao/chains#137 can be reopened, the new field added, merged, and api3/chains released. Then this PR, updated with the new api3/chains version, can be merged 😅

@vanshwassan
Copy link
Contributor Author

The @api3/chains PR is merged and v4.3.0 is released. Updated the branch and package version.

Copy link
Contributor

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

I think the CI failures are because the PR is from your fork rather than from a branch within the repo and therefore lack the secrets necessary e.g. for pushing the docker containers. Next time can you use a branch? Otherwise LGTM

Copy link
Contributor

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

Actually I see the yarn.lock file hasn't updated- can you run yarn run bootstrap followed by yarn run build?

Copy link
Contributor

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

Nice, and thanks for the patience on the small details

@dcroote dcroote merged commit 8afaf34 into api3dao:master Jan 13, 2024
17 of 19 checks passed
@dcroote dcroote added this to the 0.14.0 milestone Jan 13, 2024
Ashar2shahid pushed a commit that referenced this pull request Jan 18, 2024
* Deployed protocol contracts on lightlink-goerli

* Deployed protocol contracts on lightlink

* `@api3/chains` version bump
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.

5 participants