Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip - do not merge #145

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/gen/condition_sanitizers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,20 @@ pub fn parse_height(a: &Allocator, n: NodePtr, code: ErrorCode) -> Result<u32, V
}

// negative seconds are always valid conditions, and will return 0
// xxx TODO: guard with condition types?
pub fn parse_seconds(a: &Allocator, n: NodePtr, code: ErrorCode) -> Result<u64, ValidationErr> {
// seconds are not allowed to exceed 2^64. i.e. 8 bytes
match sanitize_uint(a, n, 8, code) {
// seconds is always positive, so a negative requirement is always true,
// we don't need to include this condition
// (ASSERT_SECONDS_RELATIVE seconds_passed) requires AT LEAST seconds_passed
// to have passed. Therefore, seconds_passed <= 0 is okay
// (ASSERT_SECONDS_RELATIVE 0) is a no-op. We don't need to pass the condition on
// Same for (ASSERT_SECONDS_ABSOLUTE 0) or (ASSERT_SECONDS_ABSOLUTE -1)

//Err(ValidationErr(_, ErrorCode::NegativeAmount)) => Ok(None),
//Ok(0) => Ok(None),

Err(ValidationErr(_, ErrorCode::NegativeAmount)) => Ok(0),
Err(ValidationErr(n, ErrorCode::AmountExceedsMaximum)) => Err(ValidationErr(n, code)),
Err(r) => Err(r),
Expand Down
90 changes: 63 additions & 27 deletions src/gen/conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,12 +283,27 @@ pub fn parse_args(
ASSERT_SECONDS_RELATIVE => {
maybe_check_args_terminator(a, c, flags)?;
let seconds = parse_seconds(a, first(a, c)?, ErrorCode::AssertSecondsRelative)?;
Ok(Condition::AssertSecondsRelative(seconds))
match seconds {
// (ASSERT_SECONDS_RELATIVE seconds_passed) requires AT LEAST seconds_passed
// to have passed. Therefore, seconds_passed <= 0 is okay
// (ASSERT_SECONDSRELATIVE 0) is a no-op. We don't need to pass the condition on
0 => Ok(Condition::Skip),
_ => Ok(Condition::AssertSecondsRelative(seconds))
}
}
ASSERT_SECONDS_ABSOLUTE => {
maybe_check_args_terminator(a, c, flags)?;
let seconds = parse_seconds(a, first(a, c)?, ErrorCode::AssertSecondsAbsolute)?;
Ok(Condition::AssertSecondsAbsolute(seconds))
/*
match seconds {
// (ASSERT_SECONDS_ABSOLUTE seconds_passed) requires AT LEAST seconds_passed
// to have passed. Therefore, seconds_passed <= 0 is okay
// (ASSERT_SECONDS_ABSOLUTE 0) is a no-op. We don't need to pass the condition on
0 => Ok(Condition::Skip),
_ => Ok(Condition::AssertSecondsAbsolute(seconds))
}
*/
}
ASSERT_HEIGHT_RELATIVE => {
maybe_check_args_terminator(a, c, flags)?;
Expand Down Expand Up @@ -388,13 +403,17 @@ pub struct Spend {
// coin_amount and puzzle_hash
pub coin_id: Arc<Bytes32>,
// conditions
// all these integers are initialized to 0, which also means "no
// constraint". i.e. a 0 in these conditions are inherently satisified and
// ignored. 0 (or negative values) are not passed up to the next layer
// A condition argument initialized to nil in the rust code means "no constraint".
// xxx TODO: add tests that use CLVM nil as condition arguments
// nil, 0, or negative values are not passed up to the next layer
// One exception is height_relative, where 0 *is* relevant.
// the highest height/time conditions (i.e. most strict)
// Only the most restrictive condition of each type is passed to the layer above.
// height_relative: the highest HEIGHT_RELATIVE found for this spend
pub height_relative: Option<u32>,
pub seconds_relative: u64,
// The largest SECONDS_RELATIVE condition found. Note that negative
// SECONDS_RELATIVE values will pass validation, and no SECONDS_RELATIVE
// condition will be passed up.
pub seconds_relative: Option<u64>,
// the most restrictive ASSERT_BEFORE_HEIGHT_RELATIVE condition (if any)
// returned by this puzzle. It doesn't make much sense for a puzzle to
// return multiple of these, but if they do, we only need to care about the
Expand Down Expand Up @@ -516,7 +535,7 @@ pub(crate) fn parse_single_spend(
puzzle_hash,
coin_id,
height_relative: None,
seconds_relative: 0,
seconds_relative: None,
before_height_relative: None,
before_seconds_relative: None,
birth_height: None,
Expand Down Expand Up @@ -594,16 +613,19 @@ pub fn parse_conditions(
}
Condition::AssertSecondsRelative(s) => {
// keep the most strict condition. i.e. the highest limit
spend.seconds_relative = max(spend.seconds_relative, s);
spend.seconds_relative = max(spend.seconds_relative, Some(s));
if let Some(bs) = spend.before_seconds_relative {
if bs <= spend.seconds_relative {
// this spend bundle requres to be spent *before* a
// timestamp and also *after* a timestamp that's the
// same or later. that's impossible.
return Err(ValidationErr(
c,
ErrorCode::ImpossibleSecondsRelativeConstraints,
));
if let Some(s) = spend.seconds_relative {
if bs <= s {
// this spend bundle requres to be spent *before* a
// timestamp and also *after* a timestamp that's the
// same or later. that's impossible.
// TODO: Check semantics of (ASSERT_SECONDS_RELATIVE 0) and (ASSERT_BEFORE_SECONDS_RELATIVE 0)
return Err(ValidationErr(
c,
ErrorCode::ImpossibleSecondsRelativeConstraints,
));
}
}
}
}
Expand Down Expand Up @@ -637,7 +659,7 @@ pub fn parse_conditions(
} else {
spend.before_seconds_relative = Some(s);
}
if s <= spend.seconds_relative {
if Some(s) <= spend.seconds_relative {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this works. I believe any Some(...) value will compare greater than None

// this spend bundle requres to be spent *before* a
// timestamp and also *after* a timestamp that's the
// same or later. that's impossible.
Expand Down Expand Up @@ -1207,7 +1229,7 @@ fn test_invalid_condition_args_terminator() {
assert_eq!(a.atom(spend.puzzle_hash), H2);
assert_eq!(spend.flags, ELIGIBLE_FOR_DEDUP);

assert_eq!(spend.seconds_relative, 50);
assert_eq!(spend.seconds_relative, Some(50));
}

#[test]
Expand All @@ -1234,7 +1256,7 @@ fn test_invalid_condition_list_terminator() {
assert_eq!(a.atom(spend.puzzle_hash), H2);
assert_eq!(spend.flags, ELIGIBLE_FOR_DEDUP);

assert_eq!(spend.seconds_relative, 50);
assert_eq!(spend.seconds_relative, Some(50));
}

#[test]
Expand Down Expand Up @@ -1326,7 +1348,7 @@ fn test_extra_arg_mempool(#[case] condition: ConditionOpcode, #[case] arg: &str)
#[cfg(test)]
#[rstest]
#[case(ASSERT_SECONDS_ABSOLUTE, "104", "", |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.seconds_absolute, 104))]
#[case(ASSERT_SECONDS_RELATIVE, "101", "", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, 101))]
#[case(ASSERT_SECONDS_RELATIVE, "101", "", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, Some(101)))]
#[case(ASSERT_HEIGHT_RELATIVE, "101", "", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.height_relative, Some(101)))]
#[case(ASSERT_HEIGHT_ABSOLUTE, "100", "", |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.height_absolute, 100))]
#[case(ASSERT_BEFORE_SECONDS_ABSOLUTE, "104", "", |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.before_seconds_absolute, Some(104)))]
Expand Down Expand Up @@ -1376,8 +1398,8 @@ fn test_extra_arg(
#[rstest]
#[case(ASSERT_SECONDS_ABSOLUTE, "104", |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.seconds_absolute, 104))]
#[case(ASSERT_SECONDS_ABSOLUTE, "-1", |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.seconds_absolute, 0))]
#[case(ASSERT_SECONDS_RELATIVE, "101", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, 101))]
#[case(ASSERT_SECONDS_RELATIVE, "-1", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, 0))]
#[case(ASSERT_SECONDS_RELATIVE, "101", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, Some(101)))]
#[case(ASSERT_SECONDS_RELATIVE, "-1", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, None))]
#[case(ASSERT_HEIGHT_RELATIVE, "101", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.height_relative, Some(101)))]
#[case(ASSERT_HEIGHT_RELATIVE, "-1", |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.height_relative, None))]
#[case(ASSERT_HEIGHT_ABSOLUTE, "100", |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.height_absolute, 100))]
Expand Down Expand Up @@ -1424,9 +1446,13 @@ fn test_single_condition(
#[case(ASSERT_BEFORE_HEIGHT_ABSOLUTE, "0x0100000000")]
#[case(ASSERT_BEFORE_HEIGHT_RELATIVE, "0x0100000000")]
#[case(ASSERT_SECONDS_ABSOLUTE, "-1")]
#[case(ASSERT_SECONDS_RELATIVE, "-1")]
//#[case(ASSERT_SECONDS_ABSOLUTE, "0")]
#[case(ASSERT_SECONDS_RELATIVE, "-1")] //xxx
//#[case(ASSERT_SECONDS_RELATIVE, "0")] //xxx
#[case(ASSERT_HEIGHT_ABSOLUTE, "-1")]
//#[case(ASSERT_HEIGHT_ABSOLUTE, "0")]
#[case(ASSERT_HEIGHT_RELATIVE, "-1")]
//#[case(ASSERT_HEIGHT_RELATIVE, "-0")] // xxx TODO: Check if clvm vas negative zero
fn test_single_condition_no_op(#[case] condition: ConditionOpcode, #[case] value: &str) {
let (_, conds) = cond_test(&format!(
"((({{h1}} ({{h2}} (123 ((({} ({} )))))",
Expand All @@ -1442,7 +1468,7 @@ fn test_single_condition_no_op(#[case] condition: ConditionOpcode, #[case] value
assert_eq!(spend.before_height_relative, None);
assert_eq!(spend.before_seconds_relative, None);
assert_eq!(spend.height_relative, None);
assert_eq!(spend.seconds_relative, 0);
assert_eq!(spend.seconds_relative, None); //xxx if this is None OR Some(0), it is equivalent to no condition.
}

#[cfg(test)]
Expand All @@ -1455,7 +1481,7 @@ fn test_single_condition_no_op(#[case] condition: ConditionOpcode, #[case] value
#[case(
ASSERT_SECONDS_RELATIVE,
"0x010000000000000000",
ErrorCode::AssertSecondsRelative
ErrorCode::AssertSecondsRelative// xxx Is this the only failure case?
)]
#[case(
ASSERT_HEIGHT_ABSOLUTE,
Expand Down Expand Up @@ -1483,7 +1509,7 @@ fn test_single_condition_no_op(#[case] condition: ConditionOpcode, #[case] value
ErrorCode::AssertBeforeSecondsRelative
)]
#[case(
ASSERT_BEFORE_SECONDS_RELATIVE,
ASSERT_BEFORE_SECONDS_RELATIVE,//xxx
"0",
ErrorCode::ImpossibleSecondsRelativeConstraints
)]
Expand Down Expand Up @@ -1611,7 +1637,7 @@ fn test_disable_assert_before(
#[rstest]
// we use the MAX value
#[case(ASSERT_SECONDS_ABSOLUTE, |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.seconds_absolute, 503))]
#[case(ASSERT_SECONDS_RELATIVE, |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, 503))]
#[case(ASSERT_SECONDS_RELATIVE, |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.seconds_relative, Some(503)))]
#[case(ASSERT_HEIGHT_RELATIVE, |_: &SpendBundleConditions, s: &Spend| assert_eq!(s.height_relative, Some(503)))]
#[case(ASSERT_HEIGHT_ABSOLUTE, |c: &SpendBundleConditions, _: &Spend| assert_eq!(c.height_absolute, 503))]
// we use the SUM of the values
Expand Down Expand Up @@ -3053,6 +3079,16 @@ fn test_assert_concurrent_puzzle_self() {
}

// the relative constraints clash because they are on the same coin spend
// xxx add this?
/*
#[case(
ASSERT_HEIGHT_RELATIVE,
0,
ASSERT_BEFORE_HEIGHT_RELATIVE,
0,
Some(ErrorCode::ImpossibleHeightRelativeConstraints)
)]
*/
#[cfg(test)]
#[rstest]
#[case(
Expand Down
2 changes: 2 additions & 0 deletions src/gen/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ pub const ASSERT_MY_BIRTH_SECONDS: ConditionOpcode = 74;
pub const ASSERT_MY_BIRTH_HEIGHT: ConditionOpcode = 75;

// the conditions below ensure that we're "far enough" in the future
// ASSERT_SECONDS_X means ASSERT_AFTER_SECONDS_X >= xxx
// wall-clock time
pub const ASSERT_SECONDS_RELATIVE: ConditionOpcode = 80;
pub const ASSERT_SECONDS_ABSOLUTE: ConditionOpcode = 81;

// block index
// ASSERT_HEIGHT_X means ASSERT_HEIGHT_SECONDS_X xxx >=
pub const ASSERT_HEIGHT_RELATIVE: ConditionOpcode = 82;
pub const ASSERT_HEIGHT_ABSOLUTE: ConditionOpcode = 83;

Expand Down
2 changes: 1 addition & 1 deletion src/gen/run_puzzle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn run_puzzle(
puzzle_hash: a.new_atom(&puzzle_hash)?,
coin_id,
height_relative: None,
seconds_relative: 0,
seconds_relative: None,
before_height_relative: None,
before_seconds_relative: None,
birth_height: None,
Expand Down
2 changes: 1 addition & 1 deletion wheel/generate_type_stubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def get_puzzle_and_solution_for_coin(program: bytes, args: bytes, max_cost: int,
"coin_id: bytes",
"puzzle_hash: bytes",
"height_relative: Optional[int]",
"seconds_relative: int",
"seconds_relative: Optional[int]",
"before_height_relative: Optional[int]",
"before_seconds_relative: Optional[int]",
"birth_height: Optional[int]",
Expand Down
2 changes: 1 addition & 1 deletion wheel/src/run_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub struct PySpend {
pub coin_id: Bytes32,
pub puzzle_hash: Bytes32,
pub height_relative: Option<u32>,
pub seconds_relative: u64,
pub seconds_relative: Option<u64>,
pub before_height_relative: Option<u32>,
pub before_seconds_relative: Option<u64>,
pub birth_height: Option<u32>,
Expand Down