Skip to content
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

split out network reqs from wallet.add_input_info, and add "Download input data" cb in qt tx dlg #8241

Merged
merged 3 commits into from
Mar 12, 2023

Conversation

SomberNight
Copy link
Member

@SomberNight SomberNight commented Mar 12, 2023

See commit messages for the individual commits.


Re the Qt tx dialog, here are some pics for tx eec306392190ddf956c3a233c764ef17fa4ded81540de9e3e8565ab7edd755df:

pics

pic1
pic2
pic3

SomberNight and others added 3 commits March 12, 2023 00:21
- wallet.add_input_info() previously had a fallback to download parent
  prev txs from the network (after a lookup in wallet.db failed).
  wallet.add_input_info() is not async, so the network request cannot
  be done cleanly there and was really just a hack.
- tx.add_info_from_wallet() calls wallet.add_input_info() on each txin,
  in which case these network requests were done sequentially, not concurrently
- the network part of wallet.add_input_info() is now split out into new method:
  txin.add_info_from_network()
- in addition to tx.add_info_from_wallet(), there is now also tx.add_info_from_network()
  - callers of old tx.add_info_from_wallet() should now called either
    - tx.add_info_from_wallet(), then tx.add_info_from_network(), preferably in that order
    - tx.add_info_from_wallet() alone is sufficient if the tx is complete,
      or typically when not in a signing context
- callers of wallet.bump_fee and wallet.dscancel are now expected to have already
  called tx.add_info_from_network(), as it cannot be done in a non-async context
  (but for the common case of all-inputs-are-ismine, bump_fee/dscancel should work regardless)
- PartialTxInput.utxo was moved to the baseclass, TxInput.utxo
ported from https://github.com/Electron-Cash/Electron-Cash/blob/e8bbf8280ccaef95c10cdb1c1cb182cd4b504937/electroncash_gui/qt/util.py
(originally added in Electron-Cash@8b8d8a5 )

Co-authored-by: Calin Culianu <calin.culianu@gmail.com>
Co-authored-by: SomberNight <somber.night@protonmail.com>
If checked, we download prev (parent) txs from the network, asynchronously.
This allows calculating the fee and showing "input addresses".

We could also SPV-verify the tx, to fill in missing tx_mined_status
(block height, blockhash, timestamp, short ids), but this is not done currently.
Note that there is no clean way to do this with electrum protocol 1.4:
`blockchain.transaction.get_merkle(tx_hash, height)` requires knowledge of the block height.

Loosely based on Electron-Cash@6112fe0
@SomberNight SomberNight added topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py gui-qt-desktop labels Mar 12, 2023
@ecdsa
Copy link
Member

ecdsa commented Mar 12, 2023

it looks cleaner that way :-)

@ecdsa ecdsa merged commit f87cac0 into spesmilo:master Mar 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gui-qt-desktop topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants