Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Show confirmations for addresses in wallet-tool and show UTXOs instead of individual addresses (Implements #676 and #259) #677

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Empty2k12
Copy link
Contributor

This pull requests add the ability to see confirmations for the addresses as requested in #676

The new output looks like this:

  m/0/0/1/000 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 used 1.00000001 btc 1000 confs 
  m/0/0/1/000 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 used 1.00000010 btc 900 confs 
  m/0/0/1/000 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 used 1.00000100 btc 800 confs 
  m/0/0/1/000 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 used 1.00001000 btc 700 confs 
  m/0/0/1/000 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 used 1.00010000 btc 600 confs 
  m/0/0/1/000 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 used 1.00100000 btc 500 confs 

For unused addresses it does not show anything.

@chris-belcher
Copy link
Collaborator

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

@Empty2k12
Copy link
Contributor Author

Empty2k12 commented Dec 3, 2016

What if an address contains more than one UTXO ? (because the address was reused)

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)

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.

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.
This could further incentivize users to switch to running a full node.

I think for this kind of thing #259 would be better

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.

@chris-belcher
Copy link
Collaborator

chris-belcher commented Dec 4, 2016

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

{u'0000000000000000000000000000000000000000000000000000000000000000:0': {'address': u'1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2',
                                                                         'value': 100000000,
                                                                         'blockheight': 410000},
 u'0000000000000000000000000000000000000000000000000000000000000000:0': {'address': u'1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2',
                                                                         'value': 100100000,
                                                                         'blockheight': 410100},
 u'0000000000000000000000000000000000000000000000000000000000000000:7': {'address': u'1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2',
                                                                         'value': 101000000,
                                                                         'blockheight': 411000}},

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)

In make_commitment() only the UTXOs already in the taker's own wallet must be queried, if the joinmarket wallet kept track of UTXO confirmations then that code could be modified to use wallet.unspent instead of query_utxo_set().

@Empty2k12
Copy link
Contributor Author

Empty2k12 commented Dec 6, 2016

I updated the PR to use blockchaininterface, show UTXO instead of address and confirmations for UTXOs

Now looks like:

mixing depth 0 m/0/0/
 external addresses m/0/0/0 XPUBHERE
  m/0/0/0/002 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 new 0.00000000 btc  
  m/0/0/0/003 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 new 0.00000000 btc  
  m/0/0/0/004 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 new 0.00000000 btc  
  m/0/0/0/005 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 new 0.00000000 btc  
  m/0/0/0/006 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 new 0.00000000 btc  
  m/0/0/0/007 1OoOOOOOooOoooOOo9Oo9o9OOo9oOoOOO2 new 0.00000000 btc  
 internal addresses m/0/0/1 
  m/0/0/1/025 o0o0o0o0ooo0o000o0o0o0o00o0o0o0o0o0o0o0o0o0o0o0o0o:0 used 0.00300000 btc 1125 confs 
  m/0/0/1/046 o0o0o0o0ooo0o000o0o0o0o00o0o0o0o0o0o0o0o0o0o0o0o0o:1 used 0.00100000 btc 597 confs 
  m/0/0/1/048 o0o0o0o0ooo0o000o0o0o0o00o0o0o0o0o0o0o0o0o0o0o0o0o:7 used 0.00700000 btc 573 confs 
  m/0/0/1/056 o0o0o0o0ooo0o000o0o0o0o00o0o0o0o0o0o0o0o0o0o0o0o0o:7 used 0.01000000 btc 171 confs 
for mixdepth=0 balance=0.03000000btc

@Empty2k12 Empty2k12 changed the title Show confirmations for addresses in wallet-tool (Implements #676) Show confirmations for addresses in wallet-tool (Implements #676 and #259) Dec 6, 2016
@Empty2k12 Empty2k12 changed the title Show confirmations for addresses in wallet-tool (Implements #676 and #259) Show confirmations for addresses in wallet-tool and show UTXOs instead of individual addresses (Implements #676 and #259) Dec 6, 2016
@Empty2k12 Empty2k12 changed the base branch from master to develop December 7, 2016 19:05
@Empty2k12
Copy link
Contributor Author

I should propably implement some caching for the block-height since simply running python wallet-tool.py wallet.json can cause the block height to be requested from Blockr/RPC around 50 times. This is unnessesarily much.

@chris-belcher
Copy link
Collaborator

chris-belcher commented Dec 26, 2016

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.

@chris-belcher
Copy link
Collaborator

Could you finish this please and then we can merge it?

@Empty2k12
Copy link
Contributor Author

@chris-belcher Will do when school work permits. (Most likely this evening (in 12+ hours))

@qubenix
Copy link

qubenix commented Mar 24, 2017

@Empty2k12 Any progress on this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants