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

chore: imporved deployement script #220

Closed
wants to merge 12 commits into from
Closed

chore: imporved deployement script #220

wants to merge 12 commits into from

Conversation

ikemHood
Copy link
Contributor

@ikemHood ikemHood commented Jul 29, 2024

Improve deploy function on deploy-contract.ts

Fixes #217

utilized maxFee
removed unused imports
imporved logging
tested on devnet and sepolia

  • Feature
  • Bug
  • [✅ ] Enhancement

Copy link
Collaborator

Choose a reason for hiding this comment

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

we cant commit this file. @ikemHood

Copy link
Collaborator

Choose a reason for hiding this comment

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

i should be able to call this

  await deployContract(
    "YourContract",
    "YourContractExportName",
    {
      owner: deployer.address
    },
    {
      maxFee: BigInt(1000000000000)
    }
  );

also this ( THIS IS NOT POSSIBLE YET , IT ENFORCES A EXPORT NAME )

  await deployContract(
    "YourContract",
    {
      owner: deployer.address,
    }

and also this for a contract that doesnt receive a constructor

  await deployContract(
    "YourContract",
  );

Copy link
Contributor Author

@ikemHood ikemHood Jul 30, 2024

Choose a reason for hiding this comment

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

This works?

  await deployContract(
    {
      contractName: "YourContract",
      exportContractName: "YourContractExportName",
      constructorArgs: {
        owner: deployer.address,
      },
      options:{
        maxFee: BigInt(1000000000000)
      }
    }
  );

Copy link
Collaborator

@0xquantum3labs 0xquantum3labs Jul 30, 2024

Choose a reason for hiding this comment

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

this also works, lets do contractName -> contract & exportContractName -> contractName. ensure correct typing. and should be able to just pass

  await deployContract(
    {
      contract: "YourContract",
    }
  );

and should work @ikemHood

Copy link
Collaborator

Choose a reason for hiding this comment

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

lets add some instructions as comments on how to use this function with example within this deployScript() function,

let deployCalls = [];

const { provider, deployer }: Network = networks[networkName];

const declareIfNot_NotWait = async (payload: any) => {
const declareIfNot_NotWait = async (payload: DeclareContractPayload, options?: { maxFee?: bigint}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is a maxDeclareFee rather than a maxFee at the moment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can add another optional maxFee here

const executeDeployCalls = async () => {

and this maxFee will be set up once inside deploy.ts and passed to this function when its called, should be nullable item and add instructions on deploy.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

executeDeployCalls usage doesn't make it straight forward to add this params, but i have added it regardless.

another option can be to set the options from deployContract to a variable just like the deployCalls and access it inside the function. Let me know if i should implement this

@0xquantum3labs
Copy link
Collaborator

this PR shoudnt change anything on packages/nextjs please check the changed files.

.gitignore Outdated

# bun
bun.lockb
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we have this ? @ikemHood

@0xquantum3labs
Copy link
Collaborator

this PR shoudnt change anything on packages/nextjs please check the changed files.
i still see changes on packages/nextjs

@0xquantum3labs
Copy link
Collaborator

also please check failing CI

@Nadai2010
Copy link
Collaborator

Hi @ikemHood , you must add the removed files, even if they are auto-generated, you cannot remove anything from packages/nextjs, keep it the same as the main, it has already been left in the comments.

Please also review the rest of the comments, the tests continue to fail, once this is corrected, upload it with the correct CI

@0xquantum3labs
Copy link
Collaborator

closing this as changes are still visible on packages/nextjs, without updates .

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.

Improve deploy function on deploy-contract.ts
3 participants