relax coin amount in fast-forward logic

fuzz/fuzz_targets/fast-forward.rs

@@ -1,16 +1,16 @@
 #![no_main]
 use chia::fast_forward::fast_forward_singleton;
-use chia::gen::conditions::MempoolVisitor;
+use chia::gen::conditions::{MempoolVisitor, ELIGIBLE_FOR_FF};
 use chia::gen::run_puzzle::run_puzzle;
-use chia::gen::validation_error::ValidationErr;
+use chia::gen::validation_error::{ErrorCode, ValidationErr};
 use chia_protocol::Bytes32;
 use chia_protocol::Coin;
 use chia_protocol::CoinSpend;
 use chia_traits::streamable::Streamable;
 use clvm_utils::tree_hash;
 use clvmr::serde::node_to_bytes;
-use clvmr::Allocator;
 use clvmr::ToNodePtr;
+use clvmr::{Allocator, NodePtr};
 use hex_literal::hex;
 use libfuzzer_sys::fuzz_target;
 use std::io::Cursor;
@@ -31,34 +31,59 @@ fuzz_target!(|data: &[u8]| {
     };
     let puzzle_hash = Bytes32::from(tree_hash(&a, puzzle));
 
-    let new_parent_coin = Coin {
-        parent_coin_info: new_parents_parent.as_slice().into(),
-        puzzle_hash,
-        amount: spend.coin.amount,
-    };
+    for new_amount in [0, 2, 3] {
+        for new_parent_amount in [0, 2, 3] {
+            let new_parent_coin = Coin {
+                parent_coin_info: new_parents_parent.as_slice().into(),
+                puzzle_hash,
+                amount: if new_parent_amount == 0 {
+                    spend.coin.amount
+                } else {
+                    new_parent_amount
+                },
+            };
 
-    let new_coin = Coin {
-        parent_coin_info: new_parent_coin.coin_id().into(),
-        puzzle_hash,
-        amount: spend.coin.amount,
-    };
+            let new_coin = Coin {
+                parent_coin_info: new_parent_coin.coin_id().into(),
+                puzzle_hash,
+                amount: if new_amount == 0 {
+                    spend.coin.amount
+                } else {
+                    new_amount
+                },
+            };
 
+            test_ff(
+                &mut a,
+                &spend,
+                &new_coin,
+                &new_parent_coin,
+                solution,
+                puzzle,
+            );
+        }
+    }
+});
+
+fn test_ff(
+    a: &mut Allocator,
+    spend: &CoinSpend,
+    new_coin: &Coin,
+    new_parent_coin: &Coin,
+    solution: NodePtr,
+    puzzle: NodePtr,
+) {
     // perform fast-forward
-    let Ok(new_solution) = fast_forward_singleton(
-        &mut a,
-        puzzle,
-        solution,
-        &spend.coin,
-        &new_coin,
-        &new_parent_coin,
-    ) else {
+    let Ok(new_solution) =
+        fast_forward_singleton(a, puzzle, solution, &spend.coin, new_coin, new_parent_coin)
+    else {
         return;
     };
-    let new_solution = node_to_bytes(&a, new_solution).expect("serialize new solution");
+    let new_solution = node_to_bytes(a, new_solution).expect("serialize new solution");
 
     // run original spend
     let conditions1 = run_puzzle::<MempoolVisitor>(
-        &mut a,
+        a,
         spend.puzzle_reveal.as_slice(),
         spend.solution.as_slice(),
         &spend.coin.parent_coin_info,
@@ -69,7 +94,7 @@ fuzz_target!(|data: &[u8]| {
 
     // run new spend
     let conditions2 = run_puzzle::<MempoolVisitor>(
-        &mut a,
+        a,
         spend.puzzle_reveal.as_slice(),
         new_solution.as_slice(),
         &new_coin.parent_coin_info,
@@ -78,19 +103,69 @@ fuzz_target!(|data: &[u8]| {
         0,
     );
 
+    // These are the kinds of failures that can happen because of the
+    // fast-forward. It's OK to fail in different ways before and after, as long
+    // as it's one of these failures
+    let discrepancy_errors = [
+        ErrorCode::AssertMyParentIdFailed,
+        ErrorCode::AssertMyCoinIdFailed,
+    ];
+
     match (conditions1, conditions2) {
         (Err(ValidationErr(n1, msg1)), Err(ValidationErr(n2, msg2))) => {
-            assert_eq!(msg1, msg2);
+            if msg1 != msg2 || node_to_bytes(a, n1).unwrap() != node_to_bytes(a, n2).unwrap() {
+                assert!(discrepancy_errors.contains(&msg1) || discrepancy_errors.contains(&msg2));
+            }
+        }
+        (Ok(conditions1), Ok(conditions2)) => {
+            assert_eq!(conditions1.reserve_fee, conditions2.reserve_fee);
+            assert_eq!(conditions1.height_absolute, conditions2.height_absolute);
+            assert_eq!(conditions1.seconds_absolute, conditions2.seconds_absolute);
+            assert_eq!(conditions1.agg_sig_unsafe, conditions2.agg_sig_unsafe);
+            assert_eq!(
+                conditions1.before_height_absolute,
+                conditions2.before_height_absolute
+            );
             assert_eq!(
-                node_to_bytes(&a, n1).unwrap(),
-                node_to_bytes(&a, n2).unwrap()
+                conditions1.before_seconds_absolute,
+                conditions2.before_seconds_absolute
             );
+            assert_eq!(conditions1.cost, conditions2.cost);
+
+            let spend1 = &conditions1.spends[0];
+            let spend2 = &conditions2.spends[0];
+            assert_eq!(spend1.create_coin, spend2.create_coin);
+            assert_eq!(spend1.coin_amount, spend.coin.amount);
+            assert_eq!(spend2.coin_amount, new_coin.amount);
+            assert!(a.atom_eq(spend1.puzzle_hash, spend2.puzzle_hash));
+
+            assert_eq!(spend1.height_relative, spend2.height_relative);
+            assert_eq!(spend1.seconds_relative, spend2.seconds_relative);
+            assert_eq!(spend1.before_height_relative, spend2.before_height_relative);
+            assert_eq!(
+                spend1.before_seconds_relative,
+                spend2.before_seconds_relative
+            );
+            assert_eq!(spend1.birth_height, spend2.birth_height);
+            assert_eq!(spend1.birth_seconds, spend2.birth_seconds);
+            assert_eq!(spend1.create_coin, spend2.create_coin);
+            assert_eq!(spend1.flags, spend2.flags);
         }
-        (Ok(conditions1), Ok(conditions2)) => {
-            assert!(conditions1.spends[0].create_coin == conditions2.spends[0].create_coin);
+        (Ok(conditions1), Err(ValidationErr(_n2, msg2))) => {
+            // if the spend is valid and becomes invalid when
+            // rebased/fast-forwarded, it should at least not be considered
+            // eligible.
+            assert!((conditions1.spends[0].flags & ELIGIBLE_FOR_FF) == 0);
+            assert!(discrepancy_errors.contains(&msg2));
         }
-        _ => {
-            panic!("unexpected");
+        (Err(ValidationErr(_n1, msg1)), Ok(conditions2)) => {
+            // if the spend is invalid and becomes valid when
+            // rebased/fast-forwarded, it should not be considered
+            // eligible. This is a bit of a far-fetched scenario, but could
+            // happen if there's an ASSERT_MY_COINID that's only valid after the
+            // fast-forward
+            assert!((conditions2.spends[0].flags & ELIGIBLE_FOR_FF) == 0);
+            assert!(discrepancy_errors.contains(&msg1));
         }
     }
-});
+}

src/fast_forward.rs

@@ -98,12 +98,6 @@ pub fn fast_forward_singleton(
         return Err(Error::CoinAmountEven);
     }
 
-    // in the case of fast-forwarding a spend, we require the amount to remain
-    // unchanged
-    if coin.amount != new_coin.amount || coin.amount != new_parent.amount {
-        return Err(Error::CoinAmountMismatch);
-    }
-
     // we can only fast-forward spends of singletons whose puzzle hash doesn't
     // change
     if coin.puzzle_hash != new_parent.puzzle_hash || coin.puzzle_hash != new_coin.puzzle_hash {
@@ -126,11 +120,9 @@ pub fn fast_forward_singleton(
         return Err(Error::NotSingletonModHash);
     }
 
-    // we can only fast-forward if the coin amount stay the same
-    // this is to minimize the risk of producing an invalid spend, after
-    // fast-forward. e.g. we might end up attempting to spend more that the
-    // amount of the coin
-    if coin.amount != new_solution.lineage_proof.parent_amount || coin.amount != new_parent.amount {
+    // if the current solution to the puzzle doesn't match the coin amount, it's
+    // an invalid spend. Don't try to fast-forward it
+    if coin.amount != new_solution.amount {
         return Err(Error::CoinAmountMismatch);
     }
 
@@ -169,6 +161,8 @@ pub fn fast_forward_singleton(
 
     // update the solution to use the new parent coin's information
     new_solution.lineage_proof.parent_parent_coin_id = new_parent.parent_coin_info;
+    new_solution.lineage_proof.parent_amount = new_parent.amount;
+    new_solution.amount = new_coin.amount;
 
     let expected_new_parent = new_parent.coin_id();
 
@@ -206,6 +200,8 @@ mod tests {
             "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"
         )]
         new_parents_parent: &str,
+        #[values(0, 1, 3, 5)] new_amount: u64,
+        #[values(0, 1, 3, 5)] prev_amount: u64,
     ) {
         let spend_bytes = fs::read(format!("ff-tests/{spend_file}.spend")).expect("read file");
         let spend = CoinSpend::from_bytes(&spend_bytes).expect("parse CoinSpend");
@@ -219,13 +215,21 @@ mod tests {
         let new_parent_coin = Coin {
             parent_coin_info: new_parents_parent.as_slice().into(),
             puzzle_hash,
-            amount: spend.coin.amount,
+            amount: if prev_amount == 0 {
+                spend.coin.amount
+            } else {
+                prev_amount
+            },
         };
 
         let new_coin = Coin {
             parent_coin_info: new_parent_coin.coin_id().into(),
             puzzle_hash,
-            amount: spend.coin.amount,
+            amount: if new_amount == 0 {
+                spend.coin.amount
+            } else {
+                new_amount
+            },
         };
 
         // perform fast-forward
@@ -353,20 +357,6 @@ mod tests {
             },
             Error::CoinAmountMismatch,
         );
-
-        run_ff_test(
-            |_a, _coin, new_coin, _new_parent, _puzzle, _solution| {
-                new_coin.amount = 3;
-            },
-            Error::CoinAmountMismatch,
-        );
-
-        run_ff_test(
-            |_a, _coin, _new_coin, new_parent, _puzzle, _solution| {
-                new_parent.amount = 3;
-            },
-            Error::CoinAmountMismatch,
-        );
     }
 
     fn parse_solution(a: &mut Allocator, solution: &[u8]) -> SingletonSolution<NodePtr> {
@@ -423,7 +413,7 @@ mod tests {
 
                 *solution = serialize_solution(a, &new_solution);
             },
-            Error::CoinAmountMismatch,
+            Error::ParentCoinMismatch,
         );
     }
 

src/gen/conditions.rs

@@ -44,7 +44,7 @@ pub const HAS_RELATIVE_CONDITION: u32 = 2;
 // 2. There are no AGG_SIG_ME, AGG_SIG_PARENT, AGG_SIG_PARENT_* conditions
 // 3. No ASSERT_MY_COIN_ID condition, no more than one ASSERT_MY_PARENT_ID condition
 //    (as the second condition)
-// 4. it has an output coin with the same puzzle hash and amount as the spend itself
+// 4. it has an output coin with the same puzzle hash as the spend itself
 pub const ELIGIBLE_FOR_FF: u32 = 4;
 
 pub struct EmptyVisitor {}
@@ -131,9 +131,10 @@ impl SpendVisitor for MempoolVisitor {
         // to look for something that looks like a singleton output, with the same
         // puzzle hash as our input coin
         if (spend.flags & ELIGIBLE_FOR_FF) != 0
-            && !spend.create_coin.iter().any(|c| {
-                c.amount == spend.coin_amount && a.atom(spend.puzzle_hash) == c.puzzle_hash
-            })
+            && !spend
+                .create_coin
+                .iter()
+                .any(|c| (c.amount & 1) == 1 && a.atom(spend.puzzle_hash) == c.puzzle_hash)
         {
             spend.flags &= !ELIGIBLE_FOR_FF;
         }
@@ -2753,7 +2754,7 @@ fn test_multiple_create_coin() {
         amount: 43_u64,
         hint: a.null()
     }));
-    assert_eq!(spend.flags, ELIGIBLE_FOR_DEDUP);
+    assert_eq!(spend.flags, ELIGIBLE_FOR_DEDUP | ELIGIBLE_FOR_FF);
 }
 
 #[test]
@@ -4311,12 +4312,13 @@ fn test_eligible_for_ff_even_amount() {
 #[cfg(test)]
 #[rstest]
 #[case(123, "{h2}", true)]
+#[case(121, "{h2}", true)]
 #[case(122, "{h1}", false)]
 #[case(1, "{h1}", false)]
 #[case(123, "{h1}", false)]
 fn test_eligible_for_ff_output_coin(#[case] amount: u64, #[case] ph: &str, #[case] eligible: bool) {
     // in order to be elgibible for fast forward, there needs to be an output
-    // coin with the same amount and same puzzle hash
+    // coin with the same puzzle hash
     // 51=CREATE_COIN
     let test: &str = &format!(
         "(\