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

Decode FORS indices similarly to WOTS #51

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

Conversation

bwesterb
Copy link
Contributor

@bwesterb bwesterb commented May 2, 2023

Copy link
Contributor

@sfluhrer sfluhrer left a comment

Choose a reason for hiding this comment

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

Looks good to me

@sfluhrer
Copy link
Contributor

sfluhrer commented May 3, 2023

Oops, David Cooper is right; this does flip the bit order for each leaf. What we want is:

indices[i] ^= ((m[offset >> 3] >> (~offset & 0x7)) & 0x1) << (SPX_FORS_HEIGHT-1-j);

@bwesterb
Copy link
Contributor Author

bwesterb commented May 4, 2023

Indeed, whoops. Generating new test vectors now ...

@bwesterb
Copy link
Contributor Author

bwesterb commented May 4, 2023

Done.

Copy link
Contributor

@sfluhrer sfluhrer left a comment

Choose a reason for hiding this comment

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

Now it looks good

@mberry
Copy link

mberry commented Jun 27, 2023

Is there a timeline for this getting merged? It's a breaking change so perhaps should come with some forewarning?

@bwesterb
Copy link
Contributor Author

Is there a timeline for this getting merged? It's a breaking change so perhaps should come with some forewarning?

SPHINCS+ (or SLH-DSA as it will be called by NIST) is not finalised, and could very well see even more changes. If you want to adopt early, you should fix on a specific commit (and keep track for bug fixes.) I'm sorry, this is annoying for early adopters, but that's the risk of adopting early.

@mberry
Copy link

mberry commented Jun 27, 2023

It's all good, was simply wondering about how the rollout would happen.

Codewise the change is trivial, it is more the versioning and nomenclature around this change.

blakehartin pushed a commit to blakehartin/hybrid-pqc that referenced this pull request Nov 25, 2023
DogeProtocol added a commit to DogeProtocol/hybrid-pqc that referenced this pull request Nov 25, 2023
@bwesterb
Copy link
Contributor Author

Rebased on master.

@bwesterb
Copy link
Contributor Author

Rebased to include fix of #59. /cc @kste

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