Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions .github/actions/cross-tests/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: "cross-tests"

inputs:
rust:
required: true
package:
required: true
target:
required: true
features:
required: true

runs:
using: "composite"
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ inputs.rust }}
target: ${{ inputs.target }}
override: true
- name: Install precompiled cross
run: |
export URL=$(curl -s https://api.github.com/repos/cross-rs/cross/releases/latest | \
jq -r '.assets[] | select(.name | contains("x86_64-unknown-linux-gnu.tar.gz")) | .browser_download_url')
wget -O /tmp/binaries.tar.gz $URL
tar -C /tmp -xzf /tmp/binaries.tar.gz
mv /tmp/cross ~/.cargo/bin
shell: bash
Comment on lines +23 to +30

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious how much time this actually saves versus cargo install cross, which we use elsewhere:

https://github.com/RustCrypto/elliptic-curves/blob/89d6ab4/.github/workflows/k256.yml#L136

Looking at the CI history, it takes about 1m11s

@aewag aewag Jan 25, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, I also thought about using cargo install cross. The benefit is not too big for the more complex CI code.

cross within my fork takes 30 - 40 seconds for a single run.
https://github.com/aewag/sponges/actions/runs/1745945800

I can push a variant using cargo install, and we can see if it performs much worse.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, there seems to be a different issue than performance:

The current MSRV of the keccak crate is 1.41. But this is not sufficient to build cross [1]. So best is probably to work with the precompiled cross. I removed the WIP commit for experimenting with cargo install.

[1] https://github.com/aewag/sponges/runs/4940099815?check_suite_focus=true#step:3:170

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's ok to bump MSRV, especially if we're making breaking changes anyway

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Isn't than the MSRV of cross the bottomline MSRV of keccak?

I just pushed a WIP with MSRV 1.46. This succeeds building cross. Interestingly prior versions fail [1], although cross MSRV is 1.42. Not really sure why this is the case.

Performance-wise it takes 2m for a single run compared to 30-40s with a precompiled binary.

[1] https://github.com/aewag/sponges/runs/4942085469?check_suite_focus=true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Perhaps @aewag can check me on this but I think the set-msrv pattern introduced in this PR (and also this one, which now has differing cross-install boilerplate) could potentially be extracted as a reusable workflow?

Yes, set-msrv is a candidate to be moved into a reusable workflow. For example, this would easily allow an extension to get the MSRV from the Cargo.toml instead of hardcoding it in the CI config. I think for such extensions it is nice to have common patterns at a single place.

Additionally it seems like actions-rs is unmaintained and a rather heavyweight solution for how we actually use it. Perhaps we could fork it and slim it down to our specific needs. That would eliminate dependence on third-party actions we don't control.

I suggest to install cross using a composite action. This seems to me the most lightweight approach, that is currently available. If actions-rs gets extended or cross supports better options to install, we could simply switch to that by modifying this action only.

@tarcieri tarcieri Feb 1, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@newpavlov WDYT? Should we give it a try?

It seems like we could use set-msrv everywhere to have MSRV specified once per workflow.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@tarcieri
Yeah, go ahead. We always can revert everything back if it will be too much trouble in the end.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I made a repo here: https://github.com/RustCrypto/actions

@aewag feel free to upstream the work there! It seems like we'll be able to unify the configs in sponges and hashes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made a repo here: https://github.com/RustCrypto/actions

@aewag feel free to upstream the work there! It seems like we'll be able to unify the configs in sponges and hashes

Perfect, we'll do that. 👍

- if: ${{ inputs.features != 'NO_FEATURE' }}
run: |
cd ${{ inputs.package }}
cross test --target ${{ inputs.target }} --no-default-features \
--features ${{ inputs.features }}
shell: bash
- if: ${{ inputs.features == 'NO_FEATURE' }}
run: |
cd ${{ inputs.package }}
cross test --target ${{ inputs.target }} --no-default-features
shell: bash
7 changes: 7 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
version: 2
updates:
- package-ecosystem: cargo
directory: "/"
schedule:
interval: daily
open-pull-requests-limit: 10
101 changes: 101 additions & 0 deletions .github/workflows/keccak.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
name: keccak

on:
pull_request:
paths:
- ".github/workflows/keccak.yml"
- "keccak/**"
- "Cargo.*"
push:
branches: master

defaults:
run:
working-directory: keccak

env:
MSRV: 1.41.0
RUSTFLAGS: "-Dwarnings"
CARGO_INCREMENTAL: 0

jobs:
set-msrv:
runs-on: ubuntu-latest
outputs:
msrv: ${{ steps.msrv.outputs.msrv }}
steps:
- uses: actions/checkout@v2
- id: msrv
run: echo "::set-output name=msrv::$(echo $MSRV)"

build:
needs: set-msrv
runs-on: ubuntu-latest
strategy:
matrix:
rust:
- ${{needs.set-msrv.outputs.msrv}}
- stable
target:
- thumbv7em-none-eabi
- wasm32-unknown-unknown
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ matrix.rust }}
target: ${{ matrix.target }}
override: true
- run: cargo build --no-default-features --target ${{ matrix.target }}

test:
needs: set-msrv
runs-on: ubuntu-latest
strategy:
matrix:
rust:
- ${{needs.set-msrv.outputs.msrv}}
- stable
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ matrix.rust }}
override: true
- run: cargo check --all-features
- run: cargo test --no-default-features
- run: cargo test
- run: cargo test --all-features

# Cross-compiled tests
cross:
needs: set-msrv
strategy:
matrix:
rust:
- ${{needs.set-msrv.outputs.msrv}}
- stable
target:
- i686-unknown-linux-gnu
- x86_64-unknown-linux-gnu
- aarch64-unknown-linux-gnu
- mips-unknown-linux-gnu
features:
- no_unroll
- 'NO_FEATURE'

runs-on: ubuntu-latest
defaults:
run:
# Cross mounts only current package, i.e. by default it ignores workspace's Cargo.toml
working-directory: .
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/cross-tests
with:
rust: ${{ matrix.rust }}
package: ${{ github.workflow }}
target: ${{ matrix.target }}
features: ${{ matrix.features }}
24 changes: 24 additions & 0 deletions .github/workflows/security-audit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: Security Audit
on:
pull_request:
paths: Cargo.lock
push:
branches: master
paths: Cargo.lock
schedule:
- cron: "0 0 * * *"

jobs:
security_audit:
name: Security Audit
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- name: Cache cargo bin
uses: actions/cache@v1
with:
path: ~/.cargo/bin
key: ${{ runner.os }}-cargo-audit-v0.12.0
- uses: actions-rs/audit-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
43 changes: 43 additions & 0 deletions .github/workflows/workspace.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
name: Workspace

on:
pull_request:
paths-ignore:
- README.md
push:
branches: master
paths-ignore:
- README.md

jobs:
clippy:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions-rs/toolchain@v1
with:
toolchain: stable
components: clippy
profile: minimal
override: true
- run: cargo clippy --all -- -D warnings

rustfmt:
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v1

- name: Install stable toolchain
uses: actions-rs/toolchain@v1
with:
toolchain: stable
components: rustfmt
profile: minimal
override: true

- name: Run cargo fmt
uses: actions-rs/cargo@v1
with:
command: fmt
args: --all -- --check
34 changes: 0 additions & 34 deletions .travis.yml

This file was deleted.

21 changes: 0 additions & 21 deletions build_nostd.sh

This file was deleted.

3 changes: 0 additions & 3 deletions keccak/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,3 @@ categories = ["cryptography", "no-std"]

[features]
no_unroll = []

[badges]
travis-ci = { repository = "RustCrypto/sponges" }
68 changes: 7 additions & 61 deletions keccak/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,17 @@
#![no_std]
#![allow(non_upper_case_globals)]

#[rustfmt::skip]
mod unroll;

const PLEN: usize = 25;

const RHO: [u32; 24] = [
1, 3, 6, 10, 15, 21, 28, 36, 45, 55, 2, 14, 27, 41, 56, 8, 25, 43, 62, 18,
39, 61, 20, 44,
1, 3, 6, 10, 15, 21, 28, 36, 45, 55, 2, 14, 27, 41, 56, 8, 25, 43, 62, 18, 39, 61, 20, 44,
];

const PI: [usize; 24] = [
10, 7, 11, 17, 18, 3, 5, 16, 8, 21, 24, 4, 15, 23, 19, 13, 12, 2, 20, 14,
22, 9, 6, 1,
10, 7, 11, 17, 18, 3, 5, 16, 8, 21, 24, 4, 15, 23, 19, 13, 12, 2, 20, 14, 22, 9, 6, 1,
];

const RC: [u64; 24] = [
Expand Down Expand Up @@ -77,67 +78,12 @@ const RC: [u64; 24] = [
0x8000000080008008,
];

#[cfg(not(feature = "no_unroll"))]
macro_rules! unroll5 {
($var:ident, $body:block) => {
{ const $var: usize = 0; $body; }
{ const $var: usize = 1; $body; }
{ const $var: usize = 2; $body; }
{ const $var: usize = 3; $body; }
{ const $var: usize = 4; $body; }
};
}

#[cfg(feature = "no_unroll")]
macro_rules! unroll5 {
($var:ident, $body:block) => {
for $var in 0..5 $body
}
}

#[cfg(not(feature = "no_unroll"))]
macro_rules! unroll24 {
($var: ident, $body: block) => {
{ const $var: usize = 0; $body; }
{ const $var: usize = 1; $body; }
{ const $var: usize = 2; $body; }
{ const $var: usize = 3; $body; }
{ const $var: usize = 4; $body; }
{ const $var: usize = 5; $body; }
{ const $var: usize = 6; $body; }
{ const $var: usize = 7; $body; }
{ const $var: usize = 8; $body; }
{ const $var: usize = 9; $body; }
{ const $var: usize = 10; $body; }
{ const $var: usize = 11; $body; }
{ const $var: usize = 12; $body; }
{ const $var: usize = 13; $body; }
{ const $var: usize = 14; $body; }
{ const $var: usize = 15; $body; }
{ const $var: usize = 16; $body; }
{ const $var: usize = 17; $body; }
{ const $var: usize = 18; $body; }
{ const $var: usize = 19; $body; }
{ const $var: usize = 20; $body; }
{ const $var: usize = 21; $body; }
{ const $var: usize = 22; $body; }
{ const $var: usize = 23; $body; }
};
}

#[cfg(feature = "no_unroll")]
macro_rules! unroll24 {
($var:ident, $body:block) => {
for $var in 0..24 $body
}
}

#[allow(unused_assignments)]
/// Keccak-f[1600] sponge function
pub fn f1600(a: &mut [u64; PLEN]) {
// not unrolling this loop results in a much smaller function, plus
// it positively influences performance due to the smaller load on I-cache
for i in 0..24 {
for rc in &RC {
let mut array = [0u64; 5];

// Theta
Expand Down Expand Up @@ -179,6 +125,6 @@ pub fn f1600(a: &mut [u64; PLEN]) {
});

// Iota
a[0] ^= RC[i];
a[0] ^= rc;
}
}
Loading