RawNewtorkMessage deserealization from IO stream #229

dr-orlovsky · 2019-01-28T01:10:30Z

While working with remote peers over the network it is required to deserealize RawNetworkMessage from TCPStream to read the incoming messages. These messages can be partial – or one TCP packet can contain few of them. To make the library usable for such use cases, I have implemented the required functionality and covered it with unit tests.

Sample usage:

fn run() -> Result<(), Error> {
    // Opening stream to the remote bitcoind peer
    let mut stream = TcpStream::connect(SocketAddr::from(([37, 187, 0, 47], 8333))    let start = SystemTime::now();

    // Constructing and sending `version` message to get some messages back from the remote peer
    let since_the_epoch = start.duration_since(UNIX_EPOCH)
        .expect("Time went backwards");
    let version_msg = message::RawNetworkMessage {
        magic: constants::Network::Bitcoin.magic(),
        payload: message::NetworkMessage::Version(message_network::VersionMessage::new(
            0,
            since_the_epoch.as_secs() as i64,
            address::Address::new(receiver, 0),
            address::Address::new(receiver, 0),
            0,
            String::from("macx0r"),
            0
        ))
    };
    stream.write(encode::serialize(&version_msg).as_slice())?;

    // Obtaining three messages in return
    let mut buffer = vec![];
    loop {
        match message::RawNetworkMessage::from_stream(&mut stream, &mut buffer) {
            Ok(msg) => println!("Received message: {:?}", msg.payload),
            Err(err) => {
                stream.shutdown(Shutdown::Both)?;
                return Err(Error::DataError(err))
            },
        }
    }
}

Sample output is the following:

Received message: Version(VersionMessage { version: 70015, services: 1037, timestamp: 1548637162, receiver: Address {services: 0, address: [0, 0, 0, 0, 0, 65535, 23536, 35968], port: 33716}, sender: Address {services: 1037, address: [0, 0, 0, 0, 0, 0, 0, 0], port: 0}, nonce: 1370726880972892633, user_agent: "/Satoshi:0.17.99/", start_height: 560412, relay: true })
Received message: Verack
Received message: Alert([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 127, 0, 0, 0, 0, 255, 255, 255, 127, 254, 255, 255, 127, 1, 255, 255, 255, 127, 0, 0, 0, 0, 255, 255, 255, 127, 0, 255, 255, 255, 127, 0, 47, 85, 82, 71, 69, 78, 84, 58, 32, 65, 108, 101, 114, 116, 32, 107, 101, 121, 32, 99, 111, 109, 112, 114, 111, 109, 105, 115, 101, 100, 44, 32, 117, 112, 103, 114, 97, 100, 101, 32, 114, 101, 113, 117, 105, 114, 101, 100, 0])

Working sample code can be found here: https://github.com/dr-orlovsky/bitcoinbigdata-netlistener

…for reading TcpStream partial content

…t: `RawNetworkMessage::from_tcpstream`

…ream

dongcarl · 2019-01-31T21:49:19Z

@tamasblummer Would love your thoughts on this!

tamasblummer · 2019-02-01T12:02:03Z

I think that this is application specific and should not be part of this library. Eg buffer size and its implementation depends on what kind of stream one reads.

tamasblummer · 2019-02-01T12:02:40Z

src/network/message.rs

+    /// Reads stream from a TCP socket and parses first message from it, returing
+    /// the rest of the unparsed buffer for later usage.
+    pub fn from_stream(stream: &mut Read, remaining_part: &mut Vec<u8>) -> Result<Self, encode::Error> {
+        let mut max_iterations = 16;


why 16 and not 17?

Fixed in #231 using special configuration struct StreamReaderConfig providing parameters for the number of iterations and buffer size. I also implemented Defaults for it with default values proportional to the powers of 2

tamasblummer · 2019-02-01T12:03:18Z

src/network/message.rs

+                }
+            }
+
+            let mut new_data = vec![0u8; 1024];


buffer size arbitary and here too small for a tcp channel or disk i/o

Fixed in #231 – see comment above on new configuration structure StreamReaderConfig. The defaults for the buffer are also increased to 64kb (with Defaults trait)

tamasblummer · 2019-02-01T12:07:37Z

src/network/message.rs

