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

Feature/functions integration #11

Merged

Conversation

koteld
Copy link
Contributor

@koteld koteld commented Oct 16, 2023

NPM package "@chainlink/functions-toolkit" integrated to the Hardhat-Chainlink plugin.
Its methods were wrapped and are available both as Hardhat tasks and as methods in HRE.
Thus, most of the work related to interaction with Chainlink Function and DONs, manipulating secrets, gists is delegated to the "@chainlink/functions-toolkit".
Ability to run Functions simulations was wrapped as well and is available in the Hardhat-Chainlink plugin.

@koteld koteld marked this pull request as draft October 16, 2023 16:54
Copy link

@zeuslawyer zeuslawyer left a comment

Choose a reason for hiding this comment

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

consider whether we should add npm-link
https://docs.npmjs.com/cli/v8/commands/npm-link

so that we can use the PR version of the code in a sample project to test ?

@koteld
Copy link
Contributor Author

koteld commented Oct 31, 2023

consider whether we should add npm-link https://docs.npmjs.com/cli/v8/commands/npm-link

so that we can use the PR version of the code in a sample project to test ?

In order to test that PR without publishing package to NPM you can:

  1. Run npm build -> it will produce files necessary for the package
  2. Run npm pack -> it will create a .tgz file of the package
  3. In the root folder of your Hardhat project run: npm install path/to/package.tgz
  4. Add require("@chainlink/hardhat-chainlink"); to thehardhat.config.jsin your Hardhat project
    That is it!

zeuslawyer
zeuslawyer previously approved these changes Dec 20, 2023
Copy link

@zeuslawyer zeuslawyer left a comment

Choose a reason for hiding this comment

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

LGTM - note some comments/feedback.

package.json Outdated Show resolved Hide resolved
src/sandbox/functionsSimulations/index.ts Outdated Show resolved Hide resolved
src/sandbox/functionsSimulations/index.ts Outdated Show resolved Hide resolved
src/shared/enums.ts Show resolved Hide resolved
@@ -0,0 +1,100 @@
import { BigNumberish } from "ethers";

Choose a reason for hiding this comment

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

Got it (i think!). There are two types of simulations in functions-toolkit:

  1. the local simulation which is purely a deno-runtime execution of the functions javascript (https://github.com/smartcontractkit/functions-toolkit?tab=readme-ov-file#local-functions-simulator)
  2. a testnet simulation which runs on the back of ganache, and is similar in concept to running a hardhat node, except it is with ganache and some contracts are deployed onto the testnet automatically. (https://github.com/smartcontractkit/functions-toolkit?tab=readme-ov-file#local-functions-testnet)

(1) is pure JS and no smart contracts are involved so there is no need for a provider.
(2) uses ganache and does care about the functions consumer in the sense that the consumer can be deployed to that simulated testnet.

You may have already fully grasped this but in case you didnt, please consider whether this impacts your assessment.

src/tasks/utils/index.ts Show resolved Hide resolved
@koteld koteld requested a review from zeuslawyer January 10, 2024 22:41
@koteld
Copy link
Contributor Author

koteld commented Jan 10, 2024

Hi @zeuslawyer,
After you approved my PR, I had to make several changes to the toolkit documentation and some small changes to the project structure, variables names, and usage of functions-toolkit. Consequently, merging is blocked again by the repository rules. Could you please review the PR once more?

Thank you.

src/HardhatChainlink.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link

@zeuslawyer zeuslawyer left a comment

Choose a reason for hiding this comment

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

Few comments to resolve please

Copy link

@zeuslawyer zeuslawyer left a comment

Choose a reason for hiding this comment

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

LGTM.

src/sandbox/functionsSimulations/index.ts Show resolved Hide resolved
@danielgruesso danielgruesso merged commit 5864424 into smartcontractkit:main Jan 16, 2024
1 check passed
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.

3 participants