rust-bitcoin / rust-bitcoin Goto Github PK

I think all the use of unsafe in this library is pre-1.0 stuff that can be made safe now. We should do that.

I think most of patricia_tree.rs could be used by other projects.
Could you move it to a new crate, so they can use it without depending on rust-bitcoin?

Currently apparently serialization of Option types is supported by consensus encoding and decoding. When done unintentionally, the compiler will not warn you and you'll get an ParseFailed("Invalid length") error.

It might be worth reconsidering if having this functionality (1) make sense (I don't think there is "consensus" about encoding Rust Option types) and (2) has value to have it in the library.

There has been discussions over IRC, and I'd like to move the discussion here for easier access.

History

Historically, we've stuck with rustc 1.14.0 because it was the min over the versions packaged in Debian stable on all architectures.

Delta

Currently, Debian stable has packaged rustc 1.24.1 for all architectures.

Proposal

1. Upgrade our rustc version assumption to 1.20.0 (release notes) now, so that we get associated constants. This will allow us to simplify a lot of our interfaces.
2. Determine a checklist of requirements for future proposals to upgrade rustc.

In APIs like sign_raw_transaction taking HexBytes is not very convenient. The caller will either have Transaction or String or Vec<u8> and only the last one is somewhat easily convertible. In none of the APIs we actually return HexBytes, and the fact that it is serializing to hex string, should be an implementation detail.

I think we should have be taking tx: T where T: AsRef<HexBytes> or something like that, so we can impl AsRef<HexString> for Transaction, String and Vec<u8> and &[u8].

We could potentially use the same technique in other places in which it's unclear which type exactly to return. Eg we could return T : From<TxString> and then implement it for String, Vec<u8> and Transaction. The downside of it, is that in some cases, it will be unclear which type is to be returned and user will have to do the turbofish.

Ping @stevenroose

I want to write a program that outputs all the Bitcoin addresses that have ever been used in the Bitcoin blockchain. It does not matter if the program outputs duplicates.

From where do I get the blocks? I note that I can't use the blockchain downloaded by Bitcoin core https://github.com/apoelstra/rust-bitcoin/issues/20 Would it be much work to support the .dat files? My understanding was that they are just blocks concatenated together?

This doesn't even properly validate PoW, let alone "SPV", as it's simply a single-block validation.

A better name would be validate_target(), as it validates that the required target has been met. To fix this I'd suggest that we just change the name, and add a #inline(always) wrapper for the old name that's marked depreciated.

It only happens on i686. Not sure how to proceed.

warning: literal out of range for usize
   --> src/blockdata/script.rs:525:22
    |
525 |             n if n < 0x100000000 => {
    |                      ^^^^^^^^^^^
    |
    = note: #[warn(overflowing_literals)] on by default
    = note: the literal `0x100000000` (decimal `4294967296`) does not fit into an `usize` and will become `0usize`

Offending code

I think it's useful to have a type that represents bitcoin amounts, currently there isn't a standard way to do this in rust and there isn't too much documentation on how developers should handle BTC amounts (e.g. wrongly using f64 or f32instead of using fixed point math).

Ideally it would be called Amount and live under the util::amount module.

Some features it should implement:

• implement serde Deserialize/Serialize traits.
• it should have a function to construct amounts from strason::Json numbers (for bitcoind JSON-RPC).
• implement rust math operation traits (Add, Mul, Sub, Div, etc).
• wrapping operations (wrapping_add, wrapping_sub, etc).
• pretty printing (e.g printing "0.00025587 BTC" or "25587 sat").
• MIN/MAX constants.

I think we should reexport many types at the top level so that people can type bitcoin::Address or bitcoin::Block, etc. The reason is that if you want to use, say, TxIn, currently you're forced to either type the full path or do blockdata::transaction::TxIn and have a TxIn type which is easy to confuse with other blockchains' TxIn types. It's also hard to remember the full path for a lot of these things, since the module structure was designed for code separation rather than producing a sensible API.

This is a tracking issue where I want to get a complete list of everything we should expose this way. Here is my list:

• blockdata::block::Block
• blockdata::block::BlockHeader
• blockdata::script::Script
• blockdata::transaction::Transaction
• blockdata::transaction::TxIn
• blockdata::transaction::TxOut
• blockdata::transaction::OutPoint
• blockdata::transaction::SigHashType
• network::encodable::VarInt
• network::serialize::BitcoinHash
• util::Error
• util::address::Address
• util::privkey::Privkey
• util::decimal::Decimal
• util::decimal::UDecimal

You can grep the source code for pub struct and pub trait and pub enum to see a full list of exposed data structures throughout the tree. Comments are welcome about anything you feel should be added/removed.

Notably missing are

• Anything in util::bip32 or util::bip143 modules, since I think it's perfectly clear to import the module and use paths like bip32::ChainCode.
• Anything in util::hash because eventually these structures will be moved into the bitcoin_hashes crate where they can be top-level.
• most network::serialize and network::encodable stuff, since I want to restructure these modules in a future major release.

Hello, license makes it look like I need to get permission to fork the code?

I would like to build a blockchain in Rust for my altcoin, could I start of with this code?

Happy to provide details.

Thanks!

Perhaps we should find an alternative CI provider as Travis seems to be:

1. Extremely slow
2. Have more-than-intermittent network issues

Need an implementation of the sighash for segwit.

I think this is needed for accountability/clarity, and they should be signed.

I'd be happy to create a set.

It'd be beneficial for people to not have to download and compile whole crate if they only need to work with part (e.g. transactions).

I'm willing to help with that.

https://www.wpsoftware.net/rustdoc/bitcoin/

the link is broke,

can possible host on github websites?

i thought mioco/mio is "the" io lib for rust?! We should maybe consider using it instead of the blocking std::net module.

Hi,

In case these could be of use:

https://github.com/dpc/bitcoin-secp256k1-rs

How do I extract the addresses from a Script?

I can only find an example of putting an address into a Script:

fn script_builder() {
        // from txid 3bb5e6434c11fb93f64574af5d116736510717f2c595eb45b52c28e31622dfff which was in my mempool when I wrote the test
        let script = Builder::new().push_opcode(opcodes::All::OP_DUP)
                                   .push_opcode(opcodes::All::OP_HASH160)
                                   .push_slice(&"16e1ae70ff0fa102905d4af297f6912bda6cce19".from_hex().unwrap())
                                   .push_opcode(opcodes::All::OP_EQUALVERIFY)
                                   .push_opcode(opcodes::All::OP_CHECKSIG)
                                   .into_script();
        assert_eq!(&format!("{:x}", script), "76a91416e1ae70ff0fa102905d4af297f6912bda6cce1988ac");
    }

Just Do It™

This would allow serializing into a fmt::Formatter without needing temporary allocations.

After fixing this we should also check the fmt::Display impls that use base58 encoding to make sure they're using the underlying base58ck function rather than serializing to a string then copying.

as title.

It line of code

deserialization:

It seems that bip32 doesn't describe regtest, but I think we can use any four bytes(probably send PR to bip32).

I'm fairly new at working with Bitcoin, and I've just discovered that consensus encoding and rpc encoding use different endianess. Nothing that I can't fix, now that I know about it, but maybe this library should include rpc encoding routines as well - first it would make the educational distinction between the two, second - it is useful and not a lot of code to add.

I'm not sure how much of the education burden rust-bitcoin take upon itself, but also being more explicit about which representation operations like Display trait, https://docs.rs/bitcoin/0.13.2/bitcoin/util/hash/struct.Sha256dHash.html#method.data , and similiar string/bytes conversions seems like a good idea.

Related to #134 , there could be RpcEncoder and RpcDecoder that take care of these. Ping @dongcarl , since you seem to be working on it.

After receiving a message, the loop in listener.rs immediately relocks the socket on the following iteration. This blocks requests to send responses, like a pong to a ping, instead delaying the sending of the response until the socket is free again, which doesn't happen until the next message is received. If no other messages are received between two pings, the pong sent will reference the nonce of the earlier ping, which doesn't seem right. Should that be fixed, or am I just missing something?

Asking the listening thread to sleep for just a millisecond before the call to sock.receive_message() allows responses to be sent immediately.

There is an official base58 encoding for private keys (aka wallet import format). Currently neither base58 can be accessed publicly neither does the Privkey type support serializing to WIF/base58.

This probably should be done after #125.

Hi!

Took me some time to understand why I had the issue. I have not yet investigated in rust-bitcoin code to understand the issue/propose a fix yet.

Raw transactions which do not include inputs are not properly handled.
This test fails on latest rust-bitcoin/master

#[test]
fn test_transaction_no_vin() {
    let hex_tx = hex_bytes("02000000000100ca9a3b00000000160014716f8e8fa281cb42bb6900ce35b73a13fa3e66e000000000").unwrap();
    let tx: Result<Transaction, _> = deserialize(&hex_tx);
    assert!(tx.is_ok());
}

If you replace the assert with tx.unwrap() you would get the following stack:

thread 'blockdata::transaction::tests::test_transaction_no_vin' panicked at 'called `Result::unwrap()` on an `Err` value: ByteOrder(Custom { kind: UnexpectedEof, error: StringError("failed to fill whole buffer") })', libcore/result.rs:945:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::get
             at libstd/panicking.rs:463
   5: std::panicking::try::do_call
             at libstd/panicking.rs:350
   6: std::panicking::try::do_call
             at libstd/panicking.rs:328
   7: <&'a T as core::fmt::Display>::fmt
             at libcore/panicking.rs:71
   8: core::result::unwrap_failed
             at /Users/travis/build/rust-lang/rust/src/libcore/macros.rs:26
   9: <core::result::Result<T, E>>::unwrap
             at /Users/travis/build/rust-lang/rust/src/libcore/result.rs:782
  10: bitcoin::blockdata::transaction::tests::test_transaction_no_vin
             at src/blockdata/transaction.rs:934
  11: bitcoin::__test::TESTS::{{closure}}
             at src/blockdata/transaction.rs:931
  12: core::ops::function::FnOnce::call_once
             at /Users/travis/build/rust-lang/rust/src/libcore/ops/function.rs:223
  13: <F as alloc::boxed::FnBox<A>>::call_box
             at libtest/lib.rs:1451
             at /Users/travis/build/rust-lang/rust/src/libcore/ops/function.rs:223
             at /Users/travis/build/rust-lang/rust/src/liballoc/boxed.rs:638
  14: panic_unwind::dwarf::eh::read_encoded_pointer
             at libpanic_unwind/lib.rs:105
test blockdata::transaction::tests::test_transaction_no_vin ... FAILED

Please note that bitcoind is able to decode the said transaction:
Request

{
    "jsonrpc": "1.0", 
    "id":"curltest", 
    "method": "decoderawtransaction", 
    "params": 
["02000000000100ca9a3b00000000160014716f8e8fa281cb42bb6900ce35b73a13fa3e66e000000000"
    ]
    
}

Response

{
    "result": {
        "txid": "e8069d1704d4d194f3fd6c85081851a8bc093a9f03da1be75065b961ddee1572",
        "hash": "e8069d1704d4d194f3fd6c85081851a8bc093a9f03da1be75065b961ddee1572",
        "version": 2,
        "size": 41,
        "vsize": 41,
        "locktime": 0,
        "vin": [],
        "vout": [
            {
                "value": 10,
                "n": 0,
                "scriptPubKey": {
                    "asm": "0 716f8e8fa281cb42bb6900ce35b73a13fa3e66e0",
                    "hex": "0014716f8e8fa281cb42bb6900ce35b73a13fa3e66e0",
                    "reqSigs": 1,
                    "type": "witness_v0_keyhash",
                    "addresses": [
                        "tb1qw9hcarazs8959wmfqr8rtde6z0aruehqt6m3m7"
                    ]
                }
            }
        ]
    },
    "error": null,
    "id": "curltest"
}

Looking at struct Transaction it does not look like input must be present so at this stage I am thinking about a bug in the deserialize function.

2 proposed way forward:

1. Support transaction without inputs
2. Improve the failure message to make it easier to debug

Let me know how you would prefer to move forward and I may be able to look into a patch.

I am doing blockchain indexing and when analyzing perf output of my indexer (which is quite CPU-bound at this point), I can see 10% of CPU time sitting in hex_bytes. I query blocks from the node, which get decoded from hex response to Vec<u8> and then to Block via consensus_decode. All of this happens in rust-bitcoincore-rpc https://github.com/rust-bitcoin/rust-bitcoincore-rpc/blob/master/client/src/queryable.rs#L23

Looking at:

https://github.com/rust-bitcoin/rust-bitcoin/blob/master/src/util/misc.rs#L24

I see the following room for improvement:

• output vector's capacity could be preallocated (should be typically 1/2 of the string length)
• chars is for handling Unicode chars, so I suspect there's quite a bit of overhead there. We can assume that the hex string is ASCII, and just convert it to &[u8] directly via as_bytes() and work on that; unicode characters are > 128 so it all should be fine;
• the whole iterator-based approach seems suspicious, performance-wise; pair, fold; who knows how it gets optimized; a simpler, imperative code would be both more readable, and maybe faster

Could probably rely on https://github.com/clarkmoody/rust-bech32 for the bech32-parsing side of things (yay not even any deps in it!).

Below is an extract of the log of rust-bitcoin-spv:

2018-03-07 13:05:36 INFO [bitcoin_spv::node] loading headers from database...
2018-03-07 13:05:36 INFO [bitcoin_spv::node] reading headers ...
2018-03-07 13:05:59 INFO [bitcoin_spv::node] building in-memory header chain ...
2018-03-07 13:07:38 INFO [bitcoin_spv::node] loaded 512418 headers from database

It shows that reading 512418 headers from disk with the SQLite database takes 23 secs but inserting them into the in-memory blockchain database takes 1 minutes and 39 seconds. This seems very slow in comparison.

Serde (well, really just serde-json for now) now has arbitrary-precision numerics support in its latest release! 🎊 It might thus be time to migrate to using the latest version of serde, removing the dependency on strson, and thus bringing everything up-to-date?

You may also be interested in knowing that I'm currently working on an IEEE floating-point decimal implementation for Rust, which will support serde arbitrary-precision numerics.

trait SimpleDecoder and trait SimpleEncoder seem unnecessary and generate quite a bit of generic noise elsewhere in the code. Seem like candidates for removal, by YAGNI principle,

SimpleDecoder has only one type implementing it, and it doesn't seem like there could be any use for more (except for testing/mocking maybe, I doubt it...).

SimpleEncoder has two, but the Sha256dEncoder looks quite coincidental. All it does is forwarding everything to an hash input function after little endian conversion which, seems to me, could be done more elegantly by using the fact that RawEncoder can encode into a io::Write type (and it already does LE conversion too).

I might be mistaken, but struct ConsensusDecoder (currently RawDecoder, but withoutbut without a Simple... trait) and struct ConsensusEncoder (currently RawEncoder but without a Simple... trait).

Please let me know your thoughts on this.

I think in the next major release we should fix the module names for the consensus encodable/decodable traits. Specifically I propose removing the serialize, encodable and consensus_params modules

• network::encodable::VarInt -> consensus::encode::VarInt and expose it at the crate level
• network::encodable::VarInt -> consensus::encode::CheckedData
• network::consensus_params -> consensus::params
• network::consensus_params::ConsensusParams -> consensus::params::Params
• network::encodable::ConsensusEncodable -> consensus::encode::Encodable but expose it as consensus::Encodable
• network::encodable::ConsensusDecodable -> consensus::encode::Decodable but expose it as consensus::Decodable
• network::seriailize::Error -> consensus::encode::Error
• network::serialize::BitcoinHash -> util::hash::BitcoinHash and expose it at crate level
• network::serialize::RawEncoder -> consensus::encode::RawEncoder and expose it as consensus::RawEncoder
• network::serialize::RawDecoder -> consensus::encode::RawDecoder and expose it as consensus::RawDecoder
• network::serialize::SimpleEncoder -> consensus::encode::Encoder and expose it as consensus::Encoder
• network::serialize::SimpleDecoder -> consensus::encode::Decoder and expose it as consensus::Decoder

Finally, we should deprecate everything in network::socket and network::listener because low-level networking is out of scope for this library. That leaves only p2p network messages under network, which is good. (Though we should also rename/restructure the modules there - I'll open yet another issue about that.)

Unrelatedly, we should also delete nu_select from macros.rs because this is horribly bitrotted and I doubt it even compiles anymore.

Because the network enum adds a new regtest constant that affects the whole code that uses pattern matching with the Network constant.
For example
https://github.com/exonum/exonum-btc-anchoring/blob/ab3b22c6dbc117b0cc8115b978aa0973fc22703a/src/blockchain/consensus_storage.rs#L134

   Compiling bitcoin v0.14.1 (file:///home/user/bitcoin/rust-bitcoin)
error[E0277]: the trait bound `rand::ThreadRng: rand::Rng` is not satisfied
   --> src/util/contracthash.rs:346:31
    |
346 |         let (sk1, pk1) = secp.generate_keypair(&mut thread_rng());
    |                               ^^^^^^^^^^^^^^^^ the trait `rand::Rng` is not implemented for `rand::ThreadRng`
error[E0277]: the trait bound `rand::ThreadRng: rand::Rng` is not satisfied
   --> src/util/contracthash.rs:347:31
    |
347 |         let (sk2, pk2) = secp.generate_keypair(&mut thread_rng());
    |                               ^^^^^^^^^^^^^^^^ the trait `rand::Rng` is not implemented for `rand::ThreadRng`
error[E0277]: the trait bound `rand::ThreadRng: rand::Rng` is not satisfied
   --> src/util/contracthash.rs:348:31
    |
348 |         let (sk3, pk3) = secp.generate_keypair(&mut thread_rng());
    |                               ^^^^^^^^^^^^^^^^ the trait `rand::Rng` is not implemented for `rand::ThreadRng`
error: aborting due to 3 previous errors

bisected it to:

41ebf5468c01a8411b41d8a3d5d9fcb807cc1d49 is the first bad commit
commit 41ebf5468c01a8411b41d8a3d5d9fcb807cc1d49
Author: Andrew Poelstra <[email protected]>
Date:   Wed Aug 22 19:37:32 2018 +0000
    update rust-secp dependency to 0.11
:100644 100644 7902134f3d96ecee1d4f74800089cf452bee4bf9 5f14d7388eaeb02fde3c0cee3b620a31577113ce M      Cargo.toml

(this is with rust stable rustc 1.28.0 (9634041f0 2018-07-30))

I'd like to understand the reasoning behind not using rustfmt to format the code (it has been the norm on other rust projects I've worked on).

Incase its not intentional (or perhaps just not formatted with the latest rustfmt version) I have opened a PR #171

Per @apoelstra's comment, we should pin down an Error type for SimpleDecoder and SimpleEncoder. I believe we should define a type network::serialize::Error that is a variant of util::Error.

Why we shouldn't just use util::Error:

Reason 1

It stops the overpopulation of util::Error variants.

Reason 2

In files such as script.rs, if we did

use ::Error;

Such that we can do

fn consensus_decode(d: &mut D) -> Result<Script, Error> {
    ...
}

It would trigger a namespace conflict with script::Error.

Note: this is assuming that we do

pub use util::Error;

at the top-level lib.rs.

Can you show me main function example where I can see how to use library to generate uncompressed main network bitcoin address from the given base58 encoded valid private key?

Hey, I would like to use this library to analyse the blockchain. I already have a downloaded copy, using the standard bitcoind. I assumed that I could use bitcoin::network::serialize::deserialize to deserialize a blk*.dat file into a Block struct. Is that correct? When I try to do that, the deserializer allocates an extremely large Vec<u8> and crashes because it's out of memory.

I try to verify transaction by Transaction::verify method and then I recieve an error "value: BitcoinConsensus(ERR_TX_INDEX)", but the transaction was successfully committed to the testnet blockchain.

https://testnet.blockchain.info/tx/b4abeb70c3b38f50a38adf511394cfae9c04650bcd6cbf7d484d61559e2045bd?show_adv=true

Here is code of failed test.
https://github.com/exonum/btc-transaction-utils/blob/tx_verify_bug/src/p2wsh.rs#L180
What could have gone wrong?

I've been trying my luck at bootstrapping rust 1.20.0 from mrustc. I wanted to open this thread so we can discuss our findings and methodologies.

Here are the resources I've found, where others are attempting the same thing:

Slackware (0.19.0 - 1.30.1)

https://github.com/aclemons/slack-rust-bootstrap

GUIX (plan only)

https://bootstrapping.miraheze.org/wiki/Boostrapping_Specific_Languages#Rust

Arch Linux (0.19.0 only)

mrustc and minicargo: https://aur.archlinux.org/packages/mrustc-git
rust=1.19.0: https://aur.archlinux.org/packages/rust-mrustc/

From my experience, the build instructions are quite fragile, and would perhaps be improved when thepowersgang/mrustc#88 gets merged.

So, there is a quite a decent start for a library for Bitcoin Core RPC: https://github.com/jeandudey/rust-bitcoin-rpc

I have some non-prod code, that I want to port to it, but I can't get ahold of the author: @jeandudey , I hope he's OK and will get back to us, but I can't wait and I'm tempted to just fork. However, the whole reason I'm porting from my own hacky code is to get eyeballs, feedback, polish etc.

Do you think rust-bitcoin-rpc falls under what rust-bitcoin github org. wants to have?

Is anyone interested in developing it too, or at least giving me some feedback on questions and issues I've raised?

It allows exponents larger than 18 (makes Display impl to crash)

The use of the deprecated rustc-serialize crate is preventing users from using various compile targets, specifically wasm32-unknown-unknown which is crucial for users looking to compile for browsers / nodejs.

The following crates should cover all required functionality: sha2, ripemd160, hmac.

adding dependency

[dependencies]
bitcoin = "0.3"

In this moment cause some errors
https://gist.github.com/RCasatta/b04c778b06de02bf1f029cad6f5a7a97

(while building c4cc2a6 from git works)

Hello,

as a final not of this project README you've linked the A Treatise on Altcoins by A. Poelstra as a note on altcoins in general. I must warn you that this paper while it is quite popular and readily quoted is not a prime example of unbiased and elaborate research paper on topic of cryptocurrency.
I will just warn to statement made on Peercoin, a altcoin which I help to develop and maintain.

Author states:

... prototypical proof-of-stake currency, Peercoin, depends on developer signatures to determine block validity: that is, its consensus is not distributed.

Which is all but truth. If I understood the author, he states that every block on the Peercoin network must be signed by developer (Sunny King) to become valid, and thus network is not decentralized.
Which is far from truth, naturally. Each block is signed by whoever mines it or mints it.

Current version of rust-bitcoin has some new features (Regtest network, consensus parameters, corrected multiplication).

Would it make sense to bump the version of crate?

I would like to play with atomic swaps, they need this opcode.

Are there plans to introduce a normal OP_CHECKLOCKTIMEVERIFY opcode?

What's the reasoning for NOPing the opcode?