chore(dashpay): Merge feature-dashdirect into dashpay-dashdirect #1130
Conversation
| // TODO: do we need these? | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY); | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_FUNDING); | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_TOPUP); | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.INVITATION_FUNDING); | ||
|
|
| // TODO: do we need only Platform types in here? | ||
| private val keyChainTypes = EnumSet.of( | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_TOPUP, | ||
| AuthenticationKeyChain.KeyChainType.INVITATION_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_PLATFORM_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_VOTING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OWNER | ||
| ) |
There was a problem hiding this comment.
We only need platform types here. The masternode types will be added when users go to the Masternode Keys page if those key types were not yet added to the wallet.
| } else { | ||
| // TODO: do we need this branch? addWalletAuthenticationKeysAsync is only called with non-null keyParameter | ||
| authenticationGroupExtension.addKeyChains( | ||
| wallet.params, | ||
| seed, | ||
| keyChainTypes | ||
| ) | ||
| } |
There was a problem hiding this comment.
I suppose we don't need this else. We have other places in DashPay that handle the case of a null keyParameter, but the wallet is always encrypted so this keyParameter will always be non-null.
| AuthenticationGroupExtension authExtension = | ||
| (AuthenticationGroupExtension) dashPayWallet.addOrGetExistingExtension( | ||
| // TODO: do we need to call addKeyChains here? | ||
| new AuthenticationGroupExtension(dashPayWallet.getParams()) | ||
| ); | ||
| CreditFundingTransaction cftx = authExtension.getCreditFundingTransaction(tx); | ||
| // TODO: getKeyChainGroup and getKeyChainType are protected. Any other way to get the type? | ||
| AuthenticationKeyChainGroup group = ((AuthenticationKeyChainGroup)authExtension.getKeyChainGroup()); | ||
| switch (group.getKeyChainType(cftx.getCreditBurnPublicKeyId().getBytes())) { |
There was a problem hiding this comment.
if CreditFundingTransaction.isCreditFundingTransaction(tx) is true, that means that the keychains were already added during as part of username creation. We don't need to call addKeyChains here.
The only place to call addKeyChains is during username creation.
There was a problem hiding this comment.
@HashEngineering what about the second issue - we were checking the transaction type by calling wallet. getAuthenticationKeyType, but now it's missing, and keyChainGroup.getKeyChainType method is protected.
There was a problem hiding this comment.
I made a PR for this:
dashpay/dashj#217
It is part of the CoinJoin branch, which we will be using in the next PR after this one.
You should be able to change the dashj version to 19.1-CJ-SNAPSHOT and hopefully nothing is missing or broken. Otherwise, we can make the change change to the master branch of dashj
There was a problem hiding this comment.
@HashEngineering another change might be needed. See the PR
| AuthenticationGroupExtension authExtension = | ||
| (AuthenticationGroupExtension) wallet.addOrGetExistingExtension( | ||
| // TODO: do we need to call addKeyChains here? | ||
| new AuthenticationGroupExtension(wallet.getParams()) | ||
| ); | ||
| CreditFundingTransaction cftx = authExtension.getCreditFundingTransaction(tx); |
There was a problem hiding this comment.
We don't to call addKeyChains here.
| // TODO: is there a simpler way to do this? | ||
| val authenticationGroupExtension = AuthenticationGroupExtension(wallet) | ||
| authenticationGroupExtension.addEncryptedKeyChains( | ||
| wallet.params, | ||
| decryptedSeed, | ||
| newKey, | ||
| EnumSet.of( | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_TOPUP, | ||
| AuthenticationKeyChain.KeyChainType.INVITATION_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_PLATFORM_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_VOTING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OWNER | ||
| ) | ||
| ) | ||
| wallet.addOrGetExistingExtension(authenticationGroupExtension) |
There was a problem hiding this comment.
this can be removed because WalletApplication.setWallet does this.
EncryptLiveData is only used after a user installs and does 10000 the onboarding (create wallet or restore wallet).
| // TODO: is this the right way to get the auth extension? | ||
| // Should we have static ID for the extension? | ||
| // Do we need to call addKeyChains here? | ||
| val authExtension = wallet.addOrGetExistingExtension( | ||
| AuthenticationGroupExtension(wallet.params) | ||
| ) as AuthenticationGroupExtension | ||
| authExtension.getCreditFundingTransaction(this) |
There was a problem hiding this comment.
Sadly, this is the way. having a static id will be the best way for this case. I will add it in my Platform 0.24 story. When CoinJoin (feature-coinjoin) is merged into DashPay (dashpay), then we replace this and other places.
| // TODO: not used. Do we need this? | ||
| // val invitationLinkData = liveData(Dispatchers.IO) { | ||
| // val tx = walletApplication.wallet!!.getTransaction(invitation.txid) | ||
| // val cftx = walletApplication.wallet!!.getCreditFundingTransaction(tx) | ||
| // | ||
| // val wallet = WalletApplication.getInstance().wallet!! | ||
| // val password = SecurityGuard().retrievePassword() | ||
| // var encryptionKey: KeyParameter? = null | ||
| // try { | ||
| // encryptionKey = wallet.keyCrypter!!.deriveKey(password) | ||
| // } catch (ex: KeyCrypterException) { | ||
| // analytics.logError(ex, "create invitation link: failed to derive encryption key") | ||
| // emit("") | ||
| // } | ||
| // | ||
| // val invite = platformRepo.blockchainIdentity.getInvitationString(cftx, encryptionKey) | ||
| // emit(invite) | ||
| // } |
There was a problem hiding this comment.
This was superceded by other methods that obtain the invite string. This can be removed.
| // TODO: do we need these? | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY); | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_FUNDING); | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_TOPUP); | ||
| authenticationGroupExtension.freshKey(AuthenticationKeyChain.KeyChainType.INVITATION_FUNDING); |
There was a problem hiding this comment.
Not sure about this part.
| // TODO: is this the right way to get the auth extension? | ||
| // Should we have static ID for the extension? | ||
| // Do we need to call addKeyChains here? | ||
| val authExtension = wallet.addOrGetExistingExtension( | ||
| AuthenticationGroupExtension(wallet.params) | ||
| ) as AuthenticationGroupExtension | ||
| authExtension.getCreditFundingTransaction(this) |
| // TODO: is there a simpler way to do this? | ||
| val authenticationGroupExtension = AuthenticationGroupExtension(wallet) | ||
| authenticationGroupExtension.addEncryptedKeyChains( | ||
| wallet.params, | ||
| decryptedSeed, | ||
| newKey, | ||
| EnumSet.of( | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_TOPUP, | ||
| AuthenticationKeyChain.KeyChainType.INVITATION_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_PLATFORM_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_VOTING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OWNER | ||
| ) | ||
| ) | ||
| wallet.addOrGetExistingExtension(authenticationGroupExtension) |
| AuthenticationGroupExtension authExtension = | ||
| (AuthenticationGroupExtension) wallet.addOrGetExistingExtension( | ||
| // TODO: do we need to call addKeyChains here? | ||
| new AuthenticationGroupExtension(wallet.getParams()) | ||
| ); | ||
| CreditFundingTransaction cftx = authExtension.getCreditFundingTransaction(tx); |
| val authExtension = walletApplication.wallet!!.addOrGetExistingExtension( | ||
| AuthenticationGroupExtension(walletApplication.wallet!!.params) | ||
| ) as AuthenticationGroupExtension | ||
| val cftxs = authExtension.creditFundingTransactions |
There was a problem hiding this comment.
Do we need to call addKeyChains here?
| // TODO: do we need only Platform types in here? | ||
| private val keyChainTypes = EnumSet.of( | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.BLOCKCHAIN_IDENTITY_TOPUP, | ||
| AuthenticationKeyChain.KeyChainType.INVITATION_FUNDING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_PLATFORM_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OPERATOR, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_VOTING, | ||
| AuthenticationKeyChain.KeyChainType.MASTERNODE_OWNER | ||
| ) | ||
|
|
| // TODO: do we need this branch? addWalletAuthenticationKeysAsync is only called with non-null keyParameter | ||
| authenticationGroupExtension.addKeyChains( | ||
| wallet.params, | ||
| seed, | ||
| keyChainTypes | ||
| ) | ||
| } |
| // | ||
| // val invite = platformRepo.blockchainIdentity.getInvitationString(cftx, encryptionKey) | ||
| // emit(invite) | ||
| // } | ||
|
|
There was a problem hiding this comment.
getCreditFundingTransaction required refactoring, but given that we don't use invitationLinkData it might be simpler to just remove it.
| AuthenticationGroupExtension authExtension = | ||
| (AuthenticationGroupExtension) dashPayWallet.addOrGetExistingExtension( | ||
| // TODO: do we need to call addKeyChains here? | ||
| new AuthenticationGroupExtension(dashPayWallet.getParams()) | ||
| ); | ||
| CreditFundingTransaction cftx = authExtension.getCreditFundingTransaction(tx); | ||
| // TODO: getKeyChainGroup and getKeyChainType are protected. Any other way to get the type? | ||
| AuthenticationKeyChainGroup group = ((AuthenticationKeyChainGroup)authExtension.getKeyChainGroup()); | ||
| switch (group.getKeyChainType(cftx.getCreditBurnPublicKeyId().getBytes())) { |
| AuthenticationGroupExtension authExtension = | ||
| (AuthenticationGroupExtension) dashPayWallet.addOrGetExistingExtension( | ||
| // TODO: do we need to call addKeyChains here? | ||
| new AuthenticationGroupExtension(dashPayWallet.getParams()) | ||
| ); | ||
| CreditFundingTransaction cftx = authExtension.getCreditFundingTransaction(tx); | ||
| // TODO: getKeyChainGroup and getKeyChainType are protected. Any other way to get the type? | ||
| AuthenticationKeyChainGroup group = ((AuthenticationKeyChainGroup)authExtension.getKeyChainGroup()); | ||
| switch (group.getKeyChainType(cftx.getCreditBurnPublicKeyId().getBytes())) { |
There was a problem hiding this comment.
@HashEngineering what about the second issue - we were checking the transaction type by calling wallet. getAuthenticationKeyType, but now it's missing, and keyChainGroup.getKeyChainType method is protected.
We need to merge
masterintodashpay.To avoid doing more work than necessary, we'll first merge
dashdirectbranch intodashpay-dashdirect, and then the latter intodashpay.Issue being fixed or feature implemented
getCreditFundingTransactionand authentication group.Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist: