From 654da3e246fc55c43a06cdeca67277459cd05a0e Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Tue, 23 Jan 2024 09:15:07 +0100 Subject: [PATCH] fix(savedaddresses): preferred chains maintained in two places This commit prevents the user from adding an address to the saved addresses list, if it was already added to the Wallet section. Also when the user is about to add an address to the Wallet section, which is already added to the saved addresses list, the app will ask whether to proceed with that action by removing the related saved address or cancel the action. Closes: #13109 --- .../modules/main/wallet_section/module.nim | 6 ++- .../shared_modules/add_account/controller.nim | 31 ++++++++++---- .../add_account/io_interface.nim | 5 +++ .../shared_modules/add_account/module.nim | 42 ++++++++++++------- .../shared_modules/add_account/view.nim | 9 ++++ .../service/saved_address/service.nim | 7 +++- .../popups/AddEditSavedAddressPopup.qml | 35 ++++++++++++---- .../popups/addaccount/AddAccountPopup.qml | 39 +++++++++++++++++ 8 files changed, 140 insertions(+), 34 deletions(-) diff --git a/src/app/modules/main/wallet_section/module.nim b/src/app/modules/main/wallet_section/module.nim index fc487927c5..1f2ee931f3 100644 --- a/src/app/modules/main/wallet_section/module.nim +++ b/src/app/modules/main/wallet_section/module.nim @@ -78,6 +78,7 @@ type keycardService: keycard_service.Service accountsService: accounts_service.Service walletAccountService: wallet_account_service.Service + savedAddressService: saved_address_service.Service devicesService: devices_service.Service activityController: activityc.Controller @@ -114,6 +115,7 @@ proc newModule*( result.keycardService = keycardService result.accountsService = accountsService result.walletAccountService = walletAccountService + result.savedAddressService = savedAddressService result.devicesService = devicesService result.moduleLoaded = false result.controller = newController(result, settingsService, walletAccountService, currencyService, networkService) @@ -375,13 +377,13 @@ method destroyAddAccountPopup*(self: Module) = method runAddAccountPopup*(self: Module, addingWatchOnlyAccount: bool) = self.destroyAddAccountPopup() self.addAccountModule = add_account_module.newModule(self, self.events, self.keycardService, self.accountsService, - self.walletAccountService) + self.walletAccountService, self.savedAddressService) self.addAccountModule.loadForAddingAccount(addingWatchOnlyAccount) method runEditAccountPopup*(self: Module, address: string) = self.destroyAddAccountPopup() self.addAccountModule = add_account_module.newModule(self, self.events, self.keycardService, self.accountsService, - self.walletAccountService) + self.walletAccountService, self.savedAddressService) self.addAccountModule.loadForEditingAccount(address) method getAddAccountModule*(self: Module): QVariant = diff --git a/src/app/modules/shared_modules/add_account/controller.nim b/src/app/modules/shared_modules/add_account/controller.nim index 7370cce602..1aa4595b50 100644 --- a/src/app/modules/shared_modules/add_account/controller.nim +++ b/src/app/modules/shared_modules/add_account/controller.nim @@ -2,13 +2,14 @@ import times, chronicles import uuids import io_interface -import ../../../../app_service/service/accounts/service as accounts_service -import ../../../../app_service/service/wallet_account/service as wallet_account_service -import ../../../../app_service/service/keycard/service as keycard_service +import app_service/service/accounts/service as accounts_service +import app_service/service/wallet_account/service as wallet_account_service +import app_service/service/saved_address/service as saved_address_service +import app_service/service/keycard/service as keycard_service import ../keycard_popup/io_interface as keycard_shared_module -import ../../../core/eventemitter +import app/core/eventemitter logScope: topics = "wallet-add-account-controller" @@ -21,6 +22,7 @@ type events: EventEmitter accountsService: accounts_service.Service walletAccountService: wallet_account_service.Service + savedAddressService: saved_address_service.Service keycardService: keycard_service.Service connectionIds: seq[UUID] connectionKeycardResponse: UUID @@ -40,6 +42,7 @@ proc newController*(delegate: io_interface.AccessInterface, events: EventEmitter, accountsService: accounts_service.Service, walletAccountService: wallet_account_service.Service, + savedAddressService: saved_address_service.Service, keycardService: keycard_service.Service): Controller = result = Controller() @@ -47,6 +50,7 @@ proc newController*(delegate: io_interface.AccessInterface, result.events = events result.accountsService = accountsService result.walletAccountService = walletAccountService + result.savedAddressService = savedAddressService result.keycardService = keycardService proc disconnectAll*(self: Controller) = @@ -66,27 +70,32 @@ proc init*(self: Controller) = self.connectionIds.add(handlerId) handlerId = self.events.onWithUUID(SIGNAL_WALLET_ACCOUNT_DERIVED_ADDRESSES_FETCHED) do(e:Args): - var args = DerivedAddressesArgs(e) + let args = DerivedAddressesArgs(e) self.delegate.onDerivedAddressesFetched(args.derivedAddresses, args.error) self.connectionIds.add(handlerId) handlerId = self.events.onWithUUID(SIGNAL_WALLET_ACCOUNT_DERIVED_ADDRESSES_FROM_MNEMONIC_FETCHED) do(e:Args): - var args = DerivedAddressesArgs(e) + let args = DerivedAddressesArgs(e) self.delegate.onDerivedAddressesFromMnemonicFetched(args.derivedAddresses, args.error) self.connectionIds.add(handlerId) handlerId = self.events.onWithUUID(SIGNAL_DERIVED_ADDRESSES_FROM_NOT_IMPORTED_MNEMONIC_FETCHED) do(e:Args): - var args = DerivedAddressesFromNotImportedMnemonicArgs(e) + let args = DerivedAddressesFromNotImportedMnemonicArgs(e) self.delegate.onAddressesFromNotImportedMnemonicFetched(args.derivations, args.error) self.connectionIds.add(handlerId) handlerId = self.events.onWithUUID(SIGNAL_WALLET_ACCOUNT_ADDRESS_DETAILS_FETCHED) do(e:Args): - var args = DerivedAddressesArgs(e) + let args = DerivedAddressesArgs(e) if args.uniqueId != self.uniqueFetchingDetailsId: return self.delegate.onAddressDetailsFetched(args.derivedAddresses, args.error) self.connectionIds.add(handlerId) + handlerId = self.events.onWithUUID(SIGNAL_SAVED_ADDRESS_DELETED) do(e:Args): + let args = SavedAddressArgs(e) + self.delegate.savedAddressDeleted(args.address, args.errorMsg) + self.connectionIds.add(handlerId) + proc setAuthenticatedKeyUid*(self: Controller, value: string) = self.tmpAuthenticatedKeyUid = value @@ -123,6 +132,12 @@ proc getKeypairs*(self: Controller): seq[KeypairDto] = proc getKeypairByKeyUid*(self: Controller, keyUid: string): KeypairDto = return self.walletAccountService.getKeypairByKeyUid(keyUid) +proc getSavedAddress*(self: Controller, address: string): SavedAddressDto = + return self.savedAddressService.getSavedAddress(address) + +proc deleteSavedAddress*(self: Controller, address: string) = + self.savedAddressService.deleteSavedAddress(address) + proc finalizeAction*(self: Controller) = self.delegate.finalizeAction() diff --git a/src/app/modules/shared_modules/add_account/io_interface.nim b/src/app/modules/shared_modules/add_account/io_interface.nim index c64e04ab39..1dc55332de 100644 --- a/src/app/modules/shared_modules/add_account/io_interface.nim +++ b/src/app/modules/shared_modules/add_account/io_interface.nim @@ -101,6 +101,11 @@ method buildNewPrivateKeyKeypairAndAddItToOrigin*(self: AccessInterface) {.base. method buildNewSeedPhraseKeypairAndAddItToOrigin*(self: AccessInterface) {.base.} = raise newException(ValueError, "No implementation available") +method removingSavedAddressConfirmed*(self: AccessInterface, address: string) {.base.} = + raise newException(ValueError, "No implementation available") + +method savedAddressDeleted*(self: AccessInterface, address: string, errorMsg: string) {.base.} = + raise newException(ValueError, "No implementation available") type DelegateInterface* = concept c diff --git a/src/app/modules/shared_modules/add_account/module.nim b/src/app/modules/shared_modules/add_account/module.nim index b1095c77fd..dd459a0689 100644 --- a/src/app/modules/shared_modules/add_account/module.nim +++ b/src/app/modules/shared_modules/add_account/module.nim @@ -4,18 +4,19 @@ import io_interface import view, controller import internal/[state, state_factory] -import ../../../core/eventemitter +import app/core/eventemitter -import ../../../global/global_singleton +import app/global/global_singleton -import ../../shared/keypairs -import ../../shared_models/[keypair_model, derived_address_model] -import ../../shared_modules/keycard_popup/module as keycard_shared_module +import app/modules/shared/keypairs +import app/modules/shared_models/[keypair_model, derived_address_model] +import app/modules/shared_modules/keycard_popup/module as keycard_shared_module -import ../../../../app_service/common/account_constants -import ../../../../app_service/service/accounts/service as accounts_service -import ../../../../app_service/service/wallet_account/service as wallet_account_service -import ../../../../app_service/service/keycard/service as keycard_service +import app_service/common/account_constants +import app_service/service/accounts/service as accounts_service +import app_service/service/wallet_account/service as wallet_account_service +import app_service/service/saved_address/service as saved_address_service +import app_service/service/keycard/service as keycard_service export io_interface @@ -46,8 +47,6 @@ type view: View viewVariant: QVariant controller: Controller - accountsService: accounts_service.Service - walletAccountService: wallet_account_service.Service authenticationReason: AuthenticationReason fetchingAddressesIsInProgress: bool @@ -58,15 +57,16 @@ proc newModule*[T](delegate: T, events: EventEmitter, keycardService: keycard_service.Service, accountsService: accounts_service.Service, - walletAccountService: wallet_account_service.Service): + walletAccountService: wallet_account_service.Service, + savedAddressService: saved_address_service.Service): Module[T] = result = Module[T]() result.delegate = delegate result.events = events - result.walletAccountService = walletAccountService result.view = newView(result) result.viewVariant = newQVariant(result.view) - result.controller = controller.newController(result, events, accountsService, walletAccountService, keycardService) + result.controller = controller.newController(result, events, accountsService, walletAccountService, + savedAddressService, keycardService) result.authenticationReason = AuthenticationReason.AddingAccount result.fetchingAddressesIsInProgress = false @@ -629,6 +629,11 @@ proc doAddAccount[T](self: Module[T]) = publicKey = "" keyUid = "" + let savedAddressDto = self.controller.getSavedAddress(address) + if not savedAddressDto.isNil: + self.view.sendConfirmSavedAddressRemovalSignal(savedAddressDto.name, savedAddressDto.address) + return + var success = false if addingNewKeyPair: if selectedOrigin.getPairType() == KeyPairType.PrivateKeyImport.int: @@ -689,6 +694,15 @@ proc doAddAccount[T](self: Module[T]) = self.closeAddAccountPopup() +method removingSavedAddressConfirmed[T](self: Module[T], address: string) = + self.controller.deleteSavedAddress(address) + +method savedAddressDeleted*[T](self: Module[T], address: string, errorMsg: string) = + if errorMsg.len > 0: + error "failed to delete saved address", address=address, err=errorMsg + return + self.doAddAccount() + proc doEditAccount[T](self: Module[T]) = self.view.setDisablePopup(true) let selectedOrigin = self.view.getSelectedOrigin() diff --git a/src/app/modules/shared_modules/add_account/view.nim b/src/app/modules/shared_modules/add_account/view.nim index 823d140f81..485013c2f7 100644 --- a/src/app/modules/shared_modules/add_account/view.nim +++ b/src/app/modules/shared_modules/add_account/view.nim @@ -354,3 +354,12 @@ QtObject: proc startScanningForActivity*(self: View) {.slot.} = self.delegate.startScanningForActivity() + proc confirmSavedAddressRemoval*(self: View, name: string, address: string) {.signal.} + proc sendConfirmSavedAddressRemovalSignal*(self: View, name: string, address: string) = + self.confirmSavedAddressRemoval(name, address) + + proc removingSavedAddressConfirmed*(self: View, address: string) {.slot.} = + self.delegate.removingSavedAddressConfirmed(address) + + proc removingSavedAddressRejected*(self: View) {.slot.} = + self.setDisablePopup(false) \ No newline at end of file diff --git a/src/app_service/service/saved_address/service.nim b/src/app_service/service/saved_address/service.nim index 7cb6073f54..3fa9cb404e 100644 --- a/src/app_service/service/saved_address/service.nim +++ b/src/app_service/service/saved_address/service.nim @@ -1,4 +1,4 @@ -import NimQml, chronicles, sequtils, json +import NimQml, chronicles, strutils, sequtils, json import dto @@ -73,6 +73,11 @@ QtObject: proc getSavedAddresses*(self: Service): seq[SavedAddressDto] = return self.savedAddresses + proc getSavedAddress*(self: Service, address: string): SavedAddressDto = + for sa in self.savedAddresses: + if cmpIgnoreCase(sa.address, address) == 0: + return sa + proc updateAddresses(self: Service, signal: string, arg: Args) = self.savedAddresses = self.getAddresses() self.events.emit(signal, arg) diff --git a/ui/app/AppLayouts/Wallet/popups/AddEditSavedAddressPopup.qml b/ui/app/AppLayouts/Wallet/popups/AddEditSavedAddressPopup.qml index 3c232e278d..16cb652f5c 100644 --- a/ui/app/AppLayouts/Wallet/popups/AddEditSavedAddressPopup.qml +++ b/ui/app/AppLayouts/Wallet/popups/AddEditSavedAddressPopup.qml @@ -99,11 +99,16 @@ StatusModal { readonly property var chainPrefixRegexPattern: /[^:]+\:?|:/g readonly property bool addressInputIsENS: !!d.ens - property bool addressAlreadyAdded: false - function checkIfAddressIsAlreadyAddded(address) { + property bool addressAlreadyAddedToWallet: false + function checkIfAddressIsAlreadyAdddedToWallet(address) { + let name = RootStore.getNameForWalletAddress(address) + d.addressAlreadyAddedToWallet = !!name + } + + property bool addressAlreadyAddedToSavedAddresses: false + function checkIfAddressIsAlreadyAdddedToSavedAddresses(address) { let details = RootStore.getSavedAddress(address) - d.addressAlreadyAdded = !!details.address - return !d.addressAlreadyAdded + d.addressAlreadyAddedToSavedAddresses = !!details.address } /// Ensures that the \c root.address and \c root.chainShortNames are not reset when the initial text is set @@ -220,7 +225,8 @@ StatusModal { errorMessage: qsTr("Please enter an ethereum address") }, StatusValidator { - errorMessage: d.addressAlreadyAdded? qsTr("This address is already saved") : qsTr("Ethereum address invalid") + errorMessage: d.addressAlreadyAddedToWallet? qsTr("This address is already added to Wallet") : + d.addressAlreadyAddedToSavedAddresses? qsTr("This address is already saved") : qsTr("Ethereum address invalid") validate: function (value) { if (value !== Constants.zeroAddress) { if (Utils.isValidEns(value)) { @@ -231,7 +237,12 @@ StatusModal { return true } const prefixAndAddress = Utils.splitToChainPrefixAndAddress(value) - return d.checkIfAddressIsAlreadyAddded(prefixAndAddress.address) + d.checkIfAddressIsAlreadyAdddedToWallet(prefixAndAddress.address) + if (d.addressAlreadyAddedToWallet) { + return false + } + d.checkIfAddressIsAlreadyAdddedToSavedAddresses(prefixAndAddress.address) + return !d.addressAlreadyAddedToSavedAddresses } } @@ -243,7 +254,8 @@ StatusModal { StatusAsyncValidator { id: resolvingEnsName name: "resolving-ens-name" - errorMessage: d.addressAlreadyAdded? qsTr("This address is already saved") : qsTr("Ethereum address invalid") + errorMessage: d.addressAlreadyAddedToWallet? qsTr("This address is already added to Wallet") : + d.addressAlreadyAddedToSavedAddresses? qsTr("This address is already saved") : qsTr("Ethereum address invalid") asyncOperation: (value) => { if (!Utils.isValidEns(value)) { resolvingEnsName.asyncComplete("not-ens") @@ -257,7 +269,12 @@ StatusModal { return true } if (!!value) { - return d.checkIfAddressIsAlreadyAddded(value) + d.checkIfAddressIsAlreadyAdddedToWallet(prefixAndAddress.address) + if (d.addressAlreadyAddedToWallet) { + return false + } + d.checkIfAddressIsAlreadyAdddedToSavedAddresses(value) + return !d.addressAlreadyAddedToSavedAddresses } return false } @@ -290,7 +307,7 @@ StatusModal { if (skipTextUpdate || !d.initialized) return - d.addressAlreadyAdded = false + d.addressAlreadyAddedToSavedAddresses = false plainText = input.edit.getText(0, text.length) if (input.edit.previousText != plainText) { diff --git a/ui/imports/shared/popups/addaccount/AddAccountPopup.qml b/ui/imports/shared/popups/addaccount/AddAccountPopup.qml index 7f89196647..0c8e88a498 100644 --- a/ui/imports/shared/popups/addaccount/AddAccountPopup.qml +++ b/ui/imports/shared/popups/addaccount/AddAccountPopup.qml @@ -6,6 +6,7 @@ import StatusQ.Popups 0.1 import StatusQ.Controls 0.1 import utils 1.0 +import shared.popups 1.0 import "./stores" import "./states" @@ -31,6 +32,13 @@ StatusModal { root.store.currentState.doCancelAction() } + Connections { + target: root.store.addAccountModule + function onConfirmSavedAddressRemoval(name, address) { + Global.openPopup(confirmSavedAddressRemoval, {address: address, name: name}) + } + } + StatusScrollView { id: scrollView @@ -154,6 +162,37 @@ StatusModal { } } } + + Component { + id: confirmSavedAddressRemoval + + ConfirmationDialog { + + property string name + property string address + + closePolicy: Popup.NoAutoClose + hasCloseButton: false + headerSettings.title: qsTr("Removing saved address") + confirmationText: qsTr("The account you're trying to add %1 is already saved under the name %2.

Do you want to remove it from saved addresses in favour of adding it to the Wallet?") + .arg(address) + .arg(name) + showCancelButton: true + cancelBtnType: "" + confirmButtonLabel: qsTr("Yes") + cancelButtonLabel: qsTr("No") + + onConfirmButtonClicked: { + root.store.addAccountModule.removingSavedAddressConfirmed(address) + close() + } + + onCancelButtonClicked: { + root.store.addAccountModule.removingSavedAddressRejected() + close() + } + } + } } leftButtons: [