fibercryptowallet icon indicating copy to clipboard operation
fibercryptowallet copied to clipboard

Confusing design of the wallets and addresses models.

Open stdevCrow opened this issue 3 years ago • 6 comments

I'm coding the C++ version of the model that shows wallets and its addresses in the main page, and I realize that:

  • In the walletsModel.go (https://github.com/fibercrypto/fibercryptowallet/blob/fb9e9d3455a254b9202d24ab8af689fbb6db083d/src/models/walletsModel.go#L50), there's a QWallet type.

  • In the addressesModel.go (https://github.com/fibercrypto/fibercryptowallet/blob/fb9e9d3455a254b9202d24ab8af689fbb6db083d/src/models/addressesModel.go#L41), there's a QAddress type.

But despite a wallet has a list of addresses, there's no connection between them but a walletId property in the QAddress type, and a QWallet does not have such property, so this whole design makes no sense to me.

Describe the solution you'd like

  • A C++ WalletsModel that contains a list of Wallet types that contains an AddressesModel type containing the list of Address types related to that wallet.

It's there any problem with this implementation?

stdevCrow avatar Mar 28 '21 19:03 stdevCrow

@stdevCrow @olemis pls take a look at this 👆 asap, I need a yes or no to continue.

stdevCrow avatar Mar 28 '21 19:03 stdevCrow

@stdevCrow I guess model name should be Wallet (instead of Wallets ...)

olemis avatar Mar 28 '21 22:03 olemis

@olemis Agree, but are you okay with the propossed implementation?

stdevCrow avatar Mar 29 '21 02:03 stdevCrow

@stdevCrow Original code works that way for a reason , it seems to me that it's due to the fact that golang wallet address objects are generated by wallet enumeration methods hence they might differ from one method call to the next

This would be of particular importance through the C API

olemis avatar Mar 29 '21 03:03 olemis

@olemis I'm not talking about changing underlying system. I meant that if the addresses contains a walletId, both can be grouped together in the Qt model. The backend will not know the difference. And the model of addresses will not know the existence of the model of wallets. The will work as always, but addresses with the same walletId will be grouped under the same wallet in the WalletModel.

stdevCrow avatar Mar 30 '21 14:03 stdevCrow

@stdevCrow it seems to me (cmiiw) it's not a wallet ID but an address ID . See BIP-44 hierachical addressing standard . It's not a list of addresses , but an inifinite derivations tree

olemis avatar Mar 30 '21 14:03 olemis