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

Default deployment #791

Open
SebastienGllmt opened this issue Aug 5, 2024 · 8 comments
Open

Default deployment #791

SebastienGllmt opened this issue Aug 5, 2024 · 8 comments
Labels
status:needs-decision We need to make a decision about this

Comments

@SebastienGllmt
Copy link
Contributor

SebastienGllmt commented Aug 5, 2024

Describe the feature

Right now, when testing locally, I always have to do

npx hardhat node
npx hardhat ignition deploy ./ignition/modules/... --network localhost

it's annoying to have to open two terminals and do an extra command just for this, and it's easy to accidentally forget to run the deploy script (esp. for new contributors to our project trying to setup their env for the first time)

Ideally, there would be a way in my hardhat config to specify a default hardhat ignition deployment (or maybe multiple deployments if there is a base deployment + mock data) to run when using the localhost network so that people don't have to type this manually

This is somewhat philosophically similar to #752

Search terms

deploy, node, default

@kanej kanej added status:needs-decision We need to make a decision about this and removed status:triaging labels Aug 6, 2024
@kanej
Copy link
Member

kanej commented Aug 6, 2024

I like the idea of starting a local Hardhat node already setup with an Ignition module.
This would need some design work around how it composes with the Hardhat accounts system (we are looking at reworking this in the next major Hardhat upgrade).

@fvictorio
Copy link
Member

As a temporary workaround, you can do something like this:

const { subtask } = require("hardhat/config")
const { TASK_NODE_SERVER_READY } = require("hardhat/builtin-tasks/task-names");
const path = require("path")

subtask(TASK_NODE_SERVER_READY, async (_, hre, runSuper) => {
  const result = await runSuper();

  await hre.run({
    scope: "ignition",
    task: "deploy"
  }, {
    modulePath: path.resolve(__dirname, "ignition", "modules", "Foo.js")
  })

  return result;
});

This will only work for the hh node task, so you won't get deployments for, say, hh console, but it might help until we have a better solution.

@SebastienGllmt
Copy link
Contributor Author

SebastienGllmt commented Sep 15, 2024

I tried the workaround suggested, and the issues I have is that calling this task directly does not generate a deployments folder, because it uses EphemeralDeploymentLoader instead of FileDeploymentLoader

This happens because hardhat node can only run on the hardhat network, whereas when you use hardhat ignition deploy you can actually specify localhost as the network (which makes it write to disk) if needed

This isn't good for my use-case because I need to get the addresses of the deployed contracts, but _deployedAddresses is not exposed anywhere in the EphemeralDeploymentLoader case (ideally I would either have a file on-disk to read, or a deployment-agnostic way to get the list of addresses deployed so I can write a tool that processes these files regardless of it this is a localhost deployment or a mainnet deployment)

@fvictorio
Copy link
Member

Ah, interesting!

@kanej is there an option to "force" ignition to write deployments to disk, even if the chain is the in-process network?

@kanej
Copy link
Member

kanej commented Sep 17, 2024

We don't have a way of forcing Ignition to ignore that it is running against the Hardhat node.
I had thought that passing a deploymentId might be enough, but our logic for determining whether to write to the deployments folder is based on the network name.

Adding an override is an option, but I suspect that @SebastienGllmt is more interested in getting the list of contracts + deployed addresses even when running against an in-memory node.

Currently the task invocation doesn't return any response, that could be changed to be something more useful like the deployed addresses.

The viem/ethers packages enhance the hre with an ignition object with a deploy method. That returns contract instances that could be used to get addresses ... but that will only show for the contracts explicitly returned from the module.

subtask(TASK_NODE_SERVER_READY, async (_: any, hre: any, runSuper: any) => {
  const result = await runSuper();

  const { default: myModule } = require(path.resolve(
    __dirname,
    "ignition",
    "modules",
    "Lock.ts"
  ));

  const deployResult = await hre.ignition.deploy(myModule, {});

  for (const futureId of Object.keys(deployResult)) {
    console.log(futureId, await deployResult[futureId].getAddress());
  }

  return result;
});

Now the underlying deployed addresses information is available, we just choose to narrow the returned information to contract instances returned from the module.

Some options here would be:

  1. Provide extra info in the result of hre.ignition.deploy(...) for both viem/ethers, maybe some variant of {_meta: {deployedAddresses: ...}}
  2. Replace our current default hre.ignition.deploy(...) which throws suggesting to install viem/ethersand instead have it do deploy and return information a tool developer would find useful including the complete list of deployed contracts and addresses. The downside here being confusion from standard user trying to use Ignition withoutviem/ethers`

@SebastienGllmt
Copy link
Contributor Author

Trying to leverage the result of deploy sounds a but tricky because I care about all contracts deployed historically (not just any new contracts deployed). In another project I also had a case where I had to add some externally deployed stuff to the deployed address list as well, so I feel just reading the file feels like the most foolproof way

I can easily get the file content through the deployments list task you folks added for me previously (thanks!), so some option to firce-write to disk sounds fine to me. Although I'm not at my computer to double-check, my understanding is that Hardhat itself also still writes compiler outputs to disk even if running in in-memory mode

If you don't want this (maybe some other plugins keep everything in memory for hardhat network and you don't want to deviate), a task to get all the deployed addresses for a specific deployment ID also sounds reasonable

@fvictorio
Copy link
Member

To me the idea that an explicit deployment id overrides the logic of "is this the in-process network? then don't write to disk" makes a lot of sense. This, combined with what @SebastienGllmt mentioned:

a task to get all the deployed addresses for a specific deployment ID also sounds reasonable

seems like an interesting solution that doesn't affect at all simpler use cases.

@SebastienGllmt
Copy link
Contributor Author

SebastienGllmt commented Sep 20, 2024

a task to get all the deployed addresses for a specific deployment ID also sounds reasonable

After thinking about this more, maybe this is a bit tricky for the in-memory case because

  1. If you don't have an in-memory instance running when the command is run, you have to run deploy internally to get the address list (since there is no cache on disk for you to read from)
  2. If you do have an in-memory instance running, you would need some kind of IPC to get the address list from the other process

On a related note, I don't think there is a good event like TASK_NODE_SERVER_READY we can listen to if we want to know if the deployment step has completed successfully (to access the deployment result after the deployment is done from a subtask)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs-decision We need to make a decision about this
Projects
Status: No status
Development

No branches or pull requests

3 participants