Clean up backend features and vendor curve25519_dalek_derive #531

pinkforest · 2023-06-13T00:08:54Z

Closes #414 (again)
Implements:

CPU auto-detection feature / cfg gate #529 consensus to use cfg(curve25519_dalek_backend = "override")
Cleans up the unused backend features which we don't want to advertise for now
Vendors in unsafe_target_feature in as curve25519-dalek-derive (co-authored by for @koute)
cfg(curve25519_dalek_diagnostics="build") now emits compile_error build diagnostics used for build script testing
Found a missing zeroize feature gate -fixed
build script is now tested with tests/build_tests.sh and added to CI
Clean up & simplify the documentation for backend override

Notes

github runner had some weird issue with incremental compilation - fixed
Closes Failed backend override - Warning or Error #532 - see: 1ac7b2 - chose compile_error to simplify gating

Co-authored-by: Jan Bujak <jan@parity.io>

tarcieri · 2023-06-13T13:44:13Z

vendor/curve25519-dalek-derive/.gitignore

@@ -0,0 +1,2 @@
+/target


Do we really need a vendor/ directory?

I'd just put this in a toplevel directory called curve25519-dalek-derive, and perhaps eventually we can move the rest of the files in the toplevel directory into curve25519-dalek

yeah sure, it was already there so I thought to follow that. will move. I'll also send the workspace PR move after rc.3

rozbb

Thanks so much for doing this. It's a big PR. Left a few comments about clarity here and there.

src/backend/mod.rs

tests/build_tests.sh

README.md

rozbb · 2023-06-15T05:21:43Z

Manually verified that curve25519_dalek_derive/ mirrors the unsafe_target_feature repo at 389ae00, modulo renaming and metadata changes.

Revert "Reduce gates in enum and allow dead_code in it" This reverts commit 35b3cef.

build.rs

README.md

src/lib.rs

This reverts commit b87ad6b. When --cfg curve25519_dalek_backend="simd" is chosen it cannot be overriden as "serial" in build.rs and will result in both selected

rozbb · 2023-06-20T04:30:07Z

Thanks for the updates! Made a pass on the README which hopefully clarifies how backend selection now works. @koute @tarcieri take a look at just the "Backend" section if you get a chance and lmk if it's good to go.

koute · 2023-06-22T05:53:10Z

LGTM!

@pinkforest Thank you for pushing this forward!

pinkforest and others added 14 commits June 12, 2023 23:55

Vendor import unsafe_target_features as curve25519-dalek-derive

39c34cb

Co-authored-by: Jan Bujak <jan@parity.io>

Rename vendored unsafe_target_features to curve25519-dalek-derive

31511c2

Plumb curve25519_dalek to curve25519_dalek_derive

4c65862

Adjust CI for simd feature-less

6c78bcb

Remove feature gates from avx2/ifma

96107fc

De-plub rest of the redundant featuresets

1aa5038

mod vector does not depend on nightly

90e92fd

Found the missing zeroize feature gate

8dd4800

Fix cfg relay via build.rs

7b68f1e

Add buildtime compile diagnostics about backend selection

b530a2a

simd override fix

6af3d03

fmt

9923241

Empty

1933ca9

Fix diagnostics typo

ead53fb

tarcieri reviewed Jun 13, 2023

View reviewed changes

pinkforest added 10 commits June 13, 2023 14:04

Move curve25519-dalek-derive to top level

47adb22

Remove .gitignore from curve25519-dalek-derive

e367aaa

Update .gitignore for subdirs

1ffbb71

Re-label CI desc for simd

20b9149

Separate fiat from test default and add serial override test

ffdefaf

Add build script tests

2b8f0ae

Documentation changes

175bc67

Fix build test

b4114cc

Ensure no incremental compilation for build script tests

07d0755

Remove redundant LICENSE from derive crate

a3d3c65

pinkforest mentioned this pull request Jun 13, 2023

Simplifying backend selection #414

Closed

rozbb reviewed Jun 15, 2023

View reviewed changes

src/backend/mod.rs Outdated Show resolved Hide resolved

tests/build_tests.sh Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

tarcieri mentioned this pull request Jun 15, 2023

Creation of SigningKey performs an additional point multiplication dalek-cryptography/ed25519-dalek#305

Closed

Clean up backend selection doc further

eeb4b6c

pinkforest added 5 commits June 16, 2023 14:03

Target not platform

4fe1079

Reduce gates in enum and allow dead_code in it

35b3cef

The enum needs to be gated for now.

95dfca0

Revert "Reduce gates in enum and allow dead_code in it" This reverts commit 35b3cef.

Default backend is serial not auto

dc507b7

Adjust build script tests to serial not auto default

0bc5de2

koute reviewed Jun 17, 2023

View reviewed changes

build.rs Outdated Show resolved Hide resolved

koute reviewed Jun 17, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

koute reviewed Jun 17, 2023

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

pinkforest added 10 commits June 17, 2023 06:54

Fix simd default on backend

b11fa4d

Fix wording about instruction sets

f41ab86

Disable simd related features unless simd was determined via build

edeb5c2

Play the fmt game

840f8e5

Simply backend gates via build

b87ad6b

Revert "Simply backend gates via build"

2755aa3

This reverts commit b87ad6b. When --cfg curve25519_dalek_backend="simd" is chosen it cannot be overriden as "serial" in build.rs and will result in both selected

Correct gating for feature enable

9c42cc9

Add note and test about the override warning when unsuccesfull

91d0d6d

Reduce complexity in build gating via compile_error

1ac7b2c

Doc typo

770a394

pinkforest mentioned this pull request Jun 17, 2023

Failed backend override - Warning or Error #532

Closed

pinkforest added 4 commits June 17, 2023 10:01

Cosmetic panic error message fix

2eb2b17

Play the fmt game again

8bf3439

Refer to issue/532 around intended compile_error

1671d80

Squash annoying typo

1bcbda5

tarcieri mentioned this pull request Jun 19, 2023

Add hazmat module with ExpandedSecretKey, raw_sign, raw_sign_prehashed dalek-cryptography/ed25519-dalek#299

Merged

Some README cleanup

919c8b4

tarcieri approved these changes Jun 20, 2023

View reviewed changes

rozbb merged commit e429bde into dalek-cryptography:main Jun 22, 2023
19 checks passed

rozbb mentioned this pull request Oct 29, 2023

Fix licensing on -derive repo #590

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up backend features and vendor curve25519_dalek_derive #531

Clean up backend features and vendor curve25519_dalek_derive #531

pinkforest commented Jun 13, 2023 •

edited

tarcieri Jun 13, 2023

pinkforest Jun 13, 2023 •

edited

rozbb left a comment

rozbb commented Jun 15, 2023

rozbb commented Jun 20, 2023 •

edited

koute commented Jun 22, 2023

Clean up backend features and vendor curve25519_dalek_derive #531

Clean up backend features and vendor curve25519_dalek_derive #531

Conversation

pinkforest commented Jun 13, 2023 • edited

tarcieri Jun 13, 2023

Choose a reason for hiding this comment

pinkforest Jun 13, 2023 • edited

Choose a reason for hiding this comment

rozbb left a comment

Choose a reason for hiding this comment

rozbb commented Jun 15, 2023

rozbb commented Jun 20, 2023 • edited

koute commented Jun 22, 2023

pinkforest commented Jun 13, 2023 •

edited

pinkforest Jun 13, 2023 •

edited

rozbb commented Jun 20, 2023 •

edited