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

Restore support for pre-0.17 Bitcoin Core in wallet #425

Merged
merged 1 commit into from
Nov 3, 2019
Merged

Restore support for pre-0.17 Bitcoin Core in wallet #425

merged 1 commit into from
Nov 3, 2019

Conversation

kristapsk
Copy link
Member

@kristapsk kristapsk commented Oct 30, 2019

With #359 changes, some places in code now assumes UTXO's has labels. But that's not true with Bitcoin Core 0.16.3, it still uses accounts, labels were introduced with 0.17. Don't think dropping support for 0.16 is a good idea currently, as it is stable default for some Linux distros, for example, Gentoo.

It fixes wallet-tool for me, but still needs more testing, both with 0.16.3 and 0.17+.

@AdamISZ
Copy link
Member

AdamISZ commented Oct 31, 2019

Thank you. This appears to have been an oversight on my part (I remember leaving a certain part of the code in bci as-is to avoid accidentally breaking versions of Core, but forgot I was adding code here which only allowed labels ... that's what I vaguely remember, anyway).

@AdamISZ
Copy link
Member

AdamISZ commented Oct 31, 2019

I think it'd be a good idea to have a function like 'is_labeled(tx, walletname)' put in blockchaininterface.py; then you could call that multiple times and, in future, if something else changes about labels/names/accounts, including new blockchain interface setups, it'd be abstracted out into one place there and wallet_service wouldn't have to care any more.

@kristapsk
Copy link
Member Author

Moved label checks to blockchain interface.

@kristapsk kristapsk changed the title [WIP] Restore support for pre-0.17 Bitcoin Core in wallet Restore support for pre-0.17 Bitcoin Core in wallet Nov 3, 2019
@kristapsk
Copy link
Member Author

Have tested this successfully as a maker, taker and with wallet-tool, haven't noticed any problems.

@@ -31,6 +31,10 @@ def is_address_imported(self, addr):
except JsonRpcError:
return len(self.rpc('getaddressinfo', [addr])['labels']) > 0

@abc.abstractmethod
def is_address_labeled(self, utxo, walletname):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an argument that this may not be needed for some interfaces.

But, one can always address that with dummy functions, so this is fine.

AdamISZ added a commit that referenced this pull request Nov 3, 2019
cf19df2 Restore account support in wallet_service, needed for pre-0.17 Bitcoin Core (Kristaps Kaupe)
@AdamISZ AdamISZ merged commit cf19df2 into JoinMarket-Org:master Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants