account V2 implementation first version + dev notes #255
Conversation
…ing + use scw package
livingrockrises
requested review from
talhamalik883,
ankurdubey521,
AmanRaj1608 and
tomarsachin2271
…entation to one based on Factory
| } | ||
|
|
||
| async buildUserOp( | ||
| transactions: Transaction[], |
There was a problem hiding this comment.
If it's more than one parameter, convert the method param into single object with multiple fields.
There was a problem hiding this comment.
wasn't this for more than 3?
I can make object input but V1 clients are used to not passing the object right now.
There was a problem hiding this comment.
hmm 3 is also fine, but looking at the last parameter it looks like more such flags can be introduced in the future.
| this.factoryAddress = biconomySmartAccountConfig.factoryAddress | ||
| // Review: if it's really needed to supply factory address | ||
| this.factoryAddress = | ||
| biconomySmartAccountConfig.factoryAddress ?? DEFAULT_BICONOMY_FACTORY_ADDRESS // This would be fetched from V2 |
There was a problem hiding this comment.
do we plan to call these by backend or we will be using the default ones in sdk from now on?
There was a problem hiding this comment.
currently I don't see value in fetching it from backend. so it's either provided in config, from constants using provided version or default from constant.
which part? |
Description
BiconomySmartAccountV2
which follows new BaseSmartAccount abstract class design and IBaseSmartAccount interface.
This account relies on validation module classes to perform signing related operations and retrieve information required specific to modules.
init is not specifically overridden (basic init exists in super class) - unlike BiconomyAccountV1
I also commented out init, initializeAtIndex and setupContracts etc methods because currently I don't see the need for them.
setInitCode doesnt need to be part of init to set local var called initcode.
generally you should be able to query getAccountAddress and _getAccountContract() to get account / proxy information (contract or address)
similarly this.factory would be initiased as part of this.getAccountAddress which only relies on the initCode.
currently nodeClient is part and accessible but it's not used to query any address config or account info. maybe once we have smartAccountAddress maybe we can query it and perform some init checks. but should not use the nodeclient method getSmartaccountByOwner() (basically any more or owner passing)
please review and check relevant method implementations in userOperation creation and dispatch lifecycle. some trace back to implementation written in abstract class.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: