Skip to content

Commit

Permalink
Merge #1478: Refactor: Add get_wallet_rescan_status() instead of `g…
Browse files Browse the repository at this point in the history
…etwalletinfo()` for `bci`

e31e839 Add get_wallet_rescan_status() instead of getwalletinfo() for bci (Kristaps Kaupe)

Pull request description:

  Noticed this when tried to rebase #1462 after merging of #1477. #1461 added public `getwalletinfo()` method to `BitcoinCoreInterface`, which was used by code outside of `jmclient/jmclient/blockchaininterface.py`. This is bad approach, as it relies on Bitcoin Core RPC `getwalletinfo` returned `dict`, which contains a lots of different stuff too, could lead to more problems in future introducing other blockchain interface classes. Let's instead have generic method returning just wallet rescan status. Also it now returns `Tuple[bool, Optional[Decimal]]` with rescan status percentage, if rescan is in progress, although that's not used by any other code for now.

ACKs for top commit:
  AdamISZ:
    utACK e31e839 , very much agree with the thinking here.

Tree-SHA512: 2d8c9b8157847e713838099d0f62dfcd5321c9498cf8453a9087407e2cd9c32906739c8e71460fc6ac6426662d2ac501261080fea08388d928933f788bda9a8d
  • Loading branch information
kristapsk committed Apr 29, 2023
2 parents a5eb019 + e31e839 commit 2289fcb
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 14 deletions.
20 changes: 17 additions & 3 deletions jmclient/jmclient/blockchaininterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import random
import sys
import time
from typing import Optional
from typing import Optional, Tuple
from decimal import Decimal
import binascii
from twisted.internet import reactor, task
Expand Down Expand Up @@ -56,6 +56,11 @@ def estimate_fee_per_kb(self, N):
required for inclusion in the next N blocks.
'''

@abc.abstractmethod
def get_wallet_rescan_status(self) -> Tuple[bool, Optional[Decimal]]:
"""Returns pair of True/False is wallet currently rescanning and
Optional[Decimal] with current rescan progress status."""

def import_addresses_if_needed(self, addresses, wallet_name):
"""import addresses to the underlying blockchain interface if needed
returns True if the sync call needs to do a system exit"""
Expand Down Expand Up @@ -99,7 +104,7 @@ def __init__(self, jsonRpc, network, wallet_name):
if not wallet_name in loaded_wallets:
self._rpc("loadwallet", [wallet_name])
# We only support legacy wallets currently
wallet_info = self._rpc("getwalletinfo", [])
wallet_info = self._getwalletinfo()
if "descriptors" in wallet_info and wallet_info["descriptors"]:
raise Exception(
"JoinMarket currently does not support Bitcoin Core "
Expand Down Expand Up @@ -146,12 +151,21 @@ def rescan_in_thread(self, start_height: int) -> None:
log.error("Failure of RPC connection to Bitcoin Core. "
"Rescanning process not started.")

def getwalletinfo(self) -> dict:
def _getwalletinfo(self) -> dict:
""" Returns detailed about currently loaded (see `loadwallet`
call in __init__) Bitcoin Core wallet.
"""
return self._rpc("getwalletinfo", [])

def get_wallet_rescan_status(self) -> Tuple[bool, Optional[Decimal]]:
winfo = self._getwalletinfo()
if "scanning" in winfo and winfo["scanning"]:
# If not 'false', it contains info that looks like:
# {'duration': 1, 'progress': Decimal('0.04665404082350701')}
return True, winfo["scanning"]["progress"]
else:
return False, None

def _rpc(self, method, args):
""" Returns the result of an rpc call to the Bitcoin Core RPC API.
If the connection is permanently or unrecognizably broken, None
Expand Down
6 changes: 1 addition & 5 deletions jmclient/jmclient/wallet_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -641,11 +641,7 @@ def session(self, request):

if self.services["wallet"]:
if self.services["wallet"].isRunning():
winfo = self.services["wallet"].get_backend_walletinfo()
if "scanning" in winfo and winfo["scanning"]:
# Note that if not 'false', it contains info
# that looks like: {'duration': 1, 'progress': Decimal('0.04665404082350701')}
rescanning = True
rescanning, _ = self.services["wallet"].get_backend_wallet_rescan_status()
wallet_name = self.wallet_name
# At this point if an `auth_header` is present, it has been checked
# by the call to `check_cookie_if_present` above.
Expand Down
9 changes: 3 additions & 6 deletions jmclient/jmclient/wallet_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import itertools
import time
import sys
from typing import Optional
from typing import Optional, Tuple
from decimal import Decimal
from copy import deepcopy
from twisted.internet import reactor
Expand Down Expand Up @@ -733,11 +733,8 @@ def get_block_height(self, blockhash):
def rescanblockchain(self, start_height: int, end_height: Optional[int] = None) -> None:
self.bci.rescanblockchain(start_height, end_height)

def get_backend_walletinfo(self) -> dict:
""" 'Backend' wallet means the Bitcoin Core wallet,
which will always be loaded if self.bci is init-ed.
"""
return self.bci.getwalletinfo()
def get_backend_wallet_rescan_status(self) -> Tuple[bool, Optional[Decimal]]:
return self.bci.get_wallet_rescan_status()

def get_transaction_block_height(self, tx):
""" Given a CTransaction object tx, return
Expand Down

0 comments on commit 2289fcb

Please sign in to comment.