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/versioning #188

Merged
merged 12 commits into from
Jun 17, 2023
Merged

Feat/versioning #188

merged 12 commits into from
Jun 17, 2023

Conversation

talhamalik883
Copy link
Contributor

@talhamalik883 talhamalik883 commented Jun 12, 2023

This PR includes following changes

  1. Integration of backend node api call for getting smart account address for EOA
  2. Enable versioning
  3. bug fix in sendUserOp function. it was calling its self instead of sendSignedUserOp

SmartAccount_v100__factory
getEntryPointContractInstance,
getFactoryContractInstance,
getProxyContractInstance
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename getProxyContractInstance to getSAProxyContract

We can remove Instance from all other names as well.

It should be getEntryPointContract, getSAFactoryContract, getSAProxyContract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

private address!: string
private smartAccountInfo!: ISmartAccount
private isInited!: boolean
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename isInited to isInitialized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so there is already a function with same name that i am using to verify either init is called or not. for this reason, i kept is variable name as isInited

  private isInitialized(): boolean{
    if (!this.isInited)
    throw new Error('BiconomySmartAccount is not initialized. Please call init() on BiconomySmartAccount before interacting with any other function')
    return true
  }

Copy link
Contributor

Choose a reason for hiding this comment

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

can just keep boolean called initialized

packages/account/src/BiconomySmartAccount.ts Outdated Show resolved Hide resolved
packages/account/src/BiconomySmartAccount.ts Show resolved Hide resolved
packages/account/src/BaseAccount.ts Outdated Show resolved Hide resolved
packages/account/src/BiconomySmartAccount.ts Show resolved Hide resolved
userOp = await this.estimateUserOpGas(userOp, overrides)
Logger.log('userOp after estimation ', userOp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

These Logger.log across all files should be called based on a debug flag in class constructor.... SDK should not print any logs on its own unless developer has enabled it using debug flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so logger only logs if debug is enabled. this check is already there in log function

Copy link
Collaborator

Choose a reason for hiding this comment

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

From where is the Logger class getting the debug flag?

packages/account/src/utils/Constants.ts Outdated Show resolved Hide resolved
packages/core-types/src/SmartAccountTypes.ts Outdated Show resolved Hide resolved
@livingrockrises
Copy link
Contributor

Is this versioning specific to entry point?
In PR description it should be clear outcomes about the change

@talhamalik883
Copy link
Contributor Author

we've got versioning all set up for the entrypoint, factory, and implementation. We're using this backend node API called getSmartAccountByOwner to grab all the info about a smart account. This includes the factory address, implementation address, and entry point address that are all linked to the smart account.

Based on the addresses we get from that API call, we're going to decide which version of typechain to use.
We can interact with the contract using the specific version of typechain that matches with those addresses.

here is how we have managed versions locally

`
export const ENTRYPOINT_ADDRESSES: EntrypointAddresses = {
'0x27a4db290b89ae3373ce4313cbeae72112ae7da9': 'V0_0_5',
'0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789': 'V0_0_6'
}
export const BICONOMY_FACTORY_ADDRESSES: BiconomyFactories = {
'0x000000f9ee1842bb72f6bbdd75e6d3d4e3e9594c': 'V1_0_0'
}
export const BICONOMY_IMPLEMENTATION_ADDRESSES: BiconomyImplementation = {
'0x00006b7e42e01957da540dc6a8f7c30c4d816af5': 'V1_0_0'
}

`

@talhamalik883 talhamalik883 merged commit a577660 into sdk_modularity Jun 17, 2023
@livingrockrises livingrockrises deleted the feat/versioning branch June 17, 2023 12:31
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.

3 participants