Skip to content

Refactor ErrorCode to include NodePtr and drop ValidationErr #1348

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.

Sign up for GitHub

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

Open
cameroncooper wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactor ErrorCode to include NodePtr and drop ValidationErr #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 13 additions & 11 deletions crates/chia-consensus/fuzz/fuzz_targets/fast-forward.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use chia_consensus::conditions::{
use chia_consensus::consensus_constants::TEST_CONSTANTS;
use chia_consensus::fast_forward::fast_forward_singleton;
use chia_consensus::spend_visitor::SpendVisitor;
use chia_consensus::validation_error::{ErrorCode, ValidationErr};
use chia_consensus::error_code::ErrorCode;
use chia_protocol::Bytes32;
use chia_protocol::Coin;
use chia_protocol::CoinSpend;
Expand Down Expand Up @@ -73,7 +73,7 @@ fn run_puzzle(
solution: &[u8],
parent_id: &[u8],
amount: u64,
) -> core::result::Result<SpendBundleConditions, ValidationErr> {
) -> core::result::Result<SpendBundleConditions, ErrorCode> {
let puzzle = node_from_bytes(a, puzzle)?;
let solution = node_from_bytes(a, solution)?;

Expand Down Expand Up @@ -160,14 +160,16 @@ fn test_ff(
// 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,
u32::from(ErrorCode::AssertMyParentIdFailed(NodePtr::NIL)),
u32::from(ErrorCode::AssertMyCoinIdFailed(NodePtr::NIL)),
];

match (conditions1, conditions2) {
(Err(ValidationErr(n1, msg1)), Err(ValidationErr(n2, 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));
(Err(msg1), Err(msg2)) => {
let code1 = u32::from(msg1);
let code2 = u32::from(msg2);
if code1 != code2 {
assert!(discrepancy_errors.contains(&code1) || discrepancy_errors.contains(&code2));
Copy link
Copy Markdown

@cursor cursor Bot Feb 6, 2026

Choose a reason for hiding this comment

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

Fuzz test drops node-data comparison weakening assertion

Low Severity

The old error comparison checked both error code equality AND node data equality (node_to_bytes). The new code only compares u32 error codes via u32::from, completely dropping the node-data check. If two runs fail with the same error type but at different CLVM nodes (indicating a meaningful behavioral difference), the fuzz test will no longer detect it.

Fix in Cursor Fix in Web

}
}
(Ok(conditions1), Ok(conditions2)) => {
Expand Down Expand Up @@ -204,21 +206,21 @@ fn test_ff(
assert_eq!(spend1.create_coin, spend2.create_coin);
assert_eq!(spend1.flags, spend2.flags);
}
(Ok(conditions1), Err(ValidationErr(_n2, msg2))) => {
(Ok(conditions1), Err(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));
assert!(discrepancy_errors.contains(&u32::from(msg2)));
}
(Err(ValidationErr(_n1, msg1)), Ok(conditions2)) => {
(Err(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));
assert!(discrepancy_errors.contains(&u32::from(msg1)));
}
}
}
4 changes: 2 additions & 2 deletions crates/chia-consensus/fuzz/fuzz_targets/run-generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use chia_bls::Signature;
use chia_consensus::allocator::make_allocator;
use chia_consensus::consensus_constants::TEST_CONSTANTS;
use chia_consensus::run_block_generator::{run_block_generator, run_block_generator2};
use chia_consensus::validation_error::{ErrorCode, ValidationErr};
use chia_consensus::error_code::ErrorCode;
use clvmr::chia_dialect::LIMIT_HEAP;
use libfuzzer_sys::fuzz_target;

Expand Down Expand Up @@ -36,7 +36,7 @@ fuzz_target!(|data: &[u8]| {

#[allow(clippy::match_same_arms)]
match (r1, r2) {
(Err(ValidationErr(_, ErrorCode::CostExceeded)), Ok(_)) => {
(Err(ErrorCode::CostExceeded(_)), Ok(_)) => {
// Since run_block_generator2 cost less, it's not a problem if the
// original generator runs out of cost while the rust implementation
// succeeds. This is part of its features.
Expand Down
8 changes: 3 additions & 5 deletions crates/chia-consensus/fuzz/fuzz_targets/sanitize-uint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use libfuzzer_sys::fuzz_target;

use chia_consensus::sanitize_int::{SanitizedUint, sanitize_uint};
use chia_consensus::validation_error::{ErrorCode, ValidationErr};
use chia_consensus::error_code::ErrorCode;
use clvmr::allocator::Allocator;

fuzz_target!(|data: &[u8]| {
Expand All @@ -21,9 +21,8 @@ fuzz_target!(|data: &[u8]| {
Ok(SanitizedUint::PositiveOverflow) => {
assert!(data.len() > 8);
}
Err(ValidationErr(n, c)) => {
Err(ErrorCode::InvalidCoinAmount(n)) => {
assert!(n == atom);
assert!(c == ErrorCode::InvalidCoinAmount);
}
}

Expand All @@ -40,9 +39,8 @@ fuzz_target!(|data: &[u8]| {
Ok(SanitizedUint::PositiveOverflow) => {
assert!(data.len() > 4);
}
Err(ValidationErr(n, c)) => {
Err(ErrorCode::InvalidCoinAmount(n)) => {
assert!(n == atom);
assert!(c == ErrorCode::InvalidCoinAmount);
}
}
});
10 changes: 5 additions & 5 deletions crates/chia-consensus/src/additions_and_removals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use chia_protocol::Coin;

use crate::allocator::make_allocator;
use crate::consensus_constants::ConsensusConstants;
use crate::validation_error::{ErrorCode, ValidationErr, atom, first, next, rest};
use crate::error_code::{ErrorCode, atom, first, next, rest};
use chia_protocol::{Bytes, Bytes32};
use clvm_traits::FromClvm;
use clvm_utils::{TreeCache, tree_hash_cached};
Expand All @@ -24,7 +24,7 @@ pub fn additions_and_removals<GenBuf: AsRef<[u8]>, I: IntoIterator<Item = GenBuf
block_refs: I,
flags: u32,
constants: &ConsensusConstants,
) -> Result<(Vec<(Coin, Option<Bytes>)>, Vec<(Bytes32, Coin)>), ValidationErr>
) -> Result<(Vec<(Coin, Option<Bytes>)>, Vec<(Bytes32, Coin)>), ErrorCode>
where
<I as IntoIterator>::IntoIter: DoubleEndedIterator,
{
Expand Down Expand Up @@ -57,7 +57,7 @@ where
iter = rest;
let (_parent_id, (puzzle, _rest)) =
<(NodePtr, (NodePtr, NodePtr))>::from_clvm(&a, spend)
.map_err(|_| ValidationErr(spend, ErrorCode::InvalidCondition))?;
.map_err(|_| ErrorCode::InvalidCondition(spend))?;
cache.visit_tree(&a, puzzle);
}

Expand All @@ -67,7 +67,7 @@ where
// process the spend
let (parent_id, (puzzle, (amount, (solution, _spend_level_extra)))) =
<(Bytes32, (NodePtr, (u64, (NodePtr, NodePtr))))>::from_clvm(&a, spend)
.map_err(|_| ValidationErr(spend, ErrorCode::InvalidCondition))?;
.map_err(|_| ErrorCode::InvalidCondition(spend))?;

let Reduction(clvm_cost, mut iter) =
run_program(&mut a, &dialect, puzzle, solution, cost_left)?;
Expand Down Expand Up @@ -100,7 +100,7 @@ where
c = rest(&a, c)?;

let (puzzle_hash, (amount, hint)) = <(Bytes32, (u64, NodePtr))>::from_clvm(&a, c)
.map_err(|_| ValidationErr(c, ErrorCode::InvalidCondition))?;
.map_err(|_| ErrorCode::InvalidCondition(c))?;

let coin = Coin {
parent_coin_info: spend_id,
Expand Down
Loading
Loading