-
Notifications
You must be signed in to change notification settings - Fork 119
Show confirmations for addresses in wallet-tool and show UTXOs instead of individual addresses (Implements #676 and #259) #677
base: develop
Are you sure you want to change the base?
Conversation
What if an address contains more than one UTXO ? (because the address was reused) Also what if the user is using blockr.io as a blockchain interface instead of bitcoin core? And if we code up new blockchain interfaces it also won't work. I think for this kind of thing #259 would be better |
I thought about that, too but since address reuse is discouraged this seems like the best solution going forward (especially considering the future implementation of #259)
This is something I did not consider, however. I have been thinking a little bit about this and came to the conclusion that this feature should only be activated when Bitcoin Core is being used for querying the blockchain. Maybe there is a way to integrate this with block explorers as well when they are returning the confirmations for a address (or more specifically the first transaction in the address) with the query, but I have not been able to come up with a data format that allows loading Confs from the server and from Core since the requests are so fundamentally different. Additionally I think this should NOT query a block explorer for every Address. If the data is returned in the initial request we can use it, if not the servers shouln't be spammed with unesseccay requests.
What do you mean spcifically by that? I have been thinking about implementing #259 for a while, I can try to tackle that next if you want. |
What I meant by #259 is that since it would show individual UTXOs then it would make sense to show the number of confirmations there, instead of next to addresses which might have more than one UTXO on them. It would be fine to skip support for blockr.io, but one day when #653 and #470 are implemented then they also won't work. I agree blockr.io and querying web blockchain explorers was a really bad idea which is why I'm thinking about how #653 and #470 would fit into joinmarket. This is the reason that the BlockchainInterface class exists, to abstract away the blockchain/bitcoin network code from everything else. So I'd say the best way going forward is to put the confirmation code inside BlockchainInterface. Do this by having sync_unspent() in all the BlockchainInterfaces also record the block in which the UTXO was mined into. So the unspent dict would look like this
Then a new method in BlockchainInterface could return the current block height, from which confirmation count can be calculated from for each UTXO. This would also be needed in the solution to this problem #470 (comment)
|
…ckchaininterface)
I updated the PR to use blockchaininterface, show UTXO instead of address and confirmations for UTXOs Now looks like:
|
I should propably implement some caching for the block-height since simply running |
Since wallet-tool is the only place that issue comes up, it's probably better to only call get_block_count() once at the beginning of the 'display' function in wallet-tool and store the result in a local variable. |
Could you finish this please and then we can merge it? |
@chris-belcher Will do when school work permits. (Most likely this evening (in 12+ hours)) |
@Empty2k12 Any progress on this? |
This pull requests add the ability to see confirmations for the addresses as requested in #676
The new output looks like this:
For unused addresses it does not show anything.