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

fix(syscall): handle invalid input in bn256 syscalls #1022

Merged
merged 6 commits into from
Mar 22, 2023
Merged

fix(syscall): handle invalid input in bn256 syscalls #1022

merged 6 commits into from
Mar 22, 2023

Conversation

magicalne
Copy link
Contributor

Handle errors in bn256 and return errors for syscalls.

@Flouse Flouse requested review from jjyr and zeroqn March 13, 2023 08:52
@Flouse Flouse requested a review from a team March 13, 2023 08:53
jjyr
jjyr previously approved these changes Mar 14, 2023
fn read_pt(buf: &[u8]) -> Result<G1> {
let map_err = BnError::G1FieldError;
let px = Fq::from_slice(&buf[0..32]).map_err(map_err)?;
let py = Fq::from_slice(&buf[32..64]).map_err(map_err)?;
Ok(if px == Fq::zero() && py == Fq::zero() {
G1::zero()
Copy link
Collaborator

@jjyr jjyr Mar 14, 2023

Choose a reason for hiding this comment

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

Is this a breaking change? (is AffineG1(Fq::zero(), Fq::zero()) == G1::zero()? )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, actually I don't break any change here. But after checking the code, looks like AffineG1(Fq::zero(), Fq::zero()) and G1::zero() are not equivalent.
G1::zero() represents:

        G {
            x: P::Base::zero(),
            y: P::Base::one(),
            z: P::Base::zero(),
        }

And for AffineG1(Fq::zero(), Fq::zero()):

      G {
           x: x,
           y: y,
           z: P::Base::one(),
     };

@Flouse Do you remember about this? Is that intended?

Copy link
Collaborator

@Flouse Flouse Mar 17, 2023

Choose a reason for hiding this comment

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

Yes, that is intended.

As my understanding, when px == Fq::zero() && py == Fq::zero(),
it is a special coordinate at infinity with z == P::Base::zero().

similar implementation: https://github.com/openethereum/parity-ethereum/blob/c7f608ec740882eac94038249037ddd955b60d31/ethcore/src/builtin.rs#L408-L414

crates/generator/src/syscalls/mod.rs Outdated Show resolved Hide resolved
Flouse
Flouse previously approved these changes Mar 22, 2023
@Flouse Flouse changed the title Fix invalid inputs of bn256 could make vm exit. fix(syscall): handle invalid input in bn256 syscalls Mar 22, 2023
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.

3 participants