Merge #80: Update rust-miniscript, pre-demo cleanups

546ee51 Cargo: bump version to '0.2.0' (Antoine Poinsot) 0076df1 Cargo: never activate rust-bitcoin's "use-serde" feature (Antoine Poinsot) 20bc6d6 ci: fuzz against our generated corpus (Antoine Poinsot) 322ac7f transactions: derive Spend transaction from outpoints and amounts (Antoine Poinsot) 90f305d transactions: build transaction chain from deposit outpoint and txo (Antoine Poinsot) b8cd7c5 transactions: add txid getters (Antoine Poinsot) 6a10e81 scripts: add a CSV getter to the Unvault descriptor (Antoine Poinsot) 60a0c8b fuzz: use secp256k1's global context (Antoine Poinsot) 80bfdcc transactions: make sure the Spend transaction size is standard (Antoine Poinsot) d025a86 scripts: require minimal CSV (Antoine Poinsot) b5478f4 scripts: use rust-miniscript's constants for sequence bit flags (Antoine Poinsot) baebf79 transactions: fixup CSV tests after Miniscript update (Antoine Poinsot) bee2254 Tree-wide: misc doc cleanups (Antoine Poinsot) ca77be2 scripts: implement to/from str for descriptors newtypes (Antoine Poinsot) b0be252 scripts: add constructors for derived descriptors type (Antoine Poinsot) 3aa53ba scripts: encapsulation for the descriptors (Antoine Poinsot) b7ab05a Cargo: use a smaller alternative to rust-secp's rand (Antoine Poinsot) cb29b3a Tree-wide: update rust-miniscript to the latest major version (Antoine Poinsot) Pull request description: This updates rust-miniscript after an overhaul of the type system for descriptor. We leverage it to refactor our descriptor types on our end too, resulting in a much cleaner API. This also contains a bunch of fixes that were waiting on a rust-miniscript update or were needed for the demo. Fixes #64 Fixes #74 Fixes #41 Fixes #59 ACKs for top commit: danielabrozzoni: utACK 546ee51 -- Reviewed briefly, looks good to me :) edouardparis: utACK 546ee51 darosior: ACK 546ee51 Tree-SHA512: f0dfe8fa6925c33788b3b04ebd34bba16288f667767909ea7e270497717bbe663298775e77545a6f3937df20e984a8ac5e22f49cf8af3058105e9d7b3c28c3b8
revault · Apr 7, 2021 · 7ff8de4 · 7ff8de4
2 parents b0502d5 + 546ee51
commit 7ff8de4
Show file tree

Hide file tree

Showing 14 changed files with 1,311 additions and 613 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -1,6 +1,6 @@
 name: CI
 
-on: [push, pull_request]
+on: [pull_request]
 
 jobs:
   unittesting:
@@ -45,7 +45,7 @@ jobs:
           override: true
           profile: minimal
       - name: Run fuzz testing script
-        run: ./fuzz/run.sh
+        run: ./contrib/ci_fuzz.sh
 
   rustfmt_check:
     runs-on: ubuntu-latest

diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "revault_tx"
-version = "0.1.2"
+version = "0.2.0"
 authors = ["Antoine Poinsot <darosior@protonmail.com>"]
 edition = "2018"
 repository = "https://github.com/revault/revault_tx"
@@ -10,15 +10,15 @@ description = "Bitcoin Script descriptors and transactions creation routines for
 exclude = [".github/", "fuzz"]
 
 [features]
-use-serde = ["serde", "miniscript/use-serde"]
+use-serde = ["serde"]
 
 [dependencies]
 bitcoinconsensus = "0.19.0-2"
-miniscript = { version = "4.0.3", features = ["compiler"] }
+miniscript = { version = "5.1.0", features = ["compiler"] }
 base64 = { version = "0.13" }
 
 serde = { version = "1.0", optional = true }
 
 [dev-dependencies]
-miniscript = { version = "4.0.3", features = ["compiler", "rand"] }
+fastrand = "1.4.0"
 serde_json = "1.0"
diff --git a/contrib/ci_fuzz.sh b/contrib/ci_fuzz.sh
@@ -0,0 +1,6 @@
+set -ex
+
+git clone https://github.com/revault/revault_tx_corpus fuzz/corpus
+. fuzz/run.sh
+
+set -ex
diff --git a/fuzz/fuzz_targets/parse_cancel.rs b/fuzz/fuzz_targets/parse_cancel.rs
@@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target;
 
 use revault_tx::{
     miniscript::bitcoin::{
-        secp256k1::{Secp256k1, Signature},
+        secp256k1::{Signature, SECP256K1},
         PublicKey, SigHashType,
     },
     transactions::{CancelTransaction, RevaultTransaction},
@@ -91,10 +91,8 @@ fuzz_target!(|data: &[u8]| {
         #[allow(unused_must_use)]
         tx.verify_input(0);
 
-        // FIXME: find a way to use the global context of secp...
-        let secp = Secp256k1::new();
         // Same for the finalization
         #[allow(unused_must_use)]
-        tx.finalize(&secp);
+        tx.finalize(&SECP256K1);
     }
 });
diff --git a/fuzz/fuzz_targets/parse_emergency.rs b/fuzz/fuzz_targets/parse_emergency.rs
@@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target;
 
 use revault_tx::{
     miniscript::bitcoin::{
-        secp256k1::{Secp256k1, Signature},
+        secp256k1::{Signature, SECP256K1},
         PublicKey, SigHashType,
     },
     transactions::{EmergencyTransaction, RevaultTransaction},
@@ -87,10 +87,8 @@ fuzz_target!(|data: &[u8]| {
         #[allow(unused_must_use)]
         tx.verify_input(0);
 
-        // FIXME: find a way to use the global context of secp...
-        let secp = Secp256k1::new();
         // Same for the finalization
         #[allow(unused_must_use)]
-        tx.finalize(&secp);
+        tx.finalize(&SECP256K1);
     }
 });
diff --git a/fuzz/fuzz_targets/parse_spend.rs b/fuzz/fuzz_targets/parse_spend.rs
@@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target;
 
 use revault_tx::{
     miniscript::bitcoin::{
-        secp256k1::{Secp256k1, Signature},
+        secp256k1::{Signature, SECP256K1},
         PublicKey, SigHashType,
     },
     transactions::{RevaultTransaction, SpendTransaction},
@@ -49,10 +49,8 @@ fuzz_target!(|data: &[u8]| {
             tx.verify_input(i);
         }
 
-        // FIXME: find a way to use the global context of secp...
-        let secp = Secp256k1::new();
         // Same for the finalization
         #[allow(unused_must_use)]
-        tx.finalize(&secp);
+        tx.finalize(&SECP256K1);
     }
 });
diff --git a/fuzz/fuzz_targets/parse_unvault.rs b/fuzz/fuzz_targets/parse_unvault.rs
@@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target;
 
 use revault_tx::{
     miniscript::bitcoin::{
-        secp256k1::{Secp256k1, Signature},
+        secp256k1::{Signature, SECP256K1},
         PublicKey, SigHashType,
     },
     transactions::{RevaultTransaction, UnvaultTransaction},
@@ -44,10 +44,8 @@ fuzz_target!(|data: &[u8]| {
         #[allow(unused_must_use)]
         tx.verify_input(0);
 
-        // FIXME: find a way to use the global context of secp...
-        let secp = Secp256k1::new();
         // Same for the finalization
         #[allow(unused_must_use)]
-        tx.finalize(&secp);
+        tx.finalize(&SECP256K1);
     }
 });
diff --git a/fuzz/fuzz_targets/parse_unvault_emergency.rs b/fuzz/fuzz_targets/parse_unvault_emergency.rs
@@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target;
 
 use revault_tx::{
     miniscript::bitcoin::{
-        secp256k1::{Secp256k1, Signature},
+        secp256k1::{Signature, SECP256K1},
         PublicKey, SigHashType,
     },
     transactions::{RevaultTransaction, UnvaultEmergencyTransaction},
@@ -94,10 +94,8 @@ fuzz_target!(|data: &[u8]| {
         #[allow(unused_must_use)]
         tx.verify_input(0);
 
-        // FIXME: find a way to use the global context of secp...
-        let secp = Secp256k1::new();
         // Same for the finalization
         #[allow(unused_must_use)]
-        tx.finalize(&secp);
+        tx.finalize(&SECP256K1);
     }
 });
