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

Typegen'd contract factories don't properly integrate storage slots for deployment methods #3355

Open
nedsalk opened this issue Oct 23, 2024 · 1 comment · May be fixed by #3396
Open

Typegen'd contract factories don't properly integrate storage slots for deployment methods #3355

nedsalk opened this issue Oct 23, 2024 · 1 comment · May be fixed by #3396
Assignees
Labels
bug Issue is a bug

Comments

@nedsalk
Copy link
Contributor

nedsalk commented Oct 23, 2024

Given a typegen'd contract factory MyContractFactory that has some storage slots, the following works and includes the storage slots:

const factory = new MyContractFactory(wallet);
await factory.deploy();

However, the two examples below don't integrate the storage slots:

await factory.deployAsCreateTx();
await factory.deployAsBlobTx();

This is because the storage slots in the typegen outputs aren't properly integrated for these methods.

There are two possible solutions:

  1. Override the two methods in the typegen'd contract factory the same way we do it with the deploy method,
  2. Add a property on ContractFactory that can be overriden in the typegen'd factory and which would be read in all the deployment methods and merged with the deployOptions passed via arguments.

The second option looks better because it encapsulates the logic in ContractFactory, thereby having the typegen'd factory do as little runtime logic as possible. A possible implementation showcasing the core components is shown below.

// contract-factory.ts
export default class ContractFactory {
  // should it be protected or public?
  protected _deployOptions: DeployContractOptions = { };
  private getDeployOptions(deployOptions) { return mergeDeepRight(this._deployOptions, deployOptions); }

  // same pattern for all other public methods that have deployOptions as a parameter
  async deploy(deployOptions) {
    const options = this.getDeployOptions(deployOptions);
  }
}

// typegen/contracts/MyContractFactory.ts
export class MyContractFactory extends ContractFactory {
  protected _deployOptions: DeployContractOptions = {
    storageSlots: MyContract.storageSlots
  }
}
@nedsalk nedsalk added the bug Issue is a bug label Oct 23, 2024
@arboleya
Copy link
Member

@nedsalk Definitely option 1.

@nedsalk nedsalk self-assigned this Nov 13, 2024
@nedsalk nedsalk linked a pull request Nov 14, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants