search

mathieulavoie / Bitcluster

https://www.bit-cluster.com
MIT License
95 stars 28 forks source link

Fix multisig clusterization #16

Open shayanb opened 7 years ago

shayanb commented 7 years ago

Fix for https://github.com/mathieulavoie/Bitcluster/issues/15

mathieulavoie commented 7 years ago

Thanks, I'll include your pull request.

However, I may not include the multisig addresses in the clustering process by default : It will be configurable in the settings of the project.

Considering multisig can lead to invalid clustering assumptions, since it may merge two different node (or entity) sharing a multisig address together.

shayanb commented 7 years ago

@mathieulavoie Multisig addresses would not really invalidate Bitcluster cluttering assumptions in the way you described.

A multisig address is being controlled by multiple entities whom has their own private keys, however in order to spend from any multisig address it needs to have its own balance, independent from the key holders individual balances (One can have balance in the bitcoin address related to his private key but that does not affect the multisig address it controls).

The only scenario that it could result in invalidating clustering assumptions, is that one signs the regular (non-multisig) address UTXO with his key (using SIGHASH_NONE|SIGHASH_ANYONECANPAY) and then (partially) sign the multisig UTXO's and then send the result to the other entity to sign the multisig UTXO. I don't know of any clients or libraries which one can do this process.

In the other hand, a CoinJoin transaction can easily invalidate clustering assumptions and the tools are also available. but that is another problem to solve.

Also if Bitcluster does not cluster multisig addresses, it fails to cluster most of the current exchanges and more services in the future, as the usage of multisig addresses is rising. There's around 1.77 Million Bitcoins stored in multisig addresses at this time. (e.g Bitcluster failed to cluster bitfinex using the current database and also current Bitstamp cluster).

shayanb commented 7 years ago

@mathieulavoie actually as for BIP11 multisig transactions you are right, they would invalidate some of the clusterization assumptions. however the fix I made is to support multisig addresses which starts with 3 (P2SH), which would not invalidate any assumptions as they would be considered an entity (group, service, ... ).

one example of the old style (bip11) multisig address is here: https://www.blocktrail.com/BTC/tx/28dcf27290c1897210dcbe818370f502f9bf3f6de85c371aa11107544e0641bc

I think the above transaction would have failed to be clustered in current and patched version of bitcluster.

mathieulavoie commented 7 years ago

Hi,

Sorry for the delay of this response. I also think that P2SH transactions can be clustered the same way as P2PKH transactions. After your comment, I did some research about the MultiSig (Bip 11), I did poke some folks about the clustering of MultiSig and I came to the same conclusion as you.

The "if" condition that you remove was in fact an ugly but very effective optimization. On most of Pay to PubKey transactions, the Public Key is included in the SigScript. Instead of loading the previous output by calling the bitcoind rpc server and getting the hash160 from there, I can directly use the Public key from the SigScript and convert it to an btc address. I was able to optimize the crawling process by approximately x1000 time this way.

I pull request does not seem to have been change since the first commit (11 day ago). Do you want me to close this pull request so you can open a "clean" new one?

Lastly, I'm still trying to figure out a way of what can I do by analyzing MultiSig transactions. I consider of making an other association cluster to see if there is some addresses used in multiples Bip 11 transactions and "link" those transactions between them.(A same actor being part of multiples different MultiSig transactions) If you have any idea or suggestion about that, let me know.

Thanks, Mathieu