RCasatta / blocks_iterator

Iterate over bitcoin blocks
MIT License
51 stars 5 forks source link

test consensus_encode with &mut #56

Closed RCasatta closed 2 years ago

dpc commented 2 years ago

Please check if binaries get any smaller (not by a huge amount, but still) e.g. examples.

RCasatta commented 2 years ago

Actually, I checked the main executable and there was no change in size

Fault on my side, sorry. The main executable reduce size of ~100k over ~13M

dpc commented 2 years ago

Hmmm... to the byte? We did have a decrease in rust-bitcoin tests, though there is a lot of ser/de code involved.

dpc commented 2 years ago

master:

~/l/blocks_iterator (master)> ls -l target/release/blocks_iterator target/release/examples/{heaviest_pipe,less_reward_pipe,most_output_pipe,outputs_versions,verify}
-rwxrwxr-x 2 dpc dpc 13362384 Jun 28 10:30 target/release/blocks_iterator // IGNORE
-rwxrwxr-x 2 dpc dpc  8934680 Jun 28 10:30 target/release/examples/heaviest_pipe
-rwxrwxr-x 2 dpc dpc  8952144 Jun 28 10:30 target/release/examples/less_reward_pipe
-rwxrwxr-x 2 dpc dpc  8929776 Jun 28 10:30 target/release/examples/most_output_pipe
-rwxrwxr-x 2 dpc dpc 13353128 Jun 28 10:30 target/release/examples/outputs_versions
-rwxrwxr-x 2 dpc dpc 13602432 Jun 28 10:30 target/release/examples/verify // IGNORE

with &mut W + one example fix:

~/l/blocks_iterator (pr/56)> ls -l target/release/blocks_iterator target/release/examples/{heaviest_pipe,less_reward_pipe,most_output_pipe,outputs_versions,verify}
-rwxrwxr-x 2 dpc dpc 13362384 Jun 28 10:30 target/release/blocks_iterator // IGNORE
-rwxrwxr-x 2 dpc dpc  4568256 Jun 28 10:32 target/release/examples/heaviest_pipe
-rwxrwxr-x 2 dpc dpc  4581664 Jun 28 10:32 target/release/examples/less_reward_pipe
-rwxrwxr-x 2 dpc dpc  4563336 Jun 28 10:32 target/release/examples/most_output_pipe
-rwxrwxr-x 2 dpc dpc  5745968 Jun 28 10:32 target/release/examples/outputs_versions
-rwxrwxr-x 2 dpc dpc 13602432 Jun 28 10:30 target/release/examples/verify // IGNORE

master + https://github.com/dpc/rust-bitcoin/commit/cf5503af7462bc142e18480f1ee390a336b68311 (git version of rust-bitcoin without my &mut W PR):

~/l/blocks_iterator ((27d3cd2d…))> ls -l target/release/blocks_iterator target/release/examples/{heaviest_pipe,less_reward_pipe,most_output_pipe,outputs_versions,verify}
-rwxrwxr-x 2 dpc dpc 13362384 Jun 28 10:30 target/release/blocks_iterator // IGNORE
-rwxrwxr-x 2 dpc dpc  8922328 Jun 28 10:38 target/release/examples/heaviest_pipe
-rwxrwxr-x 2 dpc dpc  8940128 Jun 28 10:38 target/release/examples/less_reward_pipe
-rwxrwxr-x 2 dpc dpc  8921800 Jun 28 10:38 target/release/examples/most_output_pipe
-rwxrwxr-x 2 dpc dpc 13332496 Jun 28 10:38 target/release/examples/outputs_versions
-rwxrwxr-x 2 dpc dpc 13578216 Jun 28 10:38 target/release/examples/verify // IGNORE - I also started messing (investigating why it doesn't get rebuilt) with it here, so somehow it got rebuilt in the meantime

I was confused why blocks_iterator and verify didn't change timestamp, and it turns out in a default build command:

cargo build --release --all --examples

they just don't get built! When I try to compile with:

cargo build --release --all --examples --bin blocks_iterator --features="db,consensus"

I get more compilation errors, so I give up for now (got to go back to work, RN).

Anyway: on the examples I can compile I see almost 50% improvement?! Seems a bit too good to be true so please double check

Fix to this PR I used:

~/l/blocks_iterator (pr/56)> git diff HEAD~1
diff --git a/examples/signatures_in_witness.rs b/examples/signatures_in_witness.rs
index 48b47d1..5d88e86 100644
--- a/examples/signatures_in_witness.rs
+++ b/examples/signatures_in_witness.rs
@@ -61,25 +61,25 @@ struct ParsedSignature {
 }

 impl Decodable for ParsedSignature {
-    fn consensus_decode<D: std::io::Read>(mut d: D) -> Result<Self, encode::Error> {
+    fn consensus_decode<D: std::io::Read>(mut d: &mut D) -> Result<Self, encode::Error> {
         //TODO fix for schnorr signatures!
-        let first = u8::consensus_decode(&mut d)?;
+        let first = u8::consensus_decode(d)?;
         if first != 0x30 {
             return Err(encode::Error::ParseFailed("Signature must start with 0x30"));
         }
-        let _ = u8::consensus_decode(&mut d)?;
-        let integer_header = u8::consensus_decode(&mut d)?;
+        let _ = u8::consensus_decode(d)?;
+        let integer_header = u8::consensus_decode(d)?;
         if integer_header != 0x02 {
             return Err(encode::Error::ParseFailed("No integer header"));
         }

-        let R = <Vec<u8>>::consensus_decode(&mut d)?;
-        let integer_header = u8::consensus_decode(&mut d)?;
+        let R = <Vec<u8>>::consensus_decode(d)?;
+        let integer_header = u8::consensus_decode(d)?;
         if integer_header != 0x02 {
             return Err(encode::Error::ParseFailed("No integer header"));
         }
-        let s = <Vec<u8>>::consensus_decode(&mut d)?;
-        let sighash_u8 = u8::consensus_decode(&mut d)?;
+        let s = <Vec<u8>>::consensus_decode(d)?;
+        let sighash_u8 = u8::consensus_decode(d)?;
         let sighash = EcdsaSighashType::from_consensus(sighash_u8 as u32);

         Ok(ParsedSignature {

How I tested git version of rust-bitcoin without my &mut W PR:

~/l/blocks_iterator ((27d3cd2d…)) [1]> git diff master
diff --git a/Cargo.toml b/Cargo.toml
index 2258d86..16e4b4f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -11,7 +11,8 @@ readme = "README.md"
 license = "MIT"

 [dependencies]
-bitcoin =  { version = "0.28.0", features = ["use-serde"]}
+# bitcoin =  { version = "0.28.0", features = ["use-serde"]}
+bitcoin =  { git="https://github.com/dpc/rust-bitcoin", rev="cf5503af7462bc142e18480f1ee390a336b68311", features = ["serde"]}
 structopt = "0.3.21"
 log = "0.4.11"
 glob = "0.3.0"
RCasatta commented 2 years ago

Anyway: on the examples I can compile I see almost 50% improvement?! Seems a bit too good to be true so please double check

Wow I see the 50% improvement too on examples