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

Breakage in future Rust version #625

Closed
lcnr opened this issue Feb 29, 2024 · 1 comment · Fixed by #626
Closed

Breakage in future Rust version #625

lcnr opened this issue Feb 29, 2024 · 1 comment · Fixed by #626
Assignees

Comments

@lcnr
Copy link

lcnr commented Feb 29, 2024

Hello there, unfortunately frost-core uses an impl which should result in a coherence error, but due to an oversight in the existing compiler implementation, it has not been detected up until now. This impl must not be allowed as it may overlap.

impl<C> std::ops::MulAssign<Identifier<C>> for Scalar<C>
where
C: Ciphersuite,
{
fn mul_assign(&mut self, identifier: Identifier<C>) {
*self = *self * identifier.0
}
}

Scalar is defined as the associated type <<<C as Ciphersuite>::Group as Group>::Field as Field>::Scalar. Were a user to implement Field for their own type they could choose an associated type which already implements MulAssign<Identifier<Itself>>, causing an overlap. This is rust-lang/rust#99554.

We're currently working on a fix in rust-lang/rust#117164 and will likely land it as a future compat warning for now. This will then change to a hard error in the medium term. I can't tell how to best avoid this error? It may potentially be easiest to remove that impl entirely?

I apologize for the inconvenience and thank you for your understanding

@conradoplg
Copy link
Contributor

Thank you for reporting this. Since the impl is only used internally it should be easy to remove it.

@mpguerra mpguerra moved this to Sprint Backlog in FROST Mar 1, 2024
@conradoplg conradoplg moved this from Sprint Backlog to In Progress in FROST Mar 12, 2024
@conradoplg conradoplg moved this from In Progress to Review/QA in FROST Mar 12, 2024
@mpguerra mpguerra added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels May 28, 2024
@mpguerra mpguerra added this to the FROST v2.0.0 Release milestone Jun 5, 2024
@mergify mergify bot closed this as completed in #626 Jun 19, 2024
@github-project-automation github-project-automation bot moved this from Review/QA to Done in FROST Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants