Merge #305: types: use GhostCell in place of mutexes in union-bound and type context

060455a types: use GhostCell in union_bound rather than mutexes (Andrew Poelstra)
671a836 types: replace LockedContext with a mutex guard around ContextInner (Andrew Poelstra)
9212b39 types: introduce helper type to hold token alongside slab (Andrew Poelstra)
7dce5f0 types: use GhostToken for type context identity (Andrew Poelstra)
79bebd9 satisfy: add inference context to the Satisfier trait (Andrew Poelstra)
e18ffc0 policy: add (broken, ignored) unit test showing a problem with asm (Andrew Poelstra)
7e5df30 human_encoding: pass context object as argument to Node::to_witness_node (Andrew Poelstra)
a5aa6a2 node: add inference prameter to RedeemNode::to_construct_node (Andrew Poelstra)
a534de4 node: take context object as parameter to CommitNode::unfinalize_types (Andrew Poelstra)
dec28f3 node: take context as argument to all ConstructNode constructors (Andrew Poelstra)
65aab33 node: thread invariant 'brand lifetime through freeking everything (Andrew Poelstra)
fe8dc67 types: introduce 'Incomplete' type to represent incomplete type bounds (Andrew Poelstra)
c473b17 types: move innards of Context into a ContextInner type (Andrew Poelstra)
e2d1c4c types: remove Default from Context (Andrew Poelstra)
5411440 add ghost-cell dependency (Andrew Poelstra)
c7bfcbf add simple RedeemNode benchmark (Andrew Poelstra)

Pull request description:

  I recently learned about the [GhostCell](https://crates.io/crates/ghost-cell) crate, and its associated paper from ICFP 2021. There are a couple of similar projects from that era, notably `qcell` which predates the published version of ghostcell by a bit, but none of these are used much, as evidenced by the "Dependents" tab on crates.io.
  
  I learned about them from a blog post about ["generativity"](https://arhan.sh/blog/the-generativity-pattern-in-rust/) which has a pattern that makes GhostCells a little more ergonomic-looking at the cost of using macros. (I don't think this is worthwhile and I didn't use them). 
  
  But the macro syntax isn't really the point. The key quote from the blog post is this
  
  >The fundamental purpose of the generativity pattern is not necessarily to improve performance, but to statically require that individual values come from or refer to the same source. The performance benefits are a symptom of this requirement. 
  
  Anyway, GhostCells are extremely powerful and I don't know why they are not more popular. In effect, they allow synchronizing a single zero-sized token, and gating access to an arbitrarily-sized set of otherwise-disconnected objects on them. So if you have a `&mut Token` you can get  mutable access to all of your objects.
  
  This is exactly the situation we have with our type inference context and our union bound algorithm: we have a `Context` object which has a `Mutex` controlling access to a slab of allocated bounds, and then we have a whole pile of `UbElements` which are connected in a forest structure that the Rust compiler doesn't really understand, but where we morally have exclusive access exactly when we have exclusive access to the slab. `GhostCell` lets us tell the compiler this.
  
  This PR
  
  * Eliminates all the mutexes in the type checker except the one in the `Context` object that holds the slab
  * Fixes multiple bugs related to inconsistent contexts, including one in the policy module which made it impossible to actually use the asm fragment functionality
  * Adds an invariant lifetime `'brand` which acts as a tag on all `Type`s, `ConstructNode`s and `WitnessNode`s which prevent them being combined if they come from different type inference contexts
  * Requires the user to do node construction inside a closure which is passed to `Context::with_context`, rather than constructing a context object which then sorta leaks into the ether by being contained in an `Arc` carried by each node
  
  The latter item sounds like a big ergonomic hit, but in practice it's not a big deal because it doesn't affect `CommitNode` or `RedeemNode`.
  
  The use of a lifetime rather than a marker type means that while every type context has a "distinct type" for `Type`, `Arrow`, `ConstructNode`, etc., the distinction is erased after typechecking. So compilation speed and program size will not be adversely affected. But conversely, the compiler error messages are pretty unclear when you use this incorrectly.


ACKs for top commit:
  canndrew:
    ACK 060455a


Tree-SHA512: 2f7fd1eb729ef90aaa47cb24de2f6c70d70899ef0fd152f105469673648cdec73a63f31cc753e700e759d5949dab00452fefd686f1bb3405b7b9f2d909278f86

Author: apoelstra

Cargo-recent.lock

@@ -166,6 +166,12 @@ dependencies = [
  "wasm-bindgen",
 ]
 
+[[package]]
+name = "ghost-cell"
+version = "0.2.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d8449d342b1c67f49169e92e71deb7b9b27f30062301a16dbc27a4cc8d2351b7"
+
 [[package]]
 name = "hex-conservative"
 version = "0.1.2"
@@ -456,6 +462,7 @@ dependencies = [
  "byteorder",
  "elements",
  "getrandom",
+ "ghost-cell",
  "hex-conservative 0.2.1",
  "miniscript",
  "santiago",

Cargo.toml

@@ -26,6 +26,7 @@ bitcoin = { version = "0.32.0", optional = true }
 bitcoin-miniscript = { package = "miniscript", version = "12.0.0" }
 byteorder = "1.3"
 elements = { version = "0.25.0", optional = true, default-features = false }
+ghost-cell = { version = "0.2.6", default-features = false }
 hashes = { package = "bitcoin_hashes", version = "0.14" }
 hex = { package = "hex-conservative", version = "0.2.1" }
 santiago = "1.3"

fuzz/fuzz_lib/program.rs

@@ -45,13 +45,13 @@ impl ProgramControl {
 }
 
 impl Extractor<'_> {
-    pub fn extract_core_construct_node(
+    pub fn extract_core_construct_node<'brand>(
         &mut self,
+        ctx: &types::Context<'brand>,
         force_control: Option<ProgramControl>,
-    ) -> Option<Arc<ConstructNode<Core>>> {
-        type ArcNode = Arc<ConstructNode<Core>>;
+    ) -> Option<Arc<ConstructNode<'brand, Core>>> {
+        type ArcNode<'brand> = Arc<ConstructNode<'brand, Core>>;
 
-        let ctx = types::Context::new();
         let mut stack: Vec<ArcNode> = vec![];
 
         let program_control =
@@ -89,8 +89,8 @@ impl Extractor<'_> {
                         }
                     }
                     // 1 through 63
-                    1 => stack.push(ArcNode::unit(&ctx)),
-                    2 => stack.push(ArcNode::iden(&ctx)),
+                    1 => stack.push(ArcNode::unit(ctx)),
+                    2 => stack.push(ArcNode::iden(ctx)),
                     3 => {
                         use simplicity::dag::DagLike as _;
 
@@ -105,9 +105,9 @@ impl Extractor<'_> {
                                 return None;
                             }
                         }
-                        stack.push(ArcNode::scribe(&ctx, &val));
+                        stack.push(ArcNode::scribe(ctx, &val));
                     }
-                    4 if program_control.enable_witness => stack.push(ArcNode::witness(&ctx, None)),
+                    4 if program_control.enable_witness => stack.push(ArcNode::witness(ctx, None)),
                     5 => {
                         let child = stack.pop()?;
                         stack.push(ArcNode::injl(&child));
@@ -139,7 +139,7 @@ impl Extractor<'_> {
                     11 if program_control.enable_fail => {
                         let fail_u8 = self.extract_u8()?;
                         let fail = FailEntropy::from_byte_array([fail_u8; 64]);
-                        stack.push(ArcNode::fail(&ctx, fail));
+                        stack.push(ArcNode::fail(ctx, fail));
                     }
                     12 => {
                         let rchild = stack.pop()?;
@@ -165,7 +165,7 @@ impl Extractor<'_> {
                     _ => {
                         let extra_bits = usize::from(control >> 6);
                         let idx = (extra_bits << 8) + usize::from(self.extract_u8()?);
-                        stack.push(ArcNode::jet(&ctx, Core::ALL[idx % Core::ALL.len()]));
+                        stack.push(ArcNode::jet(ctx, Core::ALL[idx % Core::ALL.len()]));
                     }
                 }
             }

fuzz/fuzz_targets/c_rust_merkle.rs

@@ -7,6 +7,7 @@ fn do_test(data: &[u8]) {
     use simplicity::ffi::tests::{ffi::SimplicityErr, run_program, TestUpTo};
     use simplicity::hashes::sha256::Midstate;
     use simplicity::jet::Elements;
+    use simplicity::types;
     use simplicity::{BitIter, RedeemNode};
 
     // To decode the program length, we first try decoding the program using
@@ -15,15 +16,19 @@ fn do_test(data: &[u8]) {
     // from the error return.
     //
     // If the program doesn't decode, just use the first byte as a "length".
-    let prog_len = {
+    let prog_len: Option<usize> = types::Context::with_context(|ctx| {
         let mut iter = BitIter::from(data);
-        match simplicity::decode::decode_expression::<_, Elements>(&mut iter) {
-            Ok(_) => (iter.n_total_read() + 7) / 8,
+        match simplicity::decode::decode_expression::<_, Elements>(&ctx, &mut iter) {
+            Ok(_) => Some((iter.n_total_read() + 7) / 8),
             Err(_) => match data.first() {
-                Some(&n) => core::cmp::min(data.len(), n.into()),
-                None => return,
+                Some(&n) => Some(core::cmp::min(data.len(), n.into())),
+                None => return None,
             },
         }
+    });
+    let prog_len = match prog_len {
+        Some(x) => x,
+        None => return,
     };
 
     let (program, witness) = data.split_at(prog_len);

fuzz/fuzz_targets/regression_286.rs

@@ -4,36 +4,40 @@
 
 #[cfg(any(fuzzing, test))]
 fn do_test(data: &[u8]) {
-    use simplicity::{jet::Core, BitIter, CommitNode};
-
-    let mut extractor = simplicity_fuzz::Extractor::new(data);
-
-    let construct =
-        match extractor.extract_core_construct_node(Some(simplicity_fuzz::ProgramControl {
-            enable_type_bomb: false,
-            enable_disconnect: false,
-            enable_witness: false,
-            enable_fail: false,
-            enable_asserts: true,
-            max_nodes: Some(25),
-        })) {
+    use simplicity::{jet::Core, types, BitIter, CommitNode};
+
+    types::Context::with_context(|ctx| {
+        let mut extractor = simplicity_fuzz::Extractor::new(data);
+
+        let construct = match extractor.extract_core_construct_node(
+            &ctx,
+            Some(simplicity_fuzz::ProgramControl {
+                enable_type_bomb: false,
+                enable_disconnect: false,
+                enable_witness: false,
+                enable_fail: false,
+                enable_asserts: true,
+                max_nodes: Some(25),
+            }),
+        ) {
             Some(x) => x,
             None => return,
         };
-    //println!("constructed {construct}");
-    let finalized = match construct.finalize_types() {
-        Ok(x) => x,
-        Err(_) => return,
-    };
-    //println!("finalized {finalized}");
-    let prog = finalized.encode_to_vec();
-    //println!("{}", simplicity::bitcoin::hex::DisplayHex::as_hex(&prog));
-    let prog = BitIter::from(prog);
-    let decode = CommitNode::<Core>::decode(prog).unwrap();
-    assert_eq!(
-        finalized, decode,
-        "Constructed committed LHS; encoded and decoded to get RHS",
-    );
+        //println!("constructed {construct}");
+        let finalized = match construct.finalize_types() {
+            Ok(x) => x,
+            Err(_) => return,
+        };
+        //println!("finalized {finalized}");
+        let prog = finalized.encode_to_vec();
+        //println!("{}", simplicity::bitcoin::hex::DisplayHex::as_hex(&prog));
+        let prog = BitIter::from(prog);
+        let decode = CommitNode::<Core>::decode(prog).unwrap();
+        assert_eq!(
+            finalized, decode,
+            "Constructed committed LHS; encoded and decoded to get RHS",
+        );
+    });
 }
 
 #[cfg(fuzzing)]

src/bit_encoding/bitwriter.rs

@@ -123,8 +123,10 @@ mod tests {
 
     #[test]
     fn vec() {
-        let program = Arc::<ConstructNode<Core>>::unit(&types::Context::new());
-        let _ = write_to_vec(|w| program.encode_without_witness(w));
+        types::Context::with_context(|ctx| {
+            let program = Arc::<ConstructNode<Core>>::unit(&ctx);
+            let _ = write_to_vec(|w| program.encode_without_witness(w));
+        })
     }
 
     #[test]

src/bit_encoding/decode.rs

@@ -21,7 +21,7 @@ use std::{cmp, error, fmt};
 
 use super::bititer::{u2, DecodeNaturalError};
 
-type ArcNode<J> = Arc<ConstructNode<J>>;
+type ArcNode<'brand, J> = Arc<ConstructNode<'brand, J>>;
 
 /// Decoding error
 #[non_exhaustive]
@@ -48,13 +48,13 @@ pub enum Error {
 }
 
 impl From<crate::EarlyEndOfStreamError> for Error {
-    fn from(_: crate::EarlyEndOfStreamError) -> Error {
+    fn from(_: crate::EarlyEndOfStreamError) -> Self {
         Error::EndOfStream
     }
 }
 
 impl From<crate::BitIterCloseError> for Error {
-    fn from(e: crate::BitIterCloseError) -> Error {
+    fn from(e: crate::BitIterCloseError) -> Self {
         Error::BitIter(e)
     }
 }
@@ -153,16 +153,17 @@ impl<J: Jet> DagLike for (usize, &'_ [DecodeNode<J>]) {
     }
 }
 
-pub fn decode_expression<I: Iterator<Item = u8>, J: Jet>(
+pub fn decode_expression<'brand, I: Iterator<Item = u8>, J: Jet>(
+    ctx: &types::Context<'brand>,
     bits: &mut BitIter<I>,
-) -> Result<ArcNode<J>, Error> {
-    enum Converted<J: Jet> {
-        Node(ArcNode<J>),
+) -> Result<ArcNode<'brand, J>, Error> {
+    enum Converted<'brand, J: Jet> {
+        Node(ArcNode<'brand, J>),
         Hidden(Cmr),
     }
     use Converted::{Hidden, Node};
-    impl<J: Jet> Converted<J> {
-        fn get(&self) -> Result<&ArcNode<J>, Error> {
+    impl<'brand, J: Jet> Converted<'brand, J> {
+        fn get(&self) -> Result<&ArcNode<'brand, J>, Error> {
             match self {
                 Node(arc) => Ok(arc),
                 Hidden(_) => Err(Error::HiddenNode),
@@ -173,7 +174,6 @@ pub fn decode_expression<I: Iterator<Item = u8>, J: Jet>(
     let len = bits.read_natural::<usize>(None)?;
     assert_ne!(len, 0, "impossible to encode 0 in Simplicity");
 
-    let inference_context = types::Context::new();
     let mut nodes = Vec::with_capacity(cmp::min(len, 10_000));
     for _ in 0..len {
         let new_node = decode_node(bits, nodes.len())?;
@@ -191,8 +191,8 @@ pub fn decode_expression<I: Iterator<Item = u8>, J: Jet>(
         }
 
         let new = match nodes[data.node.0] {
-            DecodeNode::Unit => Node(ArcNode::unit(&inference_context)),
-            DecodeNode::Iden => Node(ArcNode::iden(&inference_context)),
+            DecodeNode::Unit => Node(ArcNode::unit(ctx)),
+            DecodeNode::Iden => Node(ArcNode::iden(ctx)),
             DecodeNode::InjL(i) => Node(ArcNode::injl(converted[i].get()?)),
             DecodeNode::InjR(i) => Node(ArcNode::injr(converted[i].get()?)),
             DecodeNode::Take(i) => Node(ArcNode::take(converted[i].get()?)),
@@ -218,18 +218,16 @@ pub fn decode_expression<I: Iterator<Item = u8>, J: Jet>(
                 converted[i].get()?,
                 &Some(Arc::clone(converted[j].get()?)),
             )?),
-            DecodeNode::Witness => Node(ArcNode::witness(&inference_context, None)),
-            DecodeNode::Fail(entropy) => Node(ArcNode::fail(&inference_context, entropy)),
+            DecodeNode::Witness => Node(ArcNode::witness(ctx, None)),
+            DecodeNode::Fail(entropy) => Node(ArcNode::fail(ctx, entropy)),
             DecodeNode::Hidden(cmr) => {
                 if !hidden_set.insert(cmr) {
                     return Err(Error::SharingNotMaximal);
                 }
                 Hidden(cmr)
             }
-            DecodeNode::Jet(j) => Node(ArcNode::jet(&inference_context, j)),
-            DecodeNode::Word(ref w) => {
-                Node(ArcNode::const_word(&inference_context, w.shallow_clone()))
-            }
+            DecodeNode::Jet(j) => Node(ArcNode::jet(ctx, j)),
+            DecodeNode::Word(ref w) => Node(ArcNode::const_word(ctx, w.shallow_clone())),
         };
         converted.push(new);
     }
@@ -318,7 +316,9 @@ mod tests {
         let justjet = [0x6d, 0xb8, 0x80];
         // Should be able to decode this as an expression...
         let mut iter = BitIter::from(&justjet[..]);
-        decode_expression::<_, Core>(&mut iter).unwrap();
+        types::Context::with_context(|ctx| {
+            decode_expression::<_, Core>(&ctx, &mut iter).unwrap();
+        });
         // ...but NOT as a CommitNode
         let iter = BitIter::from(&justjet[..]);
         CommitNode::<Core>::decode(iter).unwrap_err();

src/bit_machine/mod.rs

@@ -772,15 +772,17 @@ mod tests {
     fn crash_regression2() {
         use crate::node::{CoreConstructible as _, JetConstructible as _};
 
-        type Node = Arc<crate::ConstructNode<crate::jet::Core>>;
-
-        let mut bomb = Node::jet(
-            &crate::types::Context::new(),
-            crate::jet::Core::Ch8, // arbitrary jet with nonzero output size
-        );
-        for _ in 0..100 {
-            bomb = Node::pair(&bomb, &bomb).unwrap();
-        }
-        let _ = bomb.finalize_pruned(&());
+        type Node<'brand> = Arc<crate::ConstructNode<'brand, crate::jet::Core>>;
+
+        crate::types::Context::with_context(|ctx| {
+            let mut bomb = Node::jet(
+                &ctx,
+                crate::jet::Core::Ch8, // arbitrary jet with nonzero output size
+            );
+            for _ in 0..100 {
+                bomb = Node::pair(&bomb, &bomb).unwrap();
+            }
+            let _ = bomb.finalize_pruned(&());
+        });
     }
 }

src/human_encoding/error.rs

@@ -244,7 +244,8 @@ pub enum Error {
     /// us from knowing the whole program.
     HoleAtCommitTime {
         name: Arc<str>,
-        arrow: types::arrow::Arrow,
+        arrow_source: Arc<types::Incomplete>,
+        arrow_target: Arc<types::Incomplete>,
     },
     /// When converting to a `CommitNode`, a disconnect node had an actual node rather
     /// than a hole.
@@ -303,11 +304,12 @@ impl fmt::Display for Error {
             ),
             Error::HoleAtCommitTime {
                 ref name,
-                ref arrow,
+                ref arrow_source,
+                ref arrow_target,
             } => write!(
                 f,
-                "unfilled hole ?{} at commitment time; type arrow {}",
-                name, arrow
+                "unfilled hole ?{} at commitment time; type arrow {} -> {}",
+                name, arrow_source, arrow_target,
             ),
             Error::HoleFilledAtCommitTime => {
                 f.write_str("disconnect node has a non-hole child at commit time")