-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle just in time channels in qml request creation #9606
base: master
Are you sure you want to change the base?
Conversation
electrum/gui/qml/qewallet.py
Outdated
zeroconfAvailabilityChanged = pyqtSignal() | ||
@pyqtProperty(bool, notify=zeroconfAvailabilityChanged) | ||
def canGetZeroconfChannel(self) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The convention is to name the signal the same as the property if it's used exclusively for that property, e.g. canGetZeroconfChannel
property -> canGetZeroconfChannelChanged
signal.
Also note that this property is evaluated the moment ReceiveDetailsDialog
is constructed, but not updated later if the zeroconf peer connects/disconnects. The signal should ideally be emitted if the zeroconf peer connected state changes, to trigger re-evaluation of the binding in qml. However, I don't think this is cricital for this PR, as ReceiveDetailsDialog
is usually short-lived, but if the user keeps it open for longer, the likelyhood of the connected state getting outdated increases. (also, there are currently no callbacks defined for peer connection state changes that you could use here..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hint, changed the naming in 7a073e6.
electrum/gui/qml/qerequestdetails.py
Outdated
can_receive = wallet.lnworker.num_sats_can_receive() if wallet.lnworker else 0 | ||
will_req_zeroconf = wallet.lnworker.receive_requires_jit_channel(amount_msat=amount_sat*1000) | ||
if self._req and ((can_receive > 0 and amount_sat <= can_receive) | ||
or (will_req_zeroconf and amount_sat >= 200_000)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hardcoded 200_000 limit should probably be defined in a symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 7a073e6, using the MIN_FUNDING_SAT from lnutil now as it serves the same purpose
electrum/lnworker.py
Outdated
return False | ||
try: | ||
node_id = extract_nodeid(self.wallet.config.ZEROCONF_TRUSTED_NODE)[0] | ||
except Exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can catch ConnStringFormatError
instead here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 7a073e6
// show error that amnt > 200k is neccessary to get zeroconf channel | ||
var confirmdialog = app.messageDialog.createObject(dialog, { | ||
title: qsTr('Amount too low'), | ||
text: qsTr("You don't have channels with enough inbound liquidity to receive ") | ||
+ qsTr("this payment. Request at least 200 000 sat to open a channel just-in-time."), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text here should be parameterized as well, e.g.
qsTr('%1 sat').arg(Config.formatSats(Daemon.currentWallet.minChannelFundingSat))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx, missed that, fixed in 257b63c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, my short hint was a bit confusing, I added a commit. This makes the strings better translatable (i.e. no space suffix, and don't split paragraphs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, #9598 also has the hardcoded amount in the help text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx
electrum/gui/qml/qewallet.py
Outdated
@@ -505,6 +512,10 @@ def lightningCanReceive(self): | |||
self._lightningcanreceive.satsInt = int(self.wallet.lnworker.num_sats_can_receive()) | |||
return self._lightningcanreceive | |||
|
|||
@pyqtProperty(int, notify=dataChanged) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem here, as MIN_FUNDING_SAT
will probably always fit 32 bit unsigned integer, but FYI:
See 7d8a5cc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good to know, thx. I changed it to QEAmount
in 257b63c to have it consistent with the other methods, won't hurt at least.
a4ccf52
to
92bcd4e
Compare
Allows to create invoices in QML gui if client has configured trusted zeroconf channels. Shows according warning when a channel is going to get opened or when the amount is too low to receive the payment.