diff --git a/fuzz/run.sh b/fuzz/run.sh
@@ -6,5 +6,5 @@ cd corpus && git clone https://github.com/revault/revault_tx_corpus
 
 cargo install --force cargo-fuzz
 for target in $(ls fuzz/fuzz_targets);do
-    cargo +nightly fuzz run "${target%.*}" -- -runs=0
+    cargo +nightly fuzz run -O "${target%.*}" -- -runs=0 -maxlen=500000
 done
diff --git a/src/error.rs b/src/error.rs
@@ -1,4 +1,4 @@
-//! Errors related to Revault transactions and Scripts management
+//! # Errors related to Revault transactions and Scripts management
 
 use crate::transactions::INSANE_FEES;
 
@@ -13,20 +13,27 @@ use miniscript::{
 
 use std::{convert::From, error, fmt};
 
-/// Error when creating a Revault Bitcoin Script
-#[derive(PartialEq, Eq, Debug)]
+/// Error when creating a Revault Miniscript Descriptor
+#[derive(Debug)]
 pub enum ScriptCreationError {
     /// Invalid number of keys, threshold, or timelock
     BadParameters,
+    /// At least one of the keys was not derivable
+    NonWildcardKeys,
     /// Miniscript policy compilation error
     PolicyCompilation(CompilerError),
+    /// Miniscript general error, currently only for sanity checks in descriptor
+    /// constructors
+    MiniscriptError(miniscript::Error),
 }
 
 impl fmt::Display for ScriptCreationError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             Self::BadParameters => write!(f, "Bad parameters"),
             Self::PolicyCompilation(e) => write!(f, "Policy compilation error: '{}'", e),
+            Self::MiniscriptError(e) => write!(f, "Miniscript error: '{}'", e),
+            Self::NonWildcardKeys => write!(f, "Not all xpubs were wildcard"),
         }
     }
 }
@@ -37,6 +44,12 @@ impl From<CompilerError> for ScriptCreationError {
     }
 }
 
+impl From<miniscript::Error> for ScriptCreationError {
+    fn from(e: miniscript::Error) -> Self {
+        Self::MiniscriptError(e)
+    }
+}
+
 impl error::Error for ScriptCreationError {}
 
 /// Error when creating a Revault Bitcoin transaction output
@@ -64,6 +77,8 @@ pub enum TransactionCreationError {
     Dust,
     /// Sends more than it spends
     NegativeFees,
+    /// Transaction weight more than 400k weight units.
+    TooLarge,
 }
 
 impl fmt::Display for TransactionCreationError {
@@ -75,6 +90,10 @@ impl fmt::Display for TransactionCreationError {
                 f,
                 "The sum of the inputs value is less than the sum of the outputs value"
             ),
+            Self::TooLarge => write!(
+                f,
+                "Transaction too large: satisfied it could be >400k weight units"
+            ),
         }
     }
 }
@@ -133,6 +152,7 @@ pub enum PsbtValidationError {
     InvalidPrevoutType(PsbtInput),
     PartiallyFinalized,
     InsaneAmounts,
+    TransactionTooLarge,
 }
 
 impl fmt::Display for PsbtValidationError {
@@ -183,6 +203,10 @@ impl fmt::Display for PsbtValidationError {
                 f,
                 "PSBT contains either overflowing amounts or creates more coins than it spends"
             ),
+            Self::TransactionTooLarge => write!(
+                f,
+                "Transaction too large: satisfied it could be >400k weight units"
+            ),
         }
     }
 }
@@ -232,7 +256,7 @@ impl From<PsbtValidationError> for TransactionSerialisationError {
 impl error::Error for TransactionSerialisationError {}
 
 /// An error specific to the management of Revault transactions and scripts.
-#[derive(PartialEq, Debug)]
+#[derive(Debug)]
 pub enum Error {
     /// Error when creating a Revault Bitcoin Script
     ScriptCreation(ScriptCreationError),