+            let mut new_data = vec![0u8; 1024];
+            let count = stream.read(&mut new_data)?;
+            if count > 0 {
+                remaining_part.append(&mut new_data[0..count].to_vec());


this is inefficient. remaining_part.extend(new_data[0..count].iter()) would do it cheaper.

Thank you for a good point, fixed in #231

tamasblummer · 2019-02-01T12:08:56Z

also appetite for buffer size and DoS protection there is application specific. This implementation is only useful for quick-and-dirty build of something.

dr-orlovsky · 2019-02-01T22:29:40Z

Thank you for suggestions, I will fix them. However I would like to note, the reading from TCP socket for the network app is clearly not an app-specific but rather a part of core functionality. The buffer size politics can be configured, it's not a big problem

tamasblummer · 2019-02-02T07:24:59Z

src/consensus/encode.rs

        Ok(rv)
    } else {
+        eprintln!("total {}, consumed {}", data.len(), consumed);


this should be removed. should do not print debug in a final version.

actually the change to this method should be reverted. It appears me that the entire motivation of the change is to support the debug print.

Oh, I see the other motivation is to know the number of bytes consumed in from stream.

It would be better to pass Cursor to deserialize_partial instead of rewriting its logic with a

let mut consumed = 0

also passing &mut consumed id rather C/C++ style where you are restricted to a single return value.

the type usize is meant to be handling indices, it has the appropriate size for the OS memory model. Do not use u64 for that.

Completely reworked in #231 as you suggested: no &mut parameter, using Cursor etc. Thanks for pointing all that out.

tamasblummer · 2019-02-02T07:56:29Z

Please create PRs from a branch off your master instead of your master directly. This simplifies checking out your view.

Now function communicateds back to the caller the amount of consumed data by adding Cursor part to the Result value tuple – instead of using &mut argument. This have led to simplified and more straightforward logic of the function and its callers.

…ze and iterations with special struct

dr-orlovsky · 2019-02-02T19:23:40Z

Please create PRs from a branch off your master instead of your master directly. This simplifies checking out your view.

All suggestions are fixed with new PR #231
Thanks for detailed comments

tamasblummer · 2019-02-02T21:42:10Z

duplicate

Follow-up to #229 While working with remote peers over the network it is required to deserealize RawNetworkMessage from `TCPStream` to read the incoming messages. These messages can be partial – or one TCP packet can contain few of them. To make the library usable for such use cases, I have implemented the required functionality and covered it with unit tests. Sample usage: ```rust fn run() -> Result<(), Error> { // Opening stream to the remote bitcoind peer let mut stream = TcpStream::connect(SocketAddr::from(([37, 187, 0, 47], 8333)); let start = SystemTime::now(); // Constructing and sending `version` message to get some messages back from the remote peer let since_the_epoch = start.duration_since(UNIX_EPOCH) .expect("Time went backwards"); let version_msg = message::RawNetworkMessage { magic: constants::Network::Bitcoin.magic(), payload: message::NetworkMessage::Version(message_network::VersionMessage::new( 0, since_the_epoch.as_secs() as i64, address::Address::new(receiver, 0), address::Address::new(receiver, 0), 0, String::from("macx0r"), 0 )) }; stream.write(encode::serialize(&version_msg).as_slice())?; // Receiving incoming messages let mut buffer = vec![]; loop { let result = StreamReader::new(&mut stream, None).read_messages(); if let Err(err) = result { stream.shutdown(Shutdown::Both)?; return Err(Error::DataError(err)) } for msg in result.unwrap() { println!("Received message: {:?}", msg.payload); } } } ``` Sample output is the following: ``` Received message: Version(VersionMessage { version: 70015, services: 1037, timestamp: 1548637162, receiver: Address {services: 0, address: [0, 0, 0, 0, 0, 65535, 23536, 35968], port: 33716}, sender: Address {services: 1037, address: [0, 0, 0, 0, 0, 0, 0, 0], port: 0}, nonce: 1370726880972892633, user_agent: "/Satoshi:0.17.99/", start_height: 560412, relay: true }) Received message: Verack Received message: Alert([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 127, 0, 0, 0, 0, 255, 255, 255, 127, 254, 255, 255, 127, 1, 255, 255, 255, 127, 0, 0, 0, 0, 255, 255, 255, 127, 0, 255, 255, 255, 127, 0, 47, 85, 82, 71, 69, 78, 84, 58, 32, 65, 108, 101, 114, 116, 32, 107, 101, 121, 32, 99, 111, 109, 112, 114, 111, 109, 105, 115, 101, 100, 44, 32, 117, 112, 103, 114, 97, 100, 101, 32, 114, 101, 113, 117, 105, 114, 101, 100, 0]) ``` Working sample code can be found here: https://github.com/dr-orlovsky/bitcoinbigdata-netlistener

newtype implementation of opcodes::All Removes unsafety when converting u8 -> All safe implementation of All -> Ordinary squashme: work around lack of associated constants make opcode PR work with 1.14.0 add some opcode tests Fix indentation in opcodes.rs Fix comment on transaction version Internalize unnecessarily exported macros Bump secp to 0.12 Replace slow hex decoding function with optimized version Fixes rust-bitcoin#207. Remove unused Pair iterator and util::iter module Add feature gated hex decode benchmark bump version to 0.16 Bump rustc version to 1.22.0 it is annoying to have a difference between debug and print for hash Fix typos Run cargo bench on rustc nightly in travis, remote useless move Add bitcoin_hashes dependency, rename some features Because features and dependencies share the same namespace, and we want to pass down the optional dependence on serde to bitcoin_hashes, we need to rename the feature to something other than serde. Right now only features can be passed down to dependencies. Note that we could have also renamed the dependency to something like serde-dep and kept the same feature name, however, dependency renaming has only been available since cargo 0.27.0 Features that represent optional dependencies have been prefixed with 'use-'. The travis file has also been modified to conform to this change. Implement En/Decodable for sha256d::Hash Convert codebase from util::hash to bitcoin_hashes Also replace unsafe transmute with call to read_u64_into Remove code deprecated by bitcoin_hashes from util::hash Remove unused internal macro Remove fuzz_util module Not needed anymore as the bitcoin_hashes crate handles this. Remove rust-crypto dependency We no longer need rust-crypto after integrating bitcoin_hashes. Fix typos and clarify some comment in blockdata, block, address (rust-bitcoin#230) Remove Address::p2pk There is no address format for p2pk. add BIP157 (Client Side Block Filtering) Messages (rust-bitcoin#225) * add BIP57 (Client Side Block Filtering) Messages * rabased after rust-bitcoin#215 Cleanup util::privkey in preparation for PublicKey - Rename privkey::PrivKey to privkey::PrivateKey - Remove unnecessary methods for privkey::PrivateKey - Modify tests to work with above changes Add PublicKey struct encapsulating compressedness - Move util::privkey to util::key - Add PublicKey struct to util::key - Implement de/serialization methods for util::key::PublicKey Integrate newly-added PublicKey with Address - Switch util::address::Payload::Pubkey variant to wrap util::key::PublicKey - Switch util::address::Address::p*k* constructors to use util::key::PublicKey - Fix tests for aforementioned switch - Add convenience methods for util::key::PublicKey to util::key::PrivateKey conversion - Switch BIP143 tests to use util::key::PublicKey key: Reword and clarify comments key: Use correct error for decoding This change also moves the secp256k1::Error wrapper from util::Error to consensus::encode::Error, since we do not use it anywhere else. We can add it back to util::Error once we have instances of secp256k1::Error that are not related to consensus::encode. Extract travis testing into locally-runnable script Extract the Script assembly creator from fmt::Debug Added contributing part to README point to IRC bip32: ChildNumber constructors return Result They can produce an error if the index is out of range. bip32: Introduce DerivationPath type Implements Display and FromStr for easy usage with serialized types. bip32: Change test vectors to use DerivationPath bip32: Add additional methods and traits to DerivationPath - From<&[ChildNumber]> (cloning) - AsRef<[ChildNumber]> - std::iter::FromIterator<ChildNumber> - std::iter::IntoIterator<ChildNumber> - std::ops::Index (returning &[ChildNumber]) Also add two methods: - child(&self, ChildNumber) -> DerivationPath - into_child(self, ChildNumber) -> DerivationPath Implement Witness commitment check for Block. Remove MerkleRoot implementations for types implementing BitcoinHash as it is misleading. MerkleRoot is defined instead for a Block. Forbid unsafe code Remove extraneous clones in consensus::params Remove unused Option en/decoding Better RawNewtorkMessage deserealization from IO stream (rust-bitcoin#231) Follow-up to rust-bitcoin#229 While working with remote peers over the network it is required to deserealize RawNetworkMessage from `TCPStream` to read the incoming messages. These messages can be partial – or one TCP packet can contain few of them. To make the library usable for such use cases, I have implemented the required functionality and covered it with unit tests. Sample usage: ```rust fn run() -> Result<(), Error> { // Opening stream to the remote bitcoind peer let mut stream = TcpStream::connect(SocketAddr::from(([37, 187, 0, 47], 8333)); let start = SystemTime::now(); // Constructing and sending `version` message to get some messages back from the remote peer let since_the_epoch = start.duration_since(UNIX_EPOCH) .expect("Time went backwards"); let version_msg = message::RawNetworkMessage { magic: constants::Network::Bitcoin.magic(), payload: message::NetworkMessage::Version(message_network::VersionMessage::new( 0, since_the_epoch.as_secs() as i64, address::Address::new(receiver, 0), address::Address::new(receiver, 0), 0, String::from("macx0r"), 0 )) }; stream.write(encode::serialize(&version_msg).as_slice())?; // Receiving incoming messages let mut buffer = vec![]; loop { let result = StreamReader::new(&mut stream, None).read_messages(); if let Err(err) = result { stream.shutdown(Shutdown::Both)?; return Err(Error::DataError(err)) } for msg in result.unwrap() { println!("Received message: {:?}", msg.payload); } } } ``` Sample output is the following: ``` Received message: Version(VersionMessage { version: 70015, services: 1037, timestamp: 1548637162, receiver: Address {services: 0, address: [0, 0, 0, 0, 0, 65535, 23536, 35968], port: 33716}, sender: Address {services: 1037, address: [0, 0, 0, 0, 0, 0, 0, 0], port: 0}, nonce: 1370726880972892633, user_agent: "/Satoshi:0.17.99/", start_height: 560412, relay: true }) Received message: Verack Received message: Alert([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 255, 127, 0, 0, 0, 0, 255, 255, 255, 127, 254, 255, 255, 127, 1, 255, 255, 255, 127, 0, 0, 0, 0, 255, 255, 255, 127, 0, 255, 255, 255, 127, 0, 47, 85, 82, 71, 69, 78, 84, 58, 32, 65, 108, 101, 114, 116, 32, 107, 101, 121, 32, 99, 111, 109, 112, 114, 111, 109, 105, 115, 101, 100, 44, 32, 117, 112, 103, 114, 97, 100, 101, 32, 114, 101, 113, 117, 105, 114, 101, 100, 0]) ``` Working sample code can be found here: https://github.com/dr-orlovsky/bitcoinbigdata-netlistener key: add some missing functionality bip32: replace rust-secp key types with rust-bitcoin key types We continue to support only compressed keys when doing key derivation, but de/serialization of uncompressed keys will now work, and it will be easier/more consistent to implement PSBT on top of this. Add PSBT-specific Error data type - Implement psbt::Error data type - Implement conversion from psbt::Error to util::Error - Create util::psbt module - Create non-public util::psbt::error module Add data types for raw PSBT key-value pairs - Add (en)decoding logic for said data types Add trait for PSBT key-value maps Add PSBT global data key-value map type - Implement psbt::Map trait for psbt::Global - Add converting constructor logic from Transaction for psbt::Global - Add (en)decoding logic for psbt::Global - Always deserialize unsigned_tx as non-witness - Add trait for PSBT (de)serialization - Implement PSBT (de)serialization trait for relevant psbt::Global types - Add macros for consensus::encode-backed PSBT (de)serialization implementations - Add macro for implementing encoding logic for PSBT key-value maps Add PSBT output data key-value map type - Implement psbt::Map trait for psbt::Output - Add (en)decoding logic for psbt::Output - Implement PSBT (de)serialization trait for relevant psbt::Output types - Add macro for merging fields for PSBT key-value maps - Add macro for implementing decoding logic for PSBT key-value maps - Add convenience macro for implementing both encoding and decoding logic for PSBT key-value maps - Add macro for inserting raw PSBT key-value pairs into PSBT key-value maps - Add macro for getting raw PSBT key-value pairs from PSBT key-value maps Add PSBT input data key-value map type - Implement psbt::Map trait for psbt::Input - Add (en)decoding logic for psbt::Input - Implement PSBT (de)serialization trait for relevant psbt::Input types Add Partially Signed Transaction type - Add merging logic for PartiallySignedTransactions - Add (en)decoding logic for PartiallySignedTransaction - Add converting constructor logic from Transaction for PartiallySignedTransaction - Add extracting constructor logic from PartiallySignedTransaction for Transaction Squashed in fixes from stevenroose <stevenroose@gmail.com> - Prevent PSBT::extract_tx from panicking - Make PartiallySignedTransaction fields public Add test vectors from BIP174 specification - Add macro for decoding and unwrapping PartiallySignedTransaction from hex string Add fuzz testing for PartiallySignedTransaction Add copyright notice to PSBT-related files bump version to 0.17.0 add rust-bitcoin#231 to CHANGELOG for 0.17.0 key: implement ToString and FromStr for PublicKey script: add `push_key` function to Builder to allow serializing public keys more easily bump version to 0.17.1 Bump bitcoin-bech32 dependency This makes the Address::Payload::WitnessProgram inner type compatible with rust-lightning-invoice's Fallback::SegWitProgram's inner type. This allows specifying fallbacks from addresses. util::key: Provide to_bytes() methods for key types These are mainly utility methods around the existing way to serialize the key types. util::key add serde de/serialization contracthash: add fixed test vector contracthash: use `PublicKey` and `PrivateKey` types; minor cleanups contracthash: more cleanups bump version to 0.18 Fix nit in CHANGELOG.md bip32: Add DerivationPathIterator and related methods Adds methods - ChildNumber::increment - DerivationPath::children_from - DerivationPath::normal_children - DerivationPath::hardened_children Drop LoneHeaders and just use BlockHeader The protocol has a bug where a 0u8 is pushed at the end of each block header on the wire in headers messages. WHy this bug came about is unrealted and shouldn't impact API design. Implement util::misc::signed_msg_hash() Add slice consensus encode/decode functions and use for short arrays Swap a few more [d]encoders to slice emit/read functions Support sendheaders network message decode Drop some unused/not-needed Encodable impls Speed up Vec<u8> [d]e[n]code operations by dropping the generic Decrease travis-fuzz iterations to fix hangs Two serde quirks from switching dependencies Add Amount and SignedAmount types Add fuzz target for Amount parsing Fix trivial DoS when deserializing messages from the network fuzz: Add fuzzer for RawNetworkMessage. Rename deserialize_raw_network_message to make my afl scripts happy Switch Travis fuzzing to 30 seconds per target from an iter count. Rename BlockHeader::spv_validate to validate_pow Remove confusing mentions of SPV Rename OP_NOP2 and OP_NOP3 to OP_CLTV and OP_CSV Add OutPoint::new() for one-liner construction (rust-bitcoin#285) Remove Decimal and replace strason with serde_json Slightly update README

Satisfaction bugs

dr-orlovsky added 14 commits January 27, 2019 19:37

Support for debug mode formattin in RawNetworkMessage

4a16bcc

Desearilizing partial message content, which will be required lately …

83acf9a

…for reading TcpStream partial content

Improved unit tests for message peyload deserialization

e6ee7e5

Function that parses TcpStream and creates RawNetworkMessage out of i…

afcc7c8

…t: `RawNetworkMessage::from_tcpstream`

Support for automatic IO errors in ? syntax

940d63d

Hiding network::Error conversion trait from docs

6abbd15

Trying reading TcpStream with BufRead

e382886

Improving buffering while deserializing from TcpStream

28a1ffa

Generalizing stream deserialization (from TcpStream to any IO stream)

d3afdc4

Stream deserealization unit test case

4410af8

Improved stream reading algorithm

34a00e8

Stream reading unit test updated with more complex case

365facc

Fixed error reporting on incomplete message deserealization from a st…

4d464fa

…ream

Documented deserialize_partial function

f5587cd

tamasblummer reviewed Feb 1, 2019

View reviewed changes

tamasblummer reviewed Feb 2, 2019

View reviewed changes

dr-orlovsky added 2 commits February 2, 2019 20:42

Configuring network::message::RawNetworkMessage.from_stream buffer si…

48021db

…ze and iterations with special struct

dr-orlovsky mentioned this pull request Feb 2, 2019

Better RawNewtorkMessage deserealization from IO stream #231

Merged

tamasblummer closed this Feb 2, 2019

apoelstra mentioned this pull request Dec 22, 2021

Deprecate StreamReader #680

Merged

yancyribbens pushed a commit to yancyribbens/rust-bitcoin that referenced this pull request Mar 23, 2024

Merge pull request rust-bitcoin#229 from sanket1729/satisfaction_bugs

52c6174

Satisfaction bugs

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RawNewtorkMessage deserealization from IO stream #229

RawNewtorkMessage deserealization from IO stream #229

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RawNewtorkMessage deserealization from IO stream #229

RawNewtorkMessage deserealization from IO stream #229

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!