Skip to content

Reuse field inversions in Shamir recovery and division - #2558

Open
carstenartur wants to merge 1 commit into
bcgit:mainfrom
carstenartur:codex/shamir-reuse-field-inversions
Open

carstenartur wants to merge 1 commit into
bcgit:mainfrom
carstenartur:codex/shamir-reuse-field-inversions

Conversation

@carstenartur

Copy link
Copy Markdown
Contributor

Reduce repeated field inversions in ShamirSplitSecret.getSecret() and divide(), preserving the existing API.

  • Accumulate interpolation numerator and denominator products, then invert once per coefficient: 16,256 to 128 inversions for 128 shares.
  • Reuse one inverse per eligible share row: 256 to 8 inversions for eight 32-byte shares.
  • Keep the original behavior for Polynomial subclasses and small inputs.
  • Add scaling and overridden-division regression tests to the threshold suite.

Verification at upstream e40316b:

  • 16 threshold JUnit tests pass on Java 17 with --release 8.
  • Both new subclass regressions fail on the unguarded optimization and pass on upstream original and this candidate.
  • All 1,594 differential probe outputs match original, Sandbox-generated and candidate source.
  • With -Xlint:all -Xmaxwarns 10000, both builds report the same 101 existing warnings; no added diagnostics in the threshold test closure.
  • git diff --check passes.

The full Gradle/core suite, Checkstyle and JMH were not run for this submission. Counts above refer to field inversions, not a measured runtime speedup.

Identified by the Sandbox mathematics cleanup:
https://github.com/carstenartur/sandbox
Sandbox qualification:
https://github.com/carstenartur/sandbox/actions/runs/42453057191/job/128646172609

Generated source was adapted for naming, comments and formatting while preserving its guards, masks, fallback paths and per-row schedule. ChatGPT assisted with preparation, regression tests and review.

Accumulate interpolation numerator and denominator products before
inverting, and reuse an inverse within each share row during division.
Keep the original paths for Polynomial subclasses and small inputs.
Add scaling and overridden-division regression coverage.

Identified with the Sandbox mathematics cleanup:
https://github.com/carstenartur/sandbox
AI-assisted preparation and review.
Copilot AI balanced review requested due to automatic review settings November 8, 2026 18:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues block the change to reduce repeated finite-field inversions.

No open issues 🎉

What changed in this PR

This PR reduces repeated finite-field inversions in Shamir secret recovery and share division while preserving the existing API and fallback behavior for Polynomial subclasses and small inputs.

Changes:

  • Combine interpolation products to use one inversion per recovery coefficient.
  • Reuse one inverse per eligible share row during division.
  • Add scaling and overridden-division regression tests to the threshold suite.
File Description
core/​src/​test/​java/​org/​bouncycastle/​crypto/​threshold/​test/​ShamirSecretSplitterTest.java Tests recovery after scaling shares and dividing them back.
core/​src/​test/​java/​org/​bouncycastle/​crypto/​threshold/​test/​AllTests.java Registers the new subclass regression tests.
core/​src/​test/​java/​org/​bouncycastle/​crypto/​threshold/​ShamirSplitSecretTest.java Verifies recovery and division preserve overridden division behavior.
core/​src/​main/​java/​org/​bouncycastle/​crypto/​threshold/​ShamirSplitSecret.java Reuses inversions in recovery and division, retaining fallback paths.

Review effort: Balanced

@dghgit dghgit self-assigned this Nov 12, 2026
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