Remove ElectrumWalletInterface
#1477
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
conceptACK Will double check the code when I get a chance |
|
Sanity checked that there are no remaining references to Also manually ran test of tACK 3805c7a Merging. |
kristapsk
added a commit
that referenced
this pull request
Apr 29, 2023
…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
kristapsk
added a commit
that referenced
this pull request
Sep 8, 2023
3fc74fb Refactor and cleanup of blockchaininterface and related (Kristaps Kaupe) Pull request description: Summary of changes: * Add typehints to `jmclient/jmclient/blockchaininterface.py` and `jmclient/jmclient/jsonrpc.py`; * Move all methods called by external code to `BlockchainInterface` base class or add abstract methods there; * <del>Remove broken `ElectrumWalletInterface` (we can re-introduce it later from history if somebody wants to fix it);</del><ins> (done in #1477)</ins> * More dummy abstract method overrides for `DummyBlockchainInterface` (for tests); * Alphabetical ordering of imports and other minor stuff; * Behaviour change - previously fee estimation would fail if it could not get mempoolminfee, now will go with default 10 sat/vB. #1460 was part of this, but did separate PR for that one, as it is simpler to review small refactoring changes. This should make it more easy to: 1) write new code that uses blockchaininterface, 2) write new alternative implementations of `BlockchainInterface`. Top commit has no ACKs. Tree-SHA512: 7699de0419c1006ff3c6ac5da7d26055b8e3508c424b5bdd3117fea575d30ab21e8617b2d50ae7c8b2e76997d6f4b14781bb56fcdad4efc081d533a713666a31
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove broken
ElectrumWalletInterface(we can re-introduce it later from history if somebody wants to fix it).Part of splitting #1462 into smaller PRs for easier testing and reviews.