Remove OpenZeppelin CLI dep about monorepo HOT 20 CLOSED

Is there a way to deploy the factory with buidler and get the deployed instance in a similar way to truffle's MyContract.deployed() method? This is the other justification for OZCLI currently. I guess we could use truffle migrations w/ buidler, but would rather not depend on truffle at all, and if we're going to add an additional dependency then would prefer OZCLI to truffle. Will test if this is still an issue after migrating to the new boilerplate.

from monorepo.

In the process of deploying you get the address as part of what’s returned. And with ethers.js you just need the ABI and the address to create an instance of the contract.

from monorepo.

Right, AFAIK you can't access that deployed contract or its address outside of the deployment. Could easily write the address to a file though that could be accessed after, say in the frontend or tests.

from monorepo.

And actually, I don't know why I'm saying tests because the tests will always deploy on demand, providing access to the deployed contract. This is definitely just a frontend concern for loading the current deployed factory.

from monorepo.

Sorry, I should've closed this with a comment! All of the OpenZeppelin CLI deployment stuff was torn out in the monorepo migration in favor of deploying with Buidler: https://github.com/clrfund/monorepo/blob/master/contracts/package.json#L9

from monorepo.

To touch on this, I remember we had a dependency conflict and that's why we removed this at the hackathon, but on a different note I'm wondering if we want upgradeable contracts at all, or if that compromises the trustlessness of the system.

On the flip side, I will say that not having upgradeable contracts is a HUGE pain when you find a bug and there's already funds locked in the system, but this can potentially be handled by an alternative migration of sorts. Thoughts?

from monorepo.

I think the main reason we removed it was because oz compile wasn't working.

Good question re: upgradeable contracts. I've used the oz tooling for that before and liked it. I think we can make that call closer to deploy time. Because once you make it upgradeable, you're committed to freezing the order of variables, etc. But taking something that wasn't upgradeable and switching to having it be upgradeable later is pretty simple, as long as you haven't deployed it to mainnet yet.

from monorepo.

I think for this use case, there will be one main front end that we can always point at different contracts, and there's a new contract for each round anyway, so there's a natural upgrade path even with non-upgradeable contracts. So given that there's still a way to "upgrade" if we really need to, I think going trust-minimized here is the way to go. But for other use cases, I love upgradeable contracts!

from monorepo.

I think the main reason we removed it was because oz compile wasn't working.

Discussion about this on the OpenZeppelin forums https://forum.openzeppelin.com/t/compiling-contracts-with-solc-0-6-2-commit-bacdbe57-fails-with-spawnsync-bin-sh-enobufs

from monorepo.

Totally, hard coding the address works fine enough. Updating it becomes tedious during development though which is why having a dynamic .deployed()-esque method is preferable (I am admittedly somewhat spoiled by truffle and OZ in this regard).

from monorepo.

Hmm, what I was describing wasn’t hard-coding

from monorepo.

I think what I described will achieve what you’re looking for

from monorepo.

Oh cool! I think you showed me some code doing this in fact. Do you have a code snippet or link you can point me to regarding how to get the deployed address in the tests and frontend?

from monorepo.

This isn’t in the tests or the front end, but it shows an example of getting the address of the contract when deploying it https://github.com/chicago-ethereum/quadratic-funding/blob/master/contract-project/scripts/deploy.ts

from monorepo.

Yeah, the only dynamic thing we need access to in the tests is an instance of the contract that is deployed by the factory. But that's doable with the method I described above, where you use ethers.js with the ABI and the address returned by the factory deploy method to get an instance of the new contract.

As for the front end, a little script on load that loads the latest address from a local file that's .gitignored seems like a good solution.

from monorepo.

@proofoftom Related discussion here NomicFoundation/hardhat#381 (comment)

from monorepo.

I agree with the approach taken in the thread above where it gets added to the networks.json file as an address key under the network the factory is deployed to. This needs to be committed because we'll deploy locally and push the changes up, where the production front end will still need access to the deployed address.

from monorepo.

We'll want to commit testnet and mainnet addresses but not addresses on the local Buidler EVM chain (otherwise every PR will have a bigger diff than we want).

from monorepo.

I haven't been following this project closely lately, but I'd still be curious for context on why you closed this, @proofoftom. Thanks!

from monorepo.

@proofoftom Makes sense, thanks! Glad to hear you're going with Buidler!

from monorepo.