refactor: remove incorrect address assignment in Web3Wallet initialization#520
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Now that
this.addressis no longer set in the KMS and mnemonic paths, verify whether the class still needs a separateaddressproperty at all and either remove it entirely or makeaddresses[0]the single source of truth to avoid inconsistent state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `this.address` is no longer set in the KMS and mnemonic paths, verify whether the class still needs a separate `address` property at all and either remove it entirely or make `addresses[0]` the single source of truth to avoid inconsistent state.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…ss after selection
| this.address = this.filledAddresses[0] | ||
| this.proxyContract = new this.web3.eth.Contract(AdminWalletABI, adminWalletAddress, { from: this.address }) | ||
|
|
||
| if (this.conf.topAdminsOnStartup) { |
There was a problem hiding this comment.
yes but now after calling topadmins there could be more admin accounts that are available.
previously the topadmins was called before searching for valid admin accounts
There was a problem hiding this comment.
I don’t think that scenario is possible.
topAdmins simply gets this.conf.numberOfAdminWalletAccounts and doesn’t rely on the filtered admin account array (this.filledAddress).
Because of that, the change shouldn’t be related to the set of available admin accounts.
There was a problem hiding this comment.
The scenario is happening by design.
The first step selects addresses into filledAddresses, those that have a balance. If they dont have a balance they wont be selected.
After topadmins they can now have a balance.
There was a problem hiding this comment.
Ah got it, I will initialize this.filledAddresses after topAdmin.
There was a problem hiding this comment.
I have updated the logic
You can review now @sirpy
…t selection based on balance
3 participants
Description
Remove incorrect default address assignment in web3Wallet initialization
About # (link your issue here)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist:
Summary by Sourcery
Bug Fixes: