Merge pull request #92 from Chia-Network/20240522-int-conversion

Fix bug from early ported ocaml code regarding zeroes and program representation.  Add a cl23.1 sigil that enables this and add many tests.

src/classic/clvm_tools/binutils.rs

@@ -98,7 +98,7 @@ pub fn ir_for_atom(
 
         // Determine whether the bytes identity an integer in canonical form.
         // It's not canonical if there is oversized sign extension.
-        if !has_oversized_sign_extension(atom) {
+        if atom.data() != &[0] && !has_oversized_sign_extension(atom) {
             return IRRepr::Int(atom.clone(), true);
         }
     }

src/compiler/clvm.rs

@@ -1,5 +1,7 @@
 use std::borrow::Borrow;
+use std::cell::RefCell;
 use std::collections::HashMap;
+use std::mem::swap;
 use std::rc::Rc;
 
 use clvm_rs::allocator;
@@ -15,11 +17,36 @@ use crate::classic::clvm_tools::stages::stage_0::TRunProgram;
 
 use crate::compiler::prims;
 use crate::compiler::runtypes::RunFailure;
-use crate::compiler::sexp::{parse_sexp, SExp};
+use crate::compiler::sexp::{parse_sexp, printable, SExp};
 use crate::compiler::srcloc::Srcloc;
 
 use crate::util::{number_from_u8, u8_from_number, Number};
 
+thread_local! {
+    static NEW_COMPILATION_LEVEL_INT: RefCell<bool> = const { RefCell::new(true) };
+}
+
+pub struct NewStyleIntConversion(bool);
+
+impl NewStyleIntConversion {
+    pub fn new(mut new_val: bool) -> NewStyleIntConversion {
+        NewStyleIntConversion(NEW_COMPILATION_LEVEL_INT.with(|v| {
+            let mut val_ref = v.borrow_mut();
+            swap(&mut new_val, &mut val_ref);
+            new_val
+        }))
+    }
+    fn setting() -> bool {
+        NEW_COMPILATION_LEVEL_INT.with(|v| *v.borrow())
+    }
+}
+
+impl Drop for NewStyleIntConversion {
+    fn drop(&mut self) {
+        NEW_COMPILATION_LEVEL_INT.with(|v| *v.borrow_mut() = self.0)
+    }
+}
+
 /// Provide a way of intercepting and running new primitives.
 pub trait PrimOverride {
     fn try_handle(
@@ -236,7 +263,7 @@ pub fn convert_to_clvm_rs(
             .new_atom(x)
             .map_err(|_e| RunFailure::RunErr(head.loc(), format!("failed to alloc string {head}"))),
         SExp::Integer(_, i) => {
-            if *i == bi_zero() {
+            if NewStyleIntConversion::setting() && *i == bi_zero() {
                 Ok(allocator.null())
             } else {
                 allocator
@@ -264,6 +291,7 @@ pub fn convert_from_clvm_rs(
 ) -> Result<Rc<SExp>, RunFailure> {
     match allocator.sexp(head) {
         allocator::SExp::Atom => {
+            let int_conv = NewStyleIntConversion::setting();
             let atom_data = allocator.atom(head);
             if atom_data.is_empty() {
                 Ok(Rc::new(SExp::Nil(loc)))
@@ -272,11 +300,13 @@ pub fn convert_from_clvm_rs(
                 // Ensure that atom values that don't evaluate equal to integers
                 // are represented faithfully as atoms.
                 if u8_from_number(integer.clone()) == atom_data {
-                    if atom_data == [0] {
+                    if int_conv && atom_data == [0] {
                         Ok(Rc::new(SExp::QuotedString(loc, b'x', atom_data.to_vec())))
                     } else {
                         Ok(Rc::new(SExp::Integer(loc, integer)))
                     }
+                } else if int_conv && !printable(atom_data, true) {
+                    Ok(Rc::new(SExp::QuotedString(loc, b'x', atom_data.to_vec())))
                 } else {
                     Ok(Rc::new(SExp::Atom(loc, atom_data.to_vec())))
                 }
@@ -782,7 +812,13 @@ pub fn sha256tree(s: Rc<SExp>) -> Vec<u8> {
             hasher.finalize().to_vec()
         }
         SExp::Nil(_) => sha256tree_from_atom(&[]),
-        SExp::Integer(_, i) => sha256tree_from_atom(&u8_from_number(i.clone())),
+        SExp::Integer(_, i) => {
+            if NewStyleIntConversion::setting() && *i == bi_zero() {
+                sha256tree_from_atom(&[])
+            } else {
+                sha256tree_from_atom(&u8_from_number(i.clone()))
+            }
+        }
         SExp::QuotedString(_, _, v) => sha256tree_from_atom(v),
         SExp::Atom(_, v) => sha256tree_from_atom(v),
     }

src/compiler/compiler.rs

@@ -10,7 +10,7 @@ use clvm_rs::allocator::Allocator;
 use crate::classic::clvm::__type_compatibility__::{bi_one, bi_zero};
 use crate::classic::clvm_tools::stages::stage_0::TRunProgram;
 
-use crate::compiler::clvm::sha256tree;
+use crate::compiler::clvm::{sha256tree, NewStyleIntConversion};
 use crate::compiler::codegen::{codegen, hoist_body_let_binding, process_helper_let_bindings};
 use crate::compiler::comptypes::{CompileErr, CompileForm, CompilerOpts, PrimaryCodegen};
 use crate::compiler::dialect::{AcceptedDialect, KNOWN_DIALECTS};
@@ -163,6 +163,7 @@ pub fn compile_file(
     content: &str,
     symbol_table: &mut HashMap<String, String>,
 ) -> Result<SExp, CompileErr> {
+    let _int_conversion_bug = NewStyleIntConversion::new(opts.dialect().int_fix);
     let srcloc = Srcloc::start(&opts.filename());
     let pre_forms = parse_sexp(srcloc.clone(), content.bytes())?;
     let mut context_wrapper = CompileContextWrapper::new(
@@ -318,6 +319,7 @@ impl CompilerOpts for DefaultCompilerOpts {
         sexp: Rc<SExp>,
         symbol_table: &mut HashMap<String, String>,
     ) -> Result<SExp, CompileErr> {
+        let _int_conversion_bug = NewStyleIntConversion::new(self.dialect.int_fix);
         let me = Rc::new(self.clone());
         let optimizer = get_optimizer(&sexp.loc(), me.clone())?;
         let mut context_wrapper =

src/compiler/dialect.rs

@@ -11,6 +11,7 @@ use crate::compiler::sexp::decode_string;
 pub struct AcceptedDialect {
     pub stepping: Option<i32>,
     pub strict: bool,
+    pub int_fix: bool,
 }
 
 /// A package containing the content we should insert when a dialect include is
@@ -44,6 +45,7 @@ lazy_static! {
                     accepted: AcceptedDialect {
                         stepping: Some(21),
                         strict: true,
+                        int_fix: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 22)
@@ -57,6 +59,7 @@ lazy_static! {
                     accepted: AcceptedDialect {
                         stepping: Some(22),
                         strict: false,
+                        int_fix: false,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 22)
@@ -70,6 +73,21 @@ lazy_static! {
                     accepted: AcceptedDialect {
                         stepping: Some(23),
                         strict: true,
+                        int_fix: false,
+                    },
+                    content: indoc! {"(
+                    (defconstant *chialisp-version* 23)
+                )"}
+                    .to_string(),
+                },
+            ),
+            (
+                "*standard-cl-23.1*",
+                DialectDescription {
+                    accepted: AcceptedDialect {
+                        stepping: Some(23),
+                        strict: true,
+                        int_fix: true,
                     },
                     content: indoc! {"(
                     (defconstant *chialisp-version* 23)

src/tests/classic/mod.rs

@@ -5,3 +5,4 @@ mod optimize;
 pub mod run;
 mod smoke;
 mod stage_2;
+mod zero_constant_generation;

src/tests/classic/smoke.rs

@@ -64,6 +64,15 @@ fn large_odd_sized_neg_opd() {
     assert_eq!(result.rest(), "(0xfde1e61f36454dc00001)");
 }
 
+#[test]
+fn mid_negative_value_opd_00() {
+    let mut allocator = Allocator::new();
+    let result = opd_conversion()
+        .invoke(&mut allocator, &"00".to_string())
+        .unwrap();
+    assert_eq!(result.rest(), "0x00");
+}
+
 #[test]
 fn mid_negative_value_opd_m1() {
     let mut allocator = Allocator::new();
@@ -827,3 +836,35 @@ fn test_sub_args() {
         "(\"body\" (f (\"test1\" \"test2\")) (f (r (\"test1\" \"test2\"))))"
     );
 }
+
+#[test]
+fn test_smoke_cl23_program_with_zero_folding() {
+    let mut s = Stream::new(None);
+    launch_tool(
+        &mut s,
+        &vec![
+            "run".to_string(),
+            "(mod () (include *standard-cl-23*) (defconst X (concat 0x00 0x00)) X)".to_string(),
+        ],
+        &"run".to_string(),
+        2,
+    );
+    let result = s.get_value().decode().trim().to_string();
+    assert_eq!(result, "(2 (1 . 2) (4 (1 . 0) 1))");
+}
+
+#[test]
+fn test_smoke_cl23_program_without_zero_folding() {
+    let mut s = Stream::new(None);
+    launch_tool(
+        &mut s,
+        &vec![
+            "run".to_string(),
+            "(mod () (include *standard-cl-23.1*) (defconst X (concat 0x00 0x00)) X)".to_string(),
+        ],
+        &"run".to_string(),
+        2,
+    );
+    let result = s.get_value().decode().trim().to_string();
+    assert_eq!(result, "(2 (1 . 2) (4 (1 . 0x0000) 1))");
+}

src/tests/classic/zero_constant_generation.rs

@@ -0,0 +1,37 @@
+use crate::tests::classic::run::{do_basic_brun, do_basic_run};
+
+#[test]
+fn test_fuzz_constant_gen() {
+    let test_program = "(mod X %1 (defconst C %c) (concat C X))";
+
+    let test_parallel_run = |test| {
+        let final_program_classic = test_program.replace("%1", "").replace("%c", test);
+        let final_program_23_1 = test_program
+            .replace("%1", "(include *standard-cl-23.1*)")
+            .replace("%c", test);
+        let classic = do_basic_run(&vec!["run".to_string(), final_program_classic]);
+        let new_ver = do_basic_run(&vec!["run".to_string(), final_program_23_1]);
+        let classic_output = do_basic_brun(&vec!["brun".to_string(), classic, "0x01".to_string()]);
+        let new_output = do_basic_brun(&vec!["brun".to_string(), new_ver, "0x01".to_string()]);
+        assert_eq!(classic_output, new_output);
+    };
+
+    for test in [
+        "0x00",
+        "(concat 0x00 0x00)",
+        "(concat 0 0x00)",
+        "(concat 1 0x00)",
+        "(concat -1 0x00)",
+        "(concat -129 0x00)",
+        "(concat 0x00 0)",
+        "(concat 0x00 1)",
+        "(concat 0x00 -1)",
+        "(concat 0x00 -129)",
+        "(concat 0x00 (sha256 0x00 0))",
+        "(concat 0x00 (sha256 0 0x00 0))",
+    ]
+    .iter()
+    {
+        test_parallel_run(test);
+    }
+}

src/tests/compiler/clvm.rs

@@ -15,7 +15,7 @@ use crate::classic::clvm::__type_compatibility__::{bi_one, bi_zero, Bytes, Bytes
 use crate::classic::clvm::casts::{bigint_to_bytes_clvm, bigint_to_bytes_unsigned};
 use crate::classic::clvm_tools::stages::stage_0::DefaultProgramRunner;
 
-use crate::compiler::clvm::{parse_and_run, sha256tree};
+use crate::compiler::clvm::{convert_to_clvm_rs, parse_and_run, sha256tree, NewStyleIntConversion};
 use crate::compiler::runtypes::RunFailure;
 use crate::compiler::sexp::{parse_sexp, SExp};
 use crate::compiler::srcloc::Srcloc;
@@ -231,3 +231,82 @@ fn test_sha256_tree_hash() {
         "156e86309040ed6bbfee805c9c6ca7eebc140490bd1b97d6d18fb8ebc91fd05a"
     );
 }
+
+#[test]
+fn test_sha256_integer_0_hash() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, bi_zero()));
+    let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex();
+    assert_eq!(
+        hash_result,
+        "4bf5122f344554c53bde2ebb8cd2b7e3d1600ad631c385a5d7cce23c7785459a"
+    );
+}
+
+#[test]
+fn test_sha256_integer_0_hash_pre_fix() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, bi_zero()));
+    let _old_style = NewStyleIntConversion::new(false);
+    let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex();
+    assert_eq!(
+        hash_result,
+        "47dc540c94ceb704a23875c11273e16bb0b8a87aed84de911f2133568115f254"
+    );
+}
+
+#[test]
+fn test_integer_0_to_clvm() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, bi_zero()));
+    let mut allocator = Allocator::new();
+    let clvm_target = convert_to_clvm_rs(&mut allocator, value).expect("ok");
+    let empty: [u8; 0] = [];
+    assert_eq!(allocator.atom(clvm_target), &empty);
+}
+
+#[test]
+fn test_integer_hash_255() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, 255_u32.to_bigint().unwrap()));
+    let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex();
+    assert_eq!(
+        hash_result,
+        "b7ae7729555ec6829c579c2602edc6cb94b4ed3d820ddda0a45ac54030f8a53d"
+    );
+}
+
+#[test]
+fn test_integer_hash_255_pre_fix() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, 255_u32.to_bigint().unwrap()));
+    let _old_style = NewStyleIntConversion::new(false);
+    let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex();
+    assert_eq!(
+        hash_result,
+        "b7ae7729555ec6829c579c2602edc6cb94b4ed3d820ddda0a45ac54030f8a53d"
+    );
+}
+
+#[test]
+fn test_integer_hash_m129() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, -129_i32.to_bigint().unwrap()));
+    let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex();
+    assert_eq!(
+        hash_result,
+        "5a0c1fec64751e82c0d4861d0bc19c7580525d2f47667956bbd9d79e260aae00"
+    );
+}
+
+#[test]
+fn test_integer_hash_m129_pre_fix() {
+    let srcloc = Srcloc::start("*test*");
+    let value = Rc::new(SExp::Integer(srcloc, -129_i32.to_bigint().unwrap()));
+    let _old_style = NewStyleIntConversion::new(false);
+    let hash_result = Bytes::new(Some(BytesFromType::Raw(sha256tree(value)))).hex();
+    assert_eq!(
+        hash_result,
+        "5a0c1fec64751e82c0d4861d0bc19c7580525d2f47667956bbd9d79e260aae00"
+    );
+}

src/tests/compiler/compiler.rs

@@ -43,6 +43,7 @@ fn run_string_maybe_opt(
         opts = opts.set_dialect(AcceptedDialect {
             stepping: Some(21),
             strict: true,
+            int_fix: false,
         });
     }
 
@@ -2396,6 +2397,7 @@ fn test_handle_explicit_empty_atom() {
     let opts = Rc::new(DefaultCompilerOpts::new(filename)).set_dialect(AcceptedDialect {
         stepping: Some(21),
         strict: true,
+        int_fix: false,
     });
 
     let atom = |s: &str| Rc::new(SExp::Atom(srcloc.clone(), s.as_bytes().to_vec()));

src/tests/compiler/optimizer/cse.rs

@@ -852,11 +852,13 @@ fn test_generated_cse(n: u32) {
     let opts21 = opts.set_dialect(AcceptedDialect {
         stepping: Some(21),
         strict: true,
+        int_fix: false,
     });
     let opts23 = opts
         .set_dialect(AcceptedDialect {
             stepping: Some(23),
             strict: true,
+            int_fix: false,
         })
         .set_optimize(true);
     let mut allocator = Allocator::new();

src/tests/compiler/optimizer/cse_fuzz.rs

@@ -246,6 +246,7 @@ impl PropertyTestState<FuzzT> for TrickyAssignExpectation {
             opts.set_dialect(AcceptedDialect {
                 stepping: Some(23),
                 strict: true,
+                int_fix: false,
             })
             .set_optimize(true),
         )