Details

Reviewers

Fabien
bytesofman

Group Reviewers

Restricted Project

Maniphest Tasks

Restricted Maniphest Task

Commits

rABCaec3c4a0d473: [electrum] implement adding eCash alias to contacts

Summary

Add two menu actions to fetch aliases from https://alias.etokens.cash and add them to contacts. It can be reached via the main application menu (Wallet > Contacts) or in the context menu in the Contacts tab.
This will allow users to type in an alias in the "Pay to" field in the send tab, with autocompletion.

This assumes that for now aliases are immutable (phase 1 aliases). If this changes, we will have to check and update dynamically the alias when the user tries to send money to it.

For now the feature is gated by an activation flag --enable-aliases.

ref T3298

Test Plan

Start the application with the new activation flag: ./electrum-abc --enable-aliases
Try the feature with various aliases (hint: twelvechar12 is registered)

Once an alias is succesfully added to contacts, check that autocompletion works in the send tab

Try cancelling the dialog after the OK button is clicked and while the request is still running, verify there is no segfault because of QThread destroyed while still running.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

contact_list

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25173
Build 49934: Build Diff	electrum-tests
Build 49933: arc lint + arc unit

Event Timeline

PiRK created this revision.Sep 26 2023, 14:44

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 26 2023, 14:44

PiRK mentioned this in D14542: [electrum] add a new type of contact for ecash aliases.Sep 26 2023, 14:44

PiRK edited the summary of this revision. (Show Details)

PiRK added a parent revision: D14542: [electrum] add a new type of contact for ecash aliases.

Harbormaster completed remote builds in B25171: Diff 42394.Sep 26 2023, 14:45

PiRK edited the test plan for this revision. (Show Details)Sep 26 2023, 14:47

add a qregexpvalidator, handle timeouts, rename dialog, provide the error message (if any) in the error message

Harbormaster completed remote builds in B25172: Diff 42396.Sep 26 2023, 15:24

add tooltip, restrict regexp to lowercase chars, plan for translated strings (even if there is no translation happening at the moment)

Harbormaster completed remote builds in B25173: Diff 42397.Sep 26 2023, 15:45

PiRK edited the summary of this revision. (Show Details)Sep 26 2023, 15:49

use a separate thread to not block the UI, set the waiting cursor while request is running, prevent the user from modifying the alias while the request is happening

Harbormaster completed remote builds in B25178: Diff 42409.Sep 27 2023, 07:45

add a nice picture to make the dialog less boring

PiRK published this revision for review.Sep 27 2023, 07:57

Harbormaster completed remote builds in B25179: Diff 42410.Sep 27 2023, 07:58

Screenshot from 2023-09-27 10-01-45.png (270×677 px, 30 KB)

Please gate this feature behind a dev flag for now as aliases are not production ready yet.

This revision now requires changes to proceed.Sep 27 2023, 08:43

gate the feature behind an activation flag, add the action also to the Wallet > Contacts menu

Harbormaster completed remote builds in B25180: Diff 42411.Sep 27 2023, 11:25

PiRK added a child revision: D14547: [electrum] support eCash aliases in the "Pay To" field.Sep 27 2023, 13:11

bytesofman requested changes to this revision.Sep 27 2023, 19:10

bytesofman added a subscriber: bytesofman.

bytesofman added inline comments.

electrum/electrumabc_gui/qt/contact_list.py
519 ↗	(On Diff #42411)	note the base URL is likely to change here, while the endpoint(s) will not. May be better to store the base `https://alias.etokens.cash/` as a constant with the feature flag, so it can be changed when the --enabled flag is changed.
631 ↗	(On Diff #42411)	practical / possible to improve this msg, say `Alias <alias> is not registered.` "seem to be" is confusing, and, if the server is not reachable, this should be the error.

This revision now requires changes to proceed.Sep 27 2023, 19:10

improve error message and store server url as a constant

Harbormaster completed remote builds in B25191: Diff 42423.Sep 28 2023, 06:57

Fabien requested changes to this revision.Sep 28 2023, 08:53

Fabien added inline comments.

electrum/electrumabc_gui/qt/contact_list.py
586 ↗	(On Diff #42423)	Empty string will be catch by the regex as well ? Or does it only run if the user writes something in the input field ?
609 ↗	(On Diff #42423)	You could gate on self.thread.isRunning() since the cancel event will wait for the thread to complete. This removes the need for this flag
616 ↗	(On Diff #42423)	dito

This revision now requires changes to proceed.Sep 28 2023, 08:53

PiRK added inline comments.Sep 28 2023, 11:23

electrum/electrumabc_gui/qt/contact_list.py
586 ↗	(On Diff #42423)	Empty string is always allowed in a QLineEdit, even if it does not please the validator. Otherwise you would never be able to completely delete what you typed in before changing it.