Skip to content

Implement PoT proving and verification optimized for AES (aarch64) #3561

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

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

nazar-pc
Copy link
Member

@nazar-pc nazar-pc commented Jun 2, 2025

This is a backport of nazar-pc/abundance#270 and follow-up to #3552

Curious what difference it makes on Apple Silicon.

Code contributor checklist:

Copy link
Member

@teor2345 teor2345 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, the locations of constants aren't a blocker

use core::slice;
use subspace_core_primitives::pot::{PotCheckpoints, PotOutput};

const NUM_ROUND_KEYS: usize = 11;
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 put this constant in the containing module, rather than repeating it in each module?
(Same question for other constants in this file.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I was a little less comfortable because this is a platform-specific file, so extracting it somewhere would require annoying cfg() conditions. The rest of constants are specific to aarch64 and not shared with x86-64, intrinsics behave slightly differently here, which also confused me at first.

@nazar-pc
Copy link
Member Author

nazar-pc commented Jun 3, 2025

Two tests hanged on macOS:

SLOW [>5820.000s] domain-client-operator tests::test_xdm_channel_allowlist_removed_after_xdm_resp_relaying
SLOW [>5820.000s] domain-client-operator tests::test_xdm_false_invalid_fraud_proof

Would be nice for someone to dedicate time to fix these flaky tests, they fail way too often for my liking.

@teor2345
Copy link
Member

teor2345 commented Jun 3, 2025

Same macOS test issue as #3535 (comment)

Seems like the 2025-05-31 nightly compiler or 2024 edition caused some instability in the macOS tests. We don't know which because they were combined into a single PR.

@nazar-pc
Copy link
Member Author

nazar-pc commented Jun 3, 2025

Well, if it reproduces more reliably we can kind of call it a good thing, easier to debug that way

@nazar-pc nazar-pc merged commit ab9d6e6 into main Jun 3, 2025
20 of 21 checks passed
@nazar-pc nazar-pc deleted the pot-aarch64-aes branch June 3, 2025 02:45
@jfrank-summit
Copy link
Member

jfrank-summit commented Jun 3, 2025

Curious what difference it makes on Apple Silicon.

Running on M4 Max:

group     main branch                     pot-aarch64-aes branch
-----     ---------------------        -------------------
prove     1641.5±446.51ms             1104.9±6.98ms    
verify     144.5±2.52ms                   134.1±2.47ms       

@nazar-pc
Copy link
Member Author

nazar-pc commented Jun 3, 2025

Surprisingly good proving time with a solid improvement for both, nice!

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