Addresses generator (#1) #825
Conversation
- Generation amount of addresses added - Generation wallet with index > 9 fixed - Test generation addresses added
|
I pulled from develop and there had failed tests too |
|
@6od9i Hey, thanks for this PR. Could you please fix typos within added code, to make the ci pipeline green. |
- Removed prefix path from address generation
| var pathAppendix: String? | ||
|
|
||
| return [Int](0..<number).compactMap({ number in | ||
| pathAppendix = nil |
There was a problem hiding this comment.
your variable should be declared here and not line 233
| let prefixPath = self.rootPrefix | ||
| var pathAppendix: String? | ||
|
|
||
| return [Int](0..<number).compactMap({ number in |
There was a problem hiding this comment.
please use trailing closure syntax
| pathAppendix = nil | ||
| let path = prefixPath + "/\(number)" | ||
| if path.hasPrefix(prefixPath) { | ||
| let upperIndex = (path.range(of: prefixPath)?.upperBound)! |
There was a problem hiding this comment.
please no force unwrap
There was a problem hiding this comment.
copyed from createNewCustomChildAccount
| guard pathAppendix != nil else { | ||
| return nil | ||
| } | ||
| if pathAppendix!.hasPrefix("/") { |
There was a problem hiding this comment.
copyed from createNewCustomChildAccount
| return nil | ||
| } | ||
| if pathAppendix!.hasPrefix("/") { | ||
| pathAppendix = pathAppendix?.trimmingCharacters(in: CharacterSet.init(charactersIn: "/")) |
There was a problem hiding this comment.
please use proper swift init syntax
There was a problem hiding this comment.
copyed from createNewCustomChildAccount
There was a problem hiding this comment.
All CharacterSet.init were replaced with .init to reduce the length of lines where it occurs.
| } | ||
| } else { | ||
| if path.hasPrefix("/") { | ||
| pathAppendix = path.trimmingCharacters(in: CharacterSet.init(charactersIn: "/")) |
There was a problem hiding this comment.
please use proper swift init syntax
There was a problem hiding this comment.
copyed from createNewCustomChildAccount
| } | ||
| guard pathAppendix != nil, | ||
| rootNode.depth == prefixPath.components(separatedBy: "/").count - 1, | ||
| let newNode = rootNode.derive(path: pathAppendix!, derivePrivateKey: true), |
There was a problem hiding this comment.
please no force unwrap
There was a problem hiding this comment.
Hi, thank you for your comments, but this code in style of all this file
I copied createNewCustomChildAccount method and changed it here, force unwraps had been written here everywhere, that why i used it. And i do not want to change all this file, i used this style
There was a problem hiding this comment.
In addition to all of the folks said, I'd like to add that there's a fair point in your arguing about relatively bad code style within all the least file. The thing is that this is how we decided to maintain this lib some time ago: we're leaving all the code that works yet unsound for as long as we're not required to change it. But if we are we're making it as sounded as we can.
Certainly it's not necessary to rewrite the whole file at the time, actually it's just fine to write in a good way just the required part, but it would be so much better if meanwhile you're applying your effort to the main problem to improve as much relative code as your effort big enough, this would really help to all successor folks who will use this lib afterwards, and you'll get a significant part of that appreciation.
There was a problem hiding this comment.
I think it is very dangerous to change all this code, because i do not have enough time to understand all logic, i will change this method
- pathAppendix declaration place changed
- small refactoring
fix: createNewAccount always non-hardened derivation
chore: refactoring
I had added method for faster generation addresses by derivation path index in seed, and found bug with generation.
There was bug with generation wallets from seed when index has more than 1 symbol, for example:
address of wallet "m/44'/60'/0'/1" same with wallet "m/44'/60'/0'/10" or "m/44'/60'/0'/121"