Skip to content

fix: panic prone code#375

Merged
eigmax merged 3 commits intoProjectZKM:pre-1.2.4from
felicityin:fix-issue
Dec 21, 2025
Merged

fix: panic prone code#375
eigmax merged 3 commits intoProjectZKM:pre-1.2.4from
felicityin:fix-issue

Conversation

@felicityin
Copy link
Contributor

No description provided.

@felicityin felicityin force-pushed the fix-issue branch 2 times, most recently from 3f58354 to d831156 Compare December 11, 2025 08:32
let len = u32::from_be_bytes(len_bytes) as usize;

if buf.len() != 4 + 2 * len {
tracing::error!("FpOp: Invalid buffer length");
Copy link
Member

Choose a reason for hiding this comment

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

remove the tracing::error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes it easier to locate the error, since the same exception is thrown in several places.

Copy link
Member

Choose a reason for hiding this comment

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

Define difference ExecutionError. The user can not access the proof network's log.


/// The execution failed while converting a slice to an array.
#[error("failed to convert slice {0} to array")]
IntoArrayError(String),
Copy link
Member

Choose a reason for hiding this comment

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

Can we receive an Error here? so we can construct IntoArrayError with any error instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because core::array::TryFromSliceError does not implement the Serialize trait.

) -> Result<AffinePoint<E>, CurveError> {
let computed_point = k256::AffinePoint::decompress(bytes_be.into(), Choice::from(sign as u8))
.into_option()
.ok_or_else(|| CurveError::InvalidAffinePoint(bytes_be.to_vec(), sign))?;
Copy link
Member

Choose a reason for hiding this comment

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

We try not to do any type conversion when raising an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There doesn't seem to be a better way here.

@eigmax eigmax changed the base branch from main to pre-1.2.4 December 21, 2025 02:47
@eigmax eigmax merged commit 39bf022 into ProjectZKM:pre-1.2.4 Dec 21, 2025
4 of 6 checks passed
eigmax pushed a commit that referenced this pull request Dec 21, 2025
* fix: panic prone code in hook.rs

* fix: panic prone code in events/precompiles/ec.rs

* fix: panic prone code in traces generation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants