-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: tool interfaces aka fasad #10
Conversation
}, | ||
multichain: { | ||
environment: Environment.TESTNET, | ||
deploymentNetworks: ["sepolia", "optimisticGoerli"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it would be better to have something like:
networks: {
sepolia: { ... },
goerli: { ... },
mumbai: { ... },
mainnet: { ... },
polygon: { ... },
optimism: { ... },
},
multichain: {
networkConfig: {
goerli: {
deploymentTargets: ["sepolia", "Mumbai"]
},
optimism: {
deploymentTargets: ["mainnet", "polygon"]
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is already network
config from hardhat do not see the point of doing a double thing...
https://hardhat.org/tutorial/deploying-to-a-live-network#deploying-to-remote-networks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, "networkConfig" should be inside "multichain"
|
||
public constructor(private readonly hre: HardhatRuntimeEnvironment) { | ||
this.initializationPromise = new Promise<void>((resolve, reject) => { | ||
void this.initSygma(resolve, reject); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you have to do that in constructor?
Can't you have async function getConfig
which checks if there is cached config and if there is not, inits sygma ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validation: validate (using sygma SDK) that all routes exist between those networks
in the task list is to validate with Sygma if routes exist. It could not be done while initializing the config, so moved into the constructor so can be done async over there. It is not the most elegant approach, but is kinda one of the possible approaches that will reduce wait time for validation.
packages/plugin/README.md
Outdated
Example scenario: You've created an ERC20 contract and want to deploy it across multiple chains using the configuration mentioned above. | ||
```typescript | ||
// Wait for deployment initialization | ||
await hre.multichain.waitInitialization(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit hacky? Why do we need this since "deployMultichain" is already async?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed removing this before the break^^
|
||
public async deployMultichain( | ||
nameOrBytecode: string, | ||
args: string[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
args: string[], | |
contructorArgs: string[], |
and use type for args from Web3.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even better, use web3.js types to ensure typesafety of constructor args
await this.initializationPromise; | ||
} | ||
|
||
public async deployMultichain( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public async deployMultichain( | |
public async deployMultichain<ABI: Abi = any>( |
public async deployMultichain( | ||
nameOrBytecode: string, | ||
args: string[], | ||
options?: Object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe define object. You will need to use abi generic.
Since constructor args needs to remain the same for contract to get same address, usually you would invoke, a separate init method in the same tx.
So they would need to pass initi method name (use web3js typings to allow only methods define in Abi) and arguments for that method (again, you can use web3.js typings to ensure type safety.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not go deep into types for it as probably be less productive thinking about all possible types that create them when will are implemented
🤖 I have created a release *beep* *boop* --- <details><summary>hardhat-plugin-multichain-deploy: 1.0.0</summary> ## 1.0.0 (2024-02-16) ### Features * adapter address env ([#25](#25)) ([628a99c](628a99c)) * deploy multichain bytecode ([#22](#22)) ([2eff1ee](2eff1ee)) * encode constructor improve ([#24](#24)) ([9c4b7b1](9c4b7b1)) * encode function and deployMultichain flow ([#14](#14)) ([26229cf](26229cf)) * encodeInitData ([#30](#30)) ([50758ca](50758ca)) * example ([#49](#49)) ([637d0ae](637d0ae)) * initLocalEnvironment ([#47](#47)) ([e728a10](e728a10)) * rework networks input ([#23](#23)) ([3caf657](3caf657)) * tool interfaces aka fasad ([#10](#10)) ([6351819](6351819)) * transaction details and deployment info ([#33](#33)) ([2b3232d](2b3232d)) ### Bug Fixes * deployment network type and naming ([#26](#26)) ([1060069](1060069)) * small logical mistakes ([#48](#48)) ([6cfd609](6cfd609)) </details> <details><summary>hardhat-plugin-multichain-deploy-contracts: 1.0.0</summary> ## 1.0.0 (2024-02-16) ### Features * add contracts implementation ([#11](#11)) ([114c05f](114c05f)) * example ([#49](#49)) ([637d0ae](637d0ae)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This is a draft to create conversations based on some things
Resolves #4