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

feat: add support for overriding the initCode for an account #197

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

moldy530
Copy link
Collaborator

@moldy530 moldy530 commented Nov 1, 2023

Pull Request Checklist

  • Did you add new tests and confirm existing tests pass? (yarn test)
  • Did you update relevant docs? (docs are found in the site folder, see guidleines below)
  • Do your commits follow the Conventional Commits standard?
  • Does your PR title also follow the Conventional Commits standard?
  • If you have a breaking change, is it [correctly reflected in your commit message](https://www.conventionalcommits.org/en/v1.0.0/#examples? (e.g. feat!: breaking change)
  • Did you run lint (yarn lint:check) and fix any issues? (yarn lint:fix)
  • Is the base branch you're merging into development and not main?

PR-Codex overview

Detailed summary

  • Added support for optional override of the account address and init code in the createBaseSmartAccountParamsSchema function in packages/core/src/account/schema.ts.
  • Modified the SimpleSmartContractAccount constructor in packages/core/src/account/__tests__/simple.test.ts to use the account init code override.
  • Added the accountInitCode property to the BaseSmartContractAccount class in packages/core/src/account/base.ts to store the account init code.
  • Modified the getAccountInitCode method in the BaseSmartContractAccount class to return the account init code override if available, otherwise call the abstract getAccountInitCode method.
  • Added the _getAccountInitCode and create6492Signature methods to the BaseSmartContractAccount class to handle the account init code and signature creation logic.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@denniswon
Copy link
Contributor

@moldy530

q1: what was the context around needing this feature? This feature is for deployed accounts only or could be also used for non-deployed accounts?

q2: should we add some validations on when these optional parameters should be passed in so that devs can catch when incorrect set of parameters are passed in? (runtime validation using zod) cc: @avasisht23

Copy link
Contributor

@avasisht23 avasisht23 left a comment

Choose a reason for hiding this comment

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

few qs before the stamp

packages/core/src/account/base.ts Show resolved Hide resolved
packages/core/src/account/base.ts Show resolved Hide resolved
@moldy530
Copy link
Collaborator Author

moldy530 commented Nov 1, 2023

Q1: context around this is to support the case where a developer wants to snapshot the initCode that generated a given counterfactual. This is useful to protect against the case where the counterfactual might change before a user eventually sends a UO and deploys their account. with this, a dev can make sure that when a UO is finally sent, they deploy the account the user has been using the counterfactual of, even if the default factory address or account impl changes down the line

Q2: I've added validations to Zod around passing this in already here. However, beyond that we can't do much else to enforce that a dev SHOULD pass them. it's opt-in if they want to override these params

@avasisht23
Copy link
Contributor

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@avasisht23 avasisht23 merged commit 83b3813 into development Nov 3, 2023
3 checks passed
@avasisht23 avasisht23 deleted the moldy/initCodeOverride branch November 3, 2023 19:19
@moldy530 moldy530 mentioned this pull request Nov 10, 2023
@moldy530 moldy530 mentioned this pull request Nov 17, 2023
7 tasks
@moldy530 moldy530 mentioned this pull request Dec 1, 2023
8 tasks
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.

None yet

3 participants