Bitcoind rpc wrapper

[kristovatlas]

Adds wrapper for bitcoind's RPC interface.

This can be used instead of the remote API by passing the --rpc switch to ludwig.py and setting the appropriate environment variables.

There are some transactions that this cannot handle (e.g. pre-BIP30 transactions, genesis block coinbase) but that’s already broken for bci_wrapper so I didn’t try to fix things.

To ensure compatibility, I added a test class which compares wrapper output for 2 sample transactions and the capacity to add more easily.

[LaurentMT]

I've done a quick review. It seems to work fine for the cases covered with the bc.i api.

Here are a few remarks :

BitcoindRPCWrapper._get_decoded_tx()

You can replace this line

return self._con.decoderawtransaction(self._get_raw_tx(txid))

by

return self._con.getrawtransaction(txid, 1)

It returns the information needed and it removes an extra call to the rpc api.

BitcoindRPCWrapper._get_block_height()

Considering that the block height isn't required for the computation of the metrics and that the method will often return a warning because the outputs were consumed, I think it should just initialize the block height to -1 (i.e. information isn't supported by the wrapper).

An other option is simply removing the "height" attribute from Transaction and BlockchainInfoWrapper. That may make sense considering that I kept this information because I had it for free from bc.i but I never use it (even when displaying the results). Note that the same remark applies to the "time" attribute.

Management of multisig scripts (and others fancy scripts)

Good catch ! In order to solve this problem, you can replace

rpc_output['scriptPubKey']['addresses'][0]

by

' '.join(rpc_output['scriptPubKey']['addresses'])

Ironically, I've got the same problem with bc.i but I can't use the same trick because bc.i doesn't return any address for raw multisigs.

Anyway, this solution isn't going to work for scripts which don't have an address returned by the api and that may become a problem with segwit transactions.

So far, my "best" idea is to replace the address by the raw scriptpubkey if no address is returned by the API. It may become ugly for some fancy scripts when results are displayed but that should work fine for computations.

What do you think about it ?

[kristovatlas]

return self._con.getrawtransaction(txid, 1)

Nice, thank you.

Considering that the block height isn't required for the computation of the metrics and that the method will often return a warning because the outputs were consumed, I think it should just initialize the block height to -1 (i.e. information isn't supported by the wrapper).

An other option is simply removing the "height" attribute from Transaction and BlockchainInfoWrapper. That may make sense considering that I kept this information because I had it for free from bc.i but I never use it (even when displaying the results). Note that the same remark applies to the "time" attribute.

Both of those make sense to me. I somewhat like the idea of leaving information we get for free (e.g. block height and time from BCI wrapper) in, in case you decide to make use of it later on in the project's lifespan, but we could make it clear which transaction fields are optional for wrappers to implement.

' '.join(rpc_output['scriptPubKey']['addresses'])

Ironically, I've got the same problem with bc.i but I can't use the same trick because bc.i doesn't return any address for raw multisigs.

I like this idea for the rpc wrapper, but wonder what the ramifications will be down the line for having two wrappers that return different, non-null results. :/

Anyway, this solution isn't going to work for scripts which don't have an address returned by the api and that may become a problem with segwit transactions.

I'm not too worried about that -- I think the BCI API will work similarly for segwit txs if segwit is ever adopted.

So far, my "best" idea is to replace the address by the raw scriptpubkey if no address is returned by the API. It may become ugly for some fancy scripts when results are displayed but that should work fine for computations.

Yes, this makes sense to me as a fallback when no address is decodable. For outputs, we could use bci's ['out'][i]['script'] field and the rpc's ['vout'][i]['scriptPubKey']['hex'] field. For inputs, bci's ['inputs'][i]['prev_out']['script']; rpc doesn't expose this for inputs using getrawtransaction, this, so on these unusual occasions we'd need to do one additional rpc call to getrawtransaction for ['vin'][i]['txid'].

[LaurentMT]

Both of those make sense to me. I somewhat like the idea of leaving information we get for free (e.g. block height and time from BCI wrapper) in, in case you decide to make use of it later on in the project's lifespan, but we could make it clear which transaction fields are optional for wrappers to implement.

Ok. Let's keep these information. I'm going to modify #6

I like this idea for the rpc wrapper, but wonder what the ramifications will be down the line for having two wrappers that return different, non-null results. :/

It won't have any effect on the computations. Effects will be limited to the presentation of the results. In the case of a raw multisig, results based on data provided by bc.i api will be displayed with the scriptpubkey associated to the multisig, those based on the rpc api will be displayed with the multiple addresses.

Yes, this makes sense to me as a fallback when no address is decodable. For outputs, we could use bci's ['out'][i]['script'] field and the rpc's ['vout'][i]['scriptPubKey']['hex'] field. For inputs, bci's ['inputs'][i]['prev_out']['script']; rpc doesn't expose this for inputs using getrawtransaction, this, so on these unusual occasions we'd need to do one additional rpc call to getrawtransaction for ['vin'][i]['txid'].

Deal. Let's go for this fix.

[LaurentMT]

Code from this PR + modifications discussed here are included in PR #7 for merge with master